Esanders has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/316198

Change subject: Cleanup ElementLinearData#sanitize
......................................................................

Cleanup ElementLinearData#sanitize

* Always decrement counter by one and continue when splicing forwards.
* Always test the character at 'i'.

Change-Id: I494c9b4d23ec3b0f97146c3beabc1d49e063b7a0
---
M src/dm/lineardata/ve.dm.ElementLinearData.js
M tests/dm/lineardata/ve.dm.ElementLinearData.test.js
2 files changed, 14 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor 
refs/changes/98/316198/1

diff --git a/src/dm/lineardata/ve.dm.ElementLinearData.js 
b/src/dm/lineardata/ve.dm.ElementLinearData.js
index b080ed7..f573a93 100644
--- a/src/dm/lineardata/ve.dm.ElementLinearData.js
+++ b/src/dm/lineardata/ve.dm.ElementLinearData.js
@@ -1084,7 +1084,6 @@
                        type = this.getType( i );
                        canContainContent = 
ve.dm.nodeFactory.canNodeContainContent( type );
                        isOpen = this.isOpenElementData( i );
-
                        // Apply type conversions
                        if ( rules.conversions && rules.conversions[ type ] ) {
                                type = rules.conversions[ type ];
@@ -1105,33 +1104,38 @@
                                ( rules.plainText && type !== 'paragraph' && 
type !== 'internalList' )
                        ) {
                                this.splice( i, 1 );
+                               len--;
                                // Make sure you haven't just unwrapped a 
wrapper paragraph
-                               if ( ve.getProp( this.getData( i ), 'internal', 
'generated' ) ) {
+                               if ( isOpen && ve.getProp( this.getData( i ), 
'internal', 'generated' ) ) {
                                        delete this.getData( i 
).internal.generated;
                                        if ( ve.isEmptyObject( this.getData( i 
).internal ) ) {
                                                delete this.getData( i 
).internal;
                                        }
                                }
+                               // Move pointer back and continue
                                i--;
-                               len--;
                                continue;
                        }
 
                        // Split on breaks
                        if ( !rules.allowBreaks && type === 'break' && 
contentElement ) {
                                this.splice( i, 2, { type: '/' + 
contentElement.type }, ve.copy( contentElement ) );
+                               // Move pointer back and continue
+                               i--;
+                               continue;
                        }
 
                        // If a node is empty but can contain content, then 
just remove it
                        if (
                                !rules.keepEmptyContentBranches &&
-                               i > 0 && !isOpen && this.isOpenElementData( i - 
1 ) &&
-                               !ve.getProp( this.getData( i - 1 ), 'internal', 
'generated' ) &&
+                               isOpen && this.isCloseElementData( i + 1 ) &&
+                               !ve.getProp( this.getData( i ), 'internal', 
'generated' ) &&
                                canContainContent
                        ) {
-                               this.splice( i - 1, 2 );
-                               i -= 2;
+                               this.splice( i, 2 );
                                len -= 2;
+                               // Move pointer back and continue
+                               i--;
                                continue;
                        }
 
@@ -1159,8 +1163,9 @@
                                if ( this.getCharacterData( i + 1 ).match( /\s/ 
) || this.getCharacterData( i - 1 ).match( /\s/ ) ) {
                                        // If whitespace-adjacent, remove the 
newline to avoid double spaces
                                        this.splice( i, 1 );
-                                       i--;
                                        len--;
+                                       // Move pointer back and continue
+                                       i--;
                                        continue;
                                } else {
                                        // ...otherwise replace it with a space
diff --git a/tests/dm/lineardata/ve.dm.ElementLinearData.test.js 
b/tests/dm/lineardata/ve.dm.ElementLinearData.test.js
index 8d4f48a..d1ec7a2 100644
--- a/tests/dm/lineardata/ve.dm.ElementLinearData.test.js
+++ b/tests/dm/lineardata/ve.dm.ElementLinearData.test.js
@@ -1593,7 +1593,7 @@
                                        { type: '/internalList' }
                                ],
                                rules: { plainText: true },
-                               msg: 'Headings converted to paragraph is 
plainText mode'
+                               msg: 'Headings converted to paragraph in 
plainText mode'
                        },
                        {
                                html: '<h1>Bar</h1>',

-- 
To view, visit https://gerrit.wikimedia.org/r/316198
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I494c9b4d23ec3b0f97146c3beabc1d49e063b7a0
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Esanders <esand...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to