[MediaWiki-commits] [Gerrit] VisualEditor/VisualEditor[master]: Cleanup ElementLinearData#sanitize

2016-10-20 Thread jenkins-bot (Code Review) 
jenkins-bot has submitted this change and it was merged.

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(-)

Approvals:
  Jforrester: Looks good to me, approved
  jenkins-bot: Verified



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: 'Headin

[MediaWiki-commits] [Gerrit] VisualEditor/VisualEditor[master]: Cleanup ElementLinearData#sanitize

2016-10-15 Thread Esanders (Code Review) 
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: tru