Esanders has uploaded a new change for review.
https://gerrit.wikimedia.org/r/144046
Change subject: Restrict get relative offset to movements of +/-1
......................................................................
Restrict get relative offset to movements of +/-1
Moving in steps of greater than one is never done, and
would break the handlesOwnChildren detection.
Change-Id: Ic34563d6fed4f7c0aa348cc82c20a72f9f7531fb
---
M modules/ve/dm/lineardata/ve.dm.ElementLinearData.js
M modules/ve/test/dm/lineardata/ve.dm.ElementLinearData.test.js
2 files changed, 69 insertions(+), 152 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor
refs/changes/46/144046/1
diff --git a/modules/ve/dm/lineardata/ve.dm.ElementLinearData.js
b/modules/ve/dm/lineardata/ve.dm.ElementLinearData.js
index 9a5b4b9..d10d874 100644
--- a/modules/ve/dm/lineardata/ve.dm.ElementLinearData.js
+++ b/modules/ve/dm/lineardata/ve.dm.ElementLinearData.js
@@ -475,47 +475,45 @@
* - If {offset} is not already valid, one step will be used to move it to a
valid one.
* - If {offset} is already valid and cannot be moved in the direction of
{distance} and still be
* valid, it will be left where it is
- * - If {distance} is zero the result will either be {offset} if it's already
valid or the
+ * - If {initialDirection} is zero the result will either be {offset} if it's
already valid or the
* nearest valid offset to the right if possible and to the left otherwise.
- * - If {offset} is after the last valid offset and {distance} is >= 1, or if
{offset} if
- * before the first valid offset and {distance} <= 1 than the result will be
the nearest
+ * - If {offset} is after the last valid offset and {initialDirection} is 1,
or if {offset} is
+ * before the first valid offset and {initialDirection} is -1 then the
result will be the nearest
* valid offset in the opposite direction.
* - If the data does not contain a single valid offset the result will be -1
*
* @method
* @param {number} offset Offset to start from
- * @param {number} distance Number of valid offsets to move
+ * @param {number} initialDirection Initial direction so search in, -1/0/+1
* @param {Function} callback Function to call to check if an offset is valid
which will be
* given initial argument of offset
* @param {Mixed...} [args] Additional arguments to pass to the callback
* @returns {number} Relative valid offset or -1 if there are no valid offsets
in data
*/
-ve.dm.ElementLinearData.prototype.getRelativeOffset = function ( offset,
distance, callback ) {
+ve.dm.ElementLinearData.prototype.getRelativeOffset = function ( offset,
initialDirection, callback ) {
var i, direction,
dataOffset, isOpen,
args = Array.prototype.slice.call( arguments, 3 ),
start = offset,
- steps = 0,
turnedAround = false,
handlesOwnChildrenDepth = 0;
- // If offset is already a structural offset and distance is zero than
no further work is needed,
- // otherwise distance should be 1 so that we can get out of the invalid
starting offset
- if ( distance === 0 ) {
+ // If offset is already a structural offset and initialDirection is
zero than no further work is needed,
+ // otherwise initialDirection should be 1 so that we can get out of the
invalid starting offset
+ if ( initialDirection === 0 ) {
if ( callback.apply( this, [offset].concat( args ) ) ) {
return offset;
} else {
- distance = 1;
+ initialDirection = 1;
}
}
// Initial values
- direction = (
- offset <= 0 ? 1 : (
- offset >= this.getLength() ? -1 : (
- distance > 0 ? 1 : -1
- )
- )
- );
- distance = Math.abs( distance );
+ if ( offset <= 0 ) {
+ direction = 1;
+ } else if ( offset >= this.getLength() ) {
+ direction = -1;
+ } else {
+ direction = initialDirection;
+ }
i = start + direction;
offset = -1;
// Iteration
@@ -539,17 +537,11 @@
}
if ( callback.apply( this, [i].concat( args ) ) ) {
if ( !handlesOwnChildrenDepth ) {
- steps++;
- offset = i;
- if ( distance === steps ) {
- return offset;
- }
+ return i;
}
} else if (
// Don't keep turning around over and over
!turnedAround &&
- // Only turn around if not a single step could be taken
- steps === 0 &&
// Only turn around if we're about to reach the edge
( ( direction < 0 && i === 0 ) || ( direction > 0 && i
=== this.getLength() ) )
) {
@@ -561,7 +553,6 @@
// Start over going in the opposite direction
direction *= -1;
i = start;
- distance = 1;
turnedAround = true;
handlesOwnChildrenDepth = 0;
}
@@ -571,18 +562,18 @@
};
/**
- * Get a content offset at a distance from an offset.
+ * Get a content offset in a direction from an offset.
*
* This method is a wrapper around {getRelativeOffset}, using
{isContentOffset} as
* the offset validation callback.
*
* @method
* @param {number} offset Offset to start from
- * @param {number} distance Number of content offsets to move
+ * @param {number} direction Direction to move, -1/0/1
* @returns {number} Relative content offset or -1 if there are no valid
offsets in data
*/
-ve.dm.ElementLinearData.prototype.getRelativeContentOffset = function (
offset, distance ) {
- return this.getRelativeOffset( offset, distance,
this.constructor.prototype.isContentOffset );
+ve.dm.ElementLinearData.prototype.getRelativeContentOffset = function (
offset, direction ) {
+ return this.getRelativeOffset( offset, direction,
this.constructor.prototype.isContentOffset );
};
/**
@@ -613,24 +604,24 @@
};
/**
- * Get a structural offset at a distance from an offset.
+ * Get a structural offset in a direction from an offset.
*
* This method is a wrapper around {getRelativeOffset}, using
{this.isStructuralOffset} as
* the offset validation callback.
*
* @method
* @param {number} offset Offset to start from
- * @param {number} distance Number of structural offsets to move
+ * @param {number} direction Direction to move, -1/0/1
* @param {boolean} [unrestricted] Only consider offsets where any kind of
element can be inserted
* @returns {number} Relative structural offset
*/
-ve.dm.ElementLinearData.prototype.getRelativeStructuralOffset = function (
offset, distance, unrestricted ) {
+ve.dm.ElementLinearData.prototype.getRelativeStructuralOffset = function (
offset, direction, unrestricted ) {
// Optimization: start and end are always unrestricted structural
offsets
- if ( distance === 0 && ( offset === 0 || offset === this.getLength() )
) {
+ if ( direction === 0 && ( offset === 0 || offset === this.getLength() )
) {
return offset;
}
return this.getRelativeOffset(
- offset, distance,
this.constructor.prototype.isStructuralOffset, unrestricted
+ offset, direction,
this.constructor.prototype.isStructuralOffset, unrestricted
);
};
diff --git a/modules/ve/test/dm/lineardata/ve.dm.ElementLinearData.test.js
b/modules/ve/test/dm/lineardata/ve.dm.ElementLinearData.test.js
index beaa859..cd1899e 100644
--- a/modules/ve/test/dm/lineardata/ve.dm.ElementLinearData.test.js
+++ b/modules/ve/test/dm/lineardata/ve.dm.ElementLinearData.test.js
@@ -737,7 +737,7 @@
{
'msg': 'document without any valid offsets returns -1',
'offset': 0,
- 'distance': 1,
+ 'direction': 1,
'data': [],
'callback': function () {
return false;
@@ -745,14 +745,14 @@
'expected': -1
},
{
- 'msg': 'document with all valid offsets returns offset
+ distance',
+ 'msg': 'document with all valid offsets returns offset',
'offset': 0,
- 'distance': 2,
+ 'direction': 1,
'data': ['a', 'b'],
'callback': function () {
return true;
},
- 'expected': 2
+ 'expected': 1
}
];
QUnit.expect( cases.length );
@@ -761,7 +761,7 @@
assert.strictEqual(
data.getRelativeOffset(
cases[i].offset,
- cases[i].distance,
+ cases[i].direction,
cases[i].callback
),
cases[i].expected,
@@ -776,119 +776,83 @@
annDoc = ve.dm.example.createExampleDocument( 'annotationData'
),
cases = [
{
- 'msg': 'invalid starting offset with zero
distance gets corrected',
+ 'msg': 'invalid starting offset with zero
direction gets corrected',
'offset': 0,
- 'distance': 0,
+ 'direction': 0,
'expected': 1
},
{
- 'msg': 'invalid starting offset with zero
distance gets corrected',
+ 'msg': 'invalid starting offset with zero
direction gets corrected',
'offset': 61,
- 'distance': 0,
+ 'direction': 0,
'expected': 60
},
{
- 'msg': 'valid offset with zero distance returns
same offset',
+ 'msg': 'valid offset with zero direction
returns same offset',
'offset': 2,
- 'distance': 0,
+ 'direction': 0,
'expected': 2
},
{
'msg': 'invalid starting offset gets corrected',
'offset': 0,
- 'distance': -1,
+ 'direction': -1,
'expected': 1
},
{
'msg': 'invalid starting offset gets corrected',
'offset': 61,
- 'distance': 1,
+ 'direction': 1,
'expected': 60
},
{
'msg': 'stop at left edge if already valid',
'offset': 1,
- 'distance': -1,
+ 'direction': -1,
'expected': 1
},
{
'msg': 'stop at right edge if already valid',
'offset': 60,
- 'distance': 1,
+ 'direction': 1,
'expected': 60
},
{
- 'msg': 'first content offset is farthest left',
- 'offset': 2,
- 'distance': -2,
- 'expected': 1
- },
- {
- 'msg': 'last content offset is farthest right',
- 'offset': 59,
- 'distance': 2,
- 'expected': 60
- },
- {
- 'msg': '1 right within text',
+ 'msg': 'right within text',
'offset': 1,
- 'distance': 1,
+ 'direction': 1,
'expected': 2
},
{
- 'msg': '2 right within text',
- 'offset': 1,
- 'distance': 2,
- 'expected': 3
- },
- {
- 'msg': '1 left within text',
+ 'msg': 'left within text',
'offset': 2,
- 'distance': -1,
+ 'direction': -1,
'expected': 1
},
{
- 'msg': '2 left within text',
- 'offset': 3,
- 'distance': -2,
- 'expected': 1
- },
- {
- 'msg': '1 right over elements',
+ 'msg': 'right over elements',
'offset': 4,
- 'distance': 1,
+ 'direction': 1,
'expected': 10
},
{
- 'msg': '2 right over elements',
- 'offset': 4,
- 'distance': 2,
- 'expected': 11
- },
- {
- 'msg': '1 left over elements',
+ 'msg': 'left over elements',
'offset': 10,
- 'distance': -1,
+ 'direction': -1,
'expected': 4
- },
- {
- 'msg': '2 left over elements',
- 'offset': 10,
- 'distance': -2,
- 'expected': 3
},
{
'msg': 'Skips over nested handlesOwnChildren
nodes',
'doc': annDoc,
'offset': 10,
- 'distance': 1,
+ 'direction': 1,
'expected': 24
},
{
'msg': 'Skips over nested handlesOwnChildren
nodes (reverse)',
'doc': annDoc,
'offset': 23,
- 'distance': -1,
+ 'direction': -1,
'expected': 9
}
];
@@ -896,7 +860,7 @@
for ( i = 0; i < cases.length; i++ ) {
doc = cases[i].doc || simpleDoc;
assert.strictEqual(
- doc.data.getRelativeContentOffset( cases[i].offset,
cases[i].distance ),
+ doc.data.getRelativeContentOffset( cases[i].offset,
cases[i].direction ),
cases[i].expected,
cases[i].msg
);
@@ -964,105 +928,67 @@
doc = ve.dm.example.createExampleDocument(),
cases = [
{
- 'msg': 'invalid starting offset with zero
distance gets corrected',
+ 'msg': 'invalid starting offset with zero
direction gets corrected',
'offset': 1,
- 'distance': 0,
+ 'direction': 0,
'expected': 5
},
{
- 'msg': 'invalid starting offset with zero
distance gets corrected',
+ 'msg': 'invalid starting offset with zero
direction gets corrected',
'offset': 60,
- 'distance': 0,
+ 'direction': 0,
'expected': 61
},
{
- 'msg': 'valid offset with zero distance returns
same offset',
+ 'msg': 'valid offset with zero direction
returns same offset',
'offset': 0,
- 'distance': 0,
+ 'direction': 0,
'expected': 0
},
{
'msg': 'invalid starting offset gets corrected',
'offset': 2,
- 'distance': -1,
+ 'direction': -1,
'expected': 0
},
{
'msg': 'invalid starting offset gets corrected',
'offset': 59,
- 'distance': 1,
+ 'direction': 1,
'expected': 61
},
{
- 'msg': 'first structural offset is farthest
left',
- 'offset': 5,
- 'distance': -2,
- 'expected': 0
- },
- {
- 'msg': 'last structural offset is farthest
right',
- 'offset': 62,
- 'distance': 2,
- 'expected': 63
- },
- {
- 'msg': '1 right',
+ 'msg': 'right',
'offset': 0,
- 'distance': 1,
+ 'direction': 1,
'expected': 5
},
{
- 'msg': '1 right, unrestricted',
+ 'msg': 'right, unrestricted',
'offset': 5,
- 'distance': 1,
+ 'direction': 1,
'unrestricted': true,
'expected': 9
},
{
- 'msg': '2 right',
- 'offset': 0,
- 'distance': 2,
- 'expected': 6
- },
- {
- 'msg': '2 right, unrestricted',
- 'offset': 0,
- 'distance': 2,
- 'unrestricted': true,
- 'expected': 9
- },
- {
- 'msg': '1 left',
+ 'msg': 'left',
'offset': 61,
- 'distance': -1,
+ 'direction': -1,
'expected': 58
},
{
- 'msg': '1 left, unrestricted',
+ 'msg': 'left, unrestricted',
'offset': 9,
- 'distance': -1,
+ 'direction': -1,
'unrestricted': true,
'expected': 5
- },
- {
- 'msg': '2 left',
- 'offset': 61,
- 'distance': -2,
- 'expected': 55
- },
- {
- 'msg': '2 left, unrestricted',
- 'offset': 9,
- 'distance': -2,
- 'unrestricted': true,
- 'expected': 0
}
];
QUnit.expect( cases.length );
for ( i = 0; i < cases.length; i++ ) {
assert.strictEqual(
doc.data.getRelativeStructuralOffset(
- cases[i].offset, cases[i].distance,
cases[i].unrestricted
+ cases[i].offset, cases[i].direction,
cases[i].unrestricted
),
cases[i].expected,
cases[i].msg
--
To view, visit https://gerrit.wikimedia.org/r/144046
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic34563d6fed4f7c0aa348cc82c20a72f9f7531fb
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Esanders <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
