C. Scott Ananian has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/363743 )
Change subject: Fix infinite loop in ve.BranchNode#getNodeFromOffset
......................................................................
Fix infinite loop in ve.BranchNode#getNodeFromOffset
This is a minor clean up of the refactoring done in
1cbcc2f0bf66fd824c255a7e6ca1e87fdacd46e9.
If the document is empty (consisting only of ve.ce.InternalListNode)
then it is possible for `currentNode.children.length > 0` and
`currentNode.children[ 0 ] instanceof ve.ce.InternalListNode`.
This sets up an infinite loop, unless `offset === nodeOffset`.
This was being triggered when an empty document consisting only of
`<paragraph>, </paragraph>, <internalList>, </internalList>` was
p-unwrapped; for example, prior to paste in
ve.ui.MWWikitextStringTransferHandler.js or prior to update of a
`-{}-` construct by ve.ui.MWLanguageVariantInspector.
Change-Id: I933bf28637f3b3c235aa4b9f8d96d1d09a7b3085
---
M src/ve.BranchNode.js
M src/ve.Document.js
M tests/ce/ve.ce.Document.test.js
3 files changed, 37 insertions(+), 19 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor
refs/changes/43/363743/1
diff --git a/src/ve.BranchNode.js b/src/ve.BranchNode.js
index c2404db..477bb4b 100644
--- a/src/ve.BranchNode.js
+++ b/src/ve.BranchNode.js
@@ -155,14 +155,14 @@
while ( currentNode.children.length ) {
for ( i = 0, length = currentNode.children.length; i < length;
i++ ) {
childNode = currentNode.children[ i ];
- if ( childNode instanceof ve.ce.InternalListNode ) {
- break;
- }
if ( offset === nodeOffset ) {
// The requested offset is right before
childNode, so it's not
// inside any of currentNode's children, but is
inside currentNode
return currentNode;
}
+ if ( childNode instanceof ve.ce.InternalListNode ) {
+ break SIBLINGS;
+ }
nodeLength = childNode.getOuterLength();
if ( offset >= nodeOffset && offset < nodeOffset +
nodeLength ) {
if ( !shallow && childNode.hasChildren() &&
childNode.getChildren().length ) {
diff --git a/src/ve.Document.js b/src/ve.Document.js
index 58d7218..5e909c4 100644
--- a/src/ve.Document.js
+++ b/src/ve.Document.js
@@ -39,7 +39,7 @@
};
/**
- * Get a node a an offset.
+ * Get a node at an offset.
*
* @method
* @param {number} offset Offset to get node at
diff --git a/tests/ce/ve.ce.Document.test.js b/tests/ce/ve.ce.Document.test.js
index 4984c98..c1eebc8 100644
--- a/tests/ce/ve.ce.Document.test.js
+++ b/tests/ce/ve.ce.Document.test.js
@@ -35,7 +35,7 @@
// TODO: getDirectionFromRange
QUnit.test( 'getNodeAndOffset', function ( assert ) {
- var tests, i, iLen, test, parts, view, data, ceDoc, rootNode,
offsetCount, offset, j, jLen, node;
+ var tests, i, iLen, test, parts, view, data, dmDoc, ceDoc, rootNode,
offsetCount, offset, j, jLen, node, ex;
// Each test below has the following:
// html: an input document
@@ -63,6 +63,14 @@
html: '<div><p>x</p></div>',
data: [ '<div>', '<paragraph>', 'x', '</paragraph>',
'</div>' ],
positions: "<div class='ve-ce-branchNode
ve-ce-documentNode'>|<div class='ve-ce-branchNode-slug
ve-ce-branchNode-blockSlug'></div><div class='ve-ce-branchNode'>|<p
class='ve-ce-branchNode ve-ce-contentBranchNode
ve-ce-paragraphNode'><#text>|x|</#text></p>|</div><div
class='ve-ce-branchNode-slug ve-ce-branchNode-blockSlug'></div></div>"
+ },
+ {
+ title: 'Empty document',
+ html: '<p></p>',
+ unwrap: [ { type: 'paragraph' } ],
+ data: [],
+ positions: "",
+ dies: [ 1 ]
},
{
title: 'Slugless emptied paragraph',
@@ -121,7 +129,20 @@
test = tests[ i ];
parts = test.positions.split( /[|]/ );
view = ve.test.utils.createSurfaceViewFromHtml( test.html );
- data = view.getModel().getDocument().data.data
+ dmDoc = view.getModel().getDocument();
+ if ( test.unwrap ) {
+ new ve.dm.Surface( dmDoc ).change(
+ ve.dm.TransactionBuilder.static.newFromWrap(
+ dmDoc,
+ new ve.Range( 0,
dmDoc.data.countNonInternalElements() ),
+ [],
+ [],
+ test.unwrap,
+ []
+ )
+ );
+ }
+ data = dmDoc.data.data
.slice( 0, -2 )
.map( showModelItem );
ceDoc = view.documentView;
@@ -147,19 +168,6 @@
}
for ( offset = 0; offset < offsetCount; offset++ ) {
- try {
- if ( test.dies && test.dies.indexOf( offset )
!== -1 ) {
- assert.ok( false, test.title + ' (' +
offset + ') does not die' );
- continue;
- }
- } catch ( ex ) {
- assert.ok(
- test.dies && test.dies.indexOf( offset
) !== -1,
- test.title + ' (' + offset + ') dies'
- );
- continue;
- }
-
assert.strictEqual(
ve.test.utils.serializePosition(
rootNode,
@@ -174,6 +182,16 @@
test.title + ' (' + offset + ')'
);
}
+ for ( j = 0; test.dies && j < test.dies.length; j++ ) {
+ offset = test.dies[ j ];
+ ex = null;
+ try {
+ ceDoc.getNodeAndOffset( offset,
test.outsideNails );
+ } catch ( e ) {
+ ex = e;
+ }
+ assert.ok( ex !== null, test.title + ' (' + offset + ')
dies' );
+ }
view.destroy();
}
} );
--
To view, visit https://gerrit.wikimedia.org/r/363743
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I933bf28637f3b3c235aa4b9f8d96d1d09a7b3085
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: C. Scott Ananian <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
