jenkins-bot has submitted this change and it was merged.
Change subject: Prevent naming collisions when generating unique reference names
......................................................................
Prevent naming collisions when generating unique reference names
Simply generating ':3' as the "unique" name for the 4th reference
doesn't work. Even if getUniqueListKey() had been used, that only
checks for conflicts with names that have already been encountered
(i.e. occur in <ref> tags that precede the current one), not for
conflicts with names that first occur further down in the document.
The solution is to generate names at serialization time, when we
have full knowledge of which names are in use. Internally, we use
'literal/<name>' for names that literally appeared in the source,
and 'auto/<number>' for unnamed references. Then at serialization
time, we translate 'auto/<number>' to 'literal/:<number>' if needed
(i.e. if the reference was reused).
ve.dm.MWReferenceNode.js:
* toDataElement()
** Prefix listKey with literal/ or auto/ as appropriate
* toDomElements()
** Map auto/ listKeys to unique names
** Don't try to unset the name if not present (was unsetting a property
that didn't exist anyway)
ve.dm.InternalList.js:
* Remove now-unused isUniqueListKey()
* Rewrite getUniqueListKey()
** Make prefix configurable
** Take previously generated unique keys into account
** Map the same old key (auto/N) to the same generated key (literal/:M)
* Add getNextUniqueNumber() as a source for auto/N numbers: previously
used the length of the itemHtmlQueue, but that only works during
conversion, not from the UI dialog
ve.ui.MWReferenceDialog.js:
* For new references or conflicting names, generate an auto/N key and
let toDomElements() deal with actually mapping that to name
ve.dm.InternalList.test.js:
* Rename listKeys to new style
* Split the test case into two groups so we can test multi-group cases
* Add tests for getUniqueListKey()
ve.dm.mwExample.js:
* Rename things to new style
* Modify the test case so it attempts to trigger bug 54341
Bug: 54341
Change-Id: I726fb83e6fb66ffec643d996768a854ec9474b3d
(cherry picked from commit eb647434363c4b0f96ad471c39e12bce018fe264)
---
M modules/ve-mw/dm/nodes/ve.dm.MWReferenceNode.js
M modules/ve-mw/test/dm/ve.dm.InternalList.test.js
M modules/ve-mw/test/dm/ve.dm.mwExample.js
M modules/ve-mw/ui/dialogs/ve.ui.MWReferenceDialog.js
M modules/ve/dm/ve.dm.InternalList.js
5 files changed, 193 insertions(+), 105 deletions(-)
Approvals:
Catrope: Looks good to me, approved
jenkins-bot: Verified
diff --git a/modules/ve-mw/dm/nodes/ve.dm.MWReferenceNode.js
b/modules/ve-mw/dm/nodes/ve.dm.MWReferenceNode.js
index c471efe..b64b2c2 100644
--- a/modules/ve-mw/dm/nodes/ve.dm.MWReferenceNode.js
+++ b/modules/ve-mw/dm/nodes/ve.dm.MWReferenceNode.js
@@ -46,7 +46,8 @@
body = mwData.body ? mwData.body.html : '',
refGroup = mwData.attrs && mwData.attrs.group || '',
listGroup = this.name + '/' + refGroup,
- listKey = mwData.attrs && mwData.attrs.name !== undefined ?
mwData.attrs.name : ':' + converter.internalList.itemHtmlQueue.length,
+ autoKeyed = !mwData.attrs || mwData.attrs.name === undefined,
+ listKey = autoKeyed ? 'auto/' +
converter.internalList.getNextUniqueNumber() : 'literal/' + mwData.attrs.name,
queueResult = converter.internalList.queueItemHtml( listGroup,
listKey, body ),
listIndex = queueResult.index,
contentsUsed = ( body !== '' && queueResult.isNew );
@@ -69,7 +70,7 @@
ve.dm.MWReferenceNode.static.toDomElements = function ( dataElement, doc,
converter ) {
var itemNodeHtml, originalHtml, mwData, i, iLen, keyedNodes,
setContents, contentsAlreadySet,
- originalMw, childDomElements,
+ originalMw, childDomElements, listKeyParts, name,
el = doc.createElement( 'span' ),
itemNodeWrapper = doc.createElement( 'div' ),
itemNode = converter.internalList.getItemNode(
dataElement.attributes.listIndex ),
@@ -131,16 +132,30 @@
}
}
- // Set or clear key
- if (
- // We always assign unique keys. If the key hasn't be reused,
then don't render it.
- keyedNodes.length !== 1 ||
- !ve.dm.InternalList.static.isUniqueListKey(
dataElement.attributes.listKey )
- ) {
- ve.setProp( mwData, 'attrs', 'name',
dataElement.attributes.listKey );
- } else if ( mwData.attrs ) {
- delete mwData.attrs.listKey;
+ // Generate name
+ listKeyParts = dataElement.attributes.listKey.match(
/^(auto|literal)\/(.*)$/ );
+ if ( listKeyParts[1] === 'auto' ) {
+ // Only render a name if this key was reused
+ if ( keyedNodes.length > 1 ) {
+ // Allocate a unique list key, then strip the
'literal/'' prefix
+ name = converter.internalList.getUniqueListKey(
+ dataElement.attributes.listGroup,
+ dataElement.attributes.listKey,
+ // Generate a name starting with ':' to
distinguish it from normal names
+ 'literal/:'
+ ).substr( 'literal/'.length );
+ } else {
+ name = undefined;
+ }
+ } else {
+ // Use literal name
+ name = listKeyParts[2];
}
+ // Set name
+ if ( name !== undefined ) {
+ ve.setProp( mwData, 'attrs', 'name', name );
+ }
+
// Set or clear group
if ( dataElement.attributes.refGroup !== '' ) {
ve.setProp( mwData, 'attrs', 'group',
dataElement.attributes.refGroup );
diff --git a/modules/ve-mw/test/dm/ve.dm.InternalList.test.js
b/modules/ve-mw/test/dm/ve.dm.InternalList.test.js
index 3e0ccf1..8e3db71 100644
--- a/modules/ve-mw/test/dm/ve.dm.InternalList.test.js
+++ b/modules/ve-mw/test/dm/ve.dm.InternalList.test.js
@@ -9,7 +9,7 @@
/* Tests */
-QUnit.test( 'addNode/removeNode', 5, function ( assert ) {
+QUnit.test( 'addNode/removeNode', 6, function ( assert ) {
var doc = ve.dm.mwExample.createExampleDocument( 'references' ),
newInternalList = new ve.dm.InternalList( doc ),
referenceNodes = [
@@ -23,29 +23,44 @@
expectedNodes = {
'mwReference/': {
'keyedNodes': {
- ':0': [ referenceNodes[0] ],
- 'bar': [ referenceNodes[1],
referenceNodes[3] ],
- 'quux': [ referenceNodes[2] ],
- ':1': [ referenceNodes[4] ],
- ':2': [ referenceNodes[5] ]
+ 'auto/0': [ referenceNodes[0] ],
+ 'literal/bar': [ referenceNodes[1],
referenceNodes[3] ],
+ 'literal/:3': [ referenceNodes[2] ],
+ 'auto/1': [ referenceNodes[4] ]
},
'firstNodes': [
referenceNodes[0],
referenceNodes[1],
referenceNodes[2],
- referenceNodes[4],
- referenceNodes[5]
+ referenceNodes[4]
],
- 'indexOrder': [ 0, 1, 2, 3, 4 ]
+ 'indexOrder': [ 0, 1, 2, 3 ],
+ 'uniqueListKeys': {},
+ 'uniqueListKeysInUse': {}
+ },
+ 'mwReference/foo': {
+ 'keyedNodes': {
+ 'auto/2': [ referenceNodes[5] ]
+ },
+ 'firstNodes': [ undefined, undefined,
undefined, undefined, referenceNodes[5] ],
+ 'indexOrder': [ 4 ],
+ 'uniqueListKeys': {},
+ 'uniqueListKeysInUse': {}
}
};
- newInternalList.addNode( 'mwReference/', ':0', 0, referenceNodes[0] );
- newInternalList.addNode( 'mwReference/', 'bar', 1, referenceNodes[1] );
- newInternalList.addNode( 'mwReference/', 'quux', 2, referenceNodes[2] );
- newInternalList.addNode( 'mwReference/', 'bar', 1, referenceNodes[3] );
- newInternalList.addNode( 'mwReference/', ':1', 3, referenceNodes[4] );
- newInternalList.addNode( 'mwReference/', ':2', 4, referenceNodes[5] );
+ assert.deepEqualWithNodeTree(
+ doc.internalList.nodes,
+ expectedNodes,
+ 'Document construction populates internal list correctly'
+ );
+
+ newInternalList.addNode( 'mwReference/', 'auto/0', 0, referenceNodes[0]
);
+ newInternalList.addNode( 'mwReference/', 'literal/bar', 1,
referenceNodes[1] );
+ newInternalList.addNode( 'mwReference/', 'literal/:3', 2,
referenceNodes[2] );
+ newInternalList.addNode( 'mwReference/', 'literal/bar', 1,
referenceNodes[3] );
+ newInternalList.addNode( 'mwReference/', 'auto/1', 3, referenceNodes[4]
);
+ newInternalList.addNode( 'mwReference/foo', 'auto/2', 4,
referenceNodes[5] );
newInternalList.onTransact();
assert.deepEqualWithNodeTree(
@@ -56,12 +71,12 @@
newInternalList = new ve.dm.InternalList( doc );
- newInternalList.addNode( 'mwReference/', ':2', 4, referenceNodes[5] );
- newInternalList.addNode( 'mwReference/', ':1', 3, referenceNodes[4] );
- newInternalList.addNode( 'mwReference/', 'bar', 1, referenceNodes[3] );
- newInternalList.addNode( 'mwReference/', 'quux', 2, referenceNodes[2] );
- newInternalList.addNode( 'mwReference/', 'bar', 1, referenceNodes[1] );
- newInternalList.addNode( 'mwReference/', ':0', 0, referenceNodes[0] );
+ newInternalList.addNode( 'mwReference/foo', 'auto/2', 4,
referenceNodes[5] );
+ newInternalList.addNode( 'mwReference/', 'auto/1', 3, referenceNodes[4]
);
+ newInternalList.addNode( 'mwReference/', 'literal/bar', 1,
referenceNodes[3] );
+ newInternalList.addNode( 'mwReference/', 'literal/:3', 2,
referenceNodes[2] );
+ newInternalList.addNode( 'mwReference/', 'literal/bar', 1,
referenceNodes[1] );
+ newInternalList.addNode( 'mwReference/', 'auto/0', 0, referenceNodes[0]
);
newInternalList.onTransact();
@@ -71,7 +86,7 @@
'Nodes added in reverse order'
);
- newInternalList.removeNode( 'mwReference/', 'bar', 1, referenceNodes[1]
);
+ newInternalList.removeNode( 'mwReference/', 'literal/bar', 1,
referenceNodes[1] );
newInternalList.onTransact();
assert.deepEqualWithNodeTree(
@@ -79,26 +94,35 @@
{
'mwReference/': {
'keyedNodes': {
- ':0': [ referenceNodes[0] ],
- 'bar': [ referenceNodes[3] ],
- 'quux': [ referenceNodes[2] ],
- ':1': [ referenceNodes[4] ],
- ':2': [ referenceNodes[5] ]
+ 'auto/0': [ referenceNodes[0] ],
+ 'literal/bar': [ referenceNodes[3] ],
+ 'literal/:3': [ referenceNodes[2] ],
+ 'auto/1': [ referenceNodes[4] ]
},
'firstNodes': [
referenceNodes[0],
referenceNodes[3],
referenceNodes[2],
- referenceNodes[4],
- referenceNodes[5]
+ referenceNodes[4]
],
- 'indexOrder': [ 0, 2, 1, 3, 4 ]
+ 'indexOrder': [ 0, 2, 1, 3 ],
+ 'uniqueListKeys': {},
+ 'uniqueListKeysInUse': {}
+ },
+ 'mwReference/foo': {
+ 'keyedNodes': {
+ 'auto/2': [ referenceNodes[5] ]
+ },
+ 'firstNodes': [ undefined, undefined,
undefined, undefined, referenceNodes[5] ],
+ 'indexOrder': [ 4 ],
+ 'uniqueListKeys': {},
+ 'uniqueListKeysInUse': {}
}
},
'Keys re-ordered after one item of key removed'
);
- newInternalList.removeNode( 'mwReference/', 'bar', 1, referenceNodes[3]
);
+ newInternalList.removeNode( 'mwReference/', 'literal/bar', 1,
referenceNodes[3] );
newInternalList.onTransact();
assert.deepEqualWithNodeTree(
@@ -106,28 +130,37 @@
{
'mwReference/': {
'keyedNodes': {
- ':0': [ referenceNodes[0] ],
- 'quux': [ referenceNodes[2] ],
- ':1': [ referenceNodes[4] ],
- ':2': [ referenceNodes[5] ]
+ 'auto/0': [ referenceNodes[0] ],
+ 'literal/:3': [ referenceNodes[2] ],
+ 'auto/1': [ referenceNodes[4] ]
},
'firstNodes': [
referenceNodes[0],
undefined,
referenceNodes[2],
- referenceNodes[4],
- referenceNodes[5]
+ referenceNodes[4]
],
- 'indexOrder': [ 0, 2, 3, 4 ]
+ 'indexOrder': [ 0, 2, 3 ],
+ 'uniqueListKeys': {},
+ 'uniqueListKeysInUse': {}
+ },
+ 'mwReference/foo': {
+ 'keyedNodes': {
+ 'auto/2': [ referenceNodes[5] ]
+ },
+ 'firstNodes': [ undefined, undefined,
undefined, undefined, referenceNodes[5] ],
+ 'indexOrder': [ 4 ],
+ 'uniqueListKeys': {},
+ 'uniqueListKeysInUse': {}
}
},
'Keys truncated after last item of key removed'
);
- newInternalList.removeNode( 'mwReference/', ':0', 0, referenceNodes[0]
);
- newInternalList.removeNode( 'mwReference/', ':2', 4, referenceNodes[5]
);
- newInternalList.removeNode( 'mwReference/', ':1', 3, referenceNodes[4]
);
- newInternalList.removeNode( 'mwReference/', 'quux', 2,
referenceNodes[2] );
+ newInternalList.removeNode( 'mwReference/', 'auto/0', 0,
referenceNodes[0] );
+ newInternalList.removeNode( 'mwReference/foo', 'auto/2', 4,
referenceNodes[5] );
+ newInternalList.removeNode( 'mwReference/', 'auto/1', 3,
referenceNodes[4] );
+ newInternalList.removeNode( 'mwReference/', 'literal/:3', 2,
referenceNodes[2] );
newInternalList.onTransact();
assert.deepEqualWithNodeTree(
@@ -135,8 +168,17 @@
{
'mwReference/': {
'keyedNodes': {},
+ 'firstNodes': new Array( 4 ),
+ 'indexOrder': [],
+ 'uniqueListKeys': {},
+ 'uniqueListKeysInUse': {}
+ },
+ 'mwReference/foo': {
+ 'keyedNodes': {},
'firstNodes': new Array( 5 ),
- 'indexOrder': []
+ 'indexOrder': [],
+ 'uniqueListKeys': {},
+ 'uniqueListKeysInUse': {}
}
},
'All nodes removed'
@@ -148,7 +190,7 @@
doc = ve.dm.mwExample.createExampleDocument( 'references' ),
internalList = doc.getInternalList();
- insertion = internalList.getItemInsertion( 'mwReference/', 'foo', [] );
+ insertion = internalList.getItemInsertion( 'mwReference/',
'literal/foo', [] );
index = internalList.getItemNodeCount();
assert.equal( insertion.index, index, 'Insertion creates a new
reference' );
assert.deepEqual(
@@ -167,7 +209,30 @@
],
'New reference operations match' );
- insertion = internalList.getItemInsertion( 'mwReference/', 'foo', [] );
+ insertion = internalList.getItemInsertion( 'mwReference/',
'literal/foo', [] );
assert.equal( insertion.index, index, 'Insertion with duplicate key
reuses old index' );
assert.equal( insertion.transaction, null, 'Insertion with duplicate
key has null transaction' );
} );
+
+QUnit.test( 'getUniqueListKey', 7, function ( assert ) {
+ var generatedName,
+ doc = ve.dm.mwExample.createExampleDocument( 'references' ),
+ internalList = doc.getInternalList();
+
+ generatedName = internalList.getUniqueListKey( 'mwReference/',
'auto/0', 'literal/:' );
+ assert.equal( generatedName, 'literal/:0', '0 maps to 0' );
+ generatedName = internalList.getUniqueListKey( 'mwReference/',
'auto/1', 'literal/:' );
+ assert.equal( generatedName, 'literal/:1', '1 maps to 1' );
+ generatedName = internalList.getUniqueListKey( 'mwReference/',
'auto/2', 'literal/:' );
+ assert.equal( generatedName, 'literal/:2', '2 maps to 2' );
+ generatedName = internalList.getUniqueListKey( 'mwReference/',
'auto/3', 'literal/:' );
+ assert.equal( generatedName, 'literal/:4', '3 maps to 4 (because a
literal :3 is present)' );
+ generatedName = internalList.getUniqueListKey( 'mwReference/',
'auto/4', 'literal/:' );
+ assert.equal( generatedName, 'literal/:5', '4 maps to 5' );
+
+ generatedName = internalList.getUniqueListKey( 'mwReference/',
'auto/0', 'literal/:' );
+ assert.equal( generatedName, 'literal/:0', 'Reusing a key reuses the
name' );
+
+ generatedName = internalList.getUniqueListKey( 'mwReference/foo',
'auto/4', 'literal/:' );
+ assert.equal( generatedName, 'literal/:0', 'Different groups are
treated separately' );
+} );
\ No newline at end of file
diff --git a/modules/ve-mw/test/dm/ve.dm.mwExample.js
b/modules/ve-mw/test/dm/ve.dm.mwExample.js
index adb0570..5993b3e 100644
--- a/modules/ve-mw/test/dm/ve.dm.mwExample.js
+++ b/modules/ve-mw/test/dm/ve.dm.mwExample.js
@@ -369,7 +369,7 @@
'contentsUsed': true,
'listGroup': 'mwReference/',
'listIndex': 0,
- 'listKey': '"No name 1"',
+ 'listKey': 'auto/0',
'mw': {
'attrs': {},
'body': { 'html': 'No name 1' },
@@ -398,7 +398,7 @@
'contentsUsed': true,
'listGroup': 'mwReference/',
'listIndex': 1,
- 'listKey': 'bar',
+ 'listKey': 'literal/bar',
'mw': {
'attrs': { 'name': 'bar' },
'body': { 'html': 'Bar' },
@@ -425,13 +425,13 @@
'contentsUsed': true,
'listGroup': 'mwReference/',
'listIndex': 2,
- 'listKey': 'quux',
+ 'listKey': 'literal/:3',
'mw': {
- 'attrs': { 'name': 'quux' },
+ 'attrs': { 'name': ':3' },
'body': { 'html': 'Quux' },
'name': 'ref'
},
- 'originalMw':
'{"name":"ref","body":{"html":"Quux"},"attrs":{"name":"quux"}}',
+ 'originalMw':
'{"name":"ref","body":{"html":"Quux"},"attrs":{"name":":3"}}',
'refGroup': ''
},
'htmlAttributes': [ { 'values': {
@@ -452,7 +452,7 @@
'contentsUsed': false,
'listGroup': 'mwReference/',
'listIndex': 1,
- 'listKey': 'bar',
+ 'listKey': 'literal/bar',
'mw': {
'attrs': { 'name': 'bar' },
'name': 'ref'
@@ -480,7 +480,7 @@
'contentsUsed': true,
'listGroup': 'mwReference/',
'listIndex': 3,
- 'listKey': '"No name 2"',
+ 'listKey': 'auto/1',
'mw': {
'attrs': {},
'body': { 'html': 'No name 2' },
@@ -504,16 +504,16 @@
'type': 'mwReference',
'attributes': {
'contentsUsed': true,
- 'listGroup': 'mwReference/',
+ 'listGroup': 'mwReference/foo',
'listIndex': 4,
- 'listKey': '"No name 3"',
+ 'listKey': 'auto/2',
'mw': {
- 'attrs': {},
+ 'attrs': { 'group': 'foo' },
'body': { 'html': 'No name 3' },
'name': 'ref'
},
- 'originalMw': '{"name":"ref","body":{"html":"No name
3"},"attrs":{}}',
- 'refGroup': ''
+ 'originalMw': '{"name":"ref","body":{"html":"No name
3"},"attrs":{"group":"foo"}}',
+ 'refGroup': 'foo'
},
'htmlAttributes': [ { 'values': {
'about': '#mwt12',
@@ -834,7 +834,7 @@
},
'mw:Reference': {
// Wikitext:
- // Foo<ref name="bar" /> Baz<ref name="quux">Quux</ref>
Whee<ref name="bar">[[Bar]]</ref> Yay<ref group="g1">No name</ref> Quux<ref
name="bar">Different content</ref> Foo<ref group="g1">No name</ref> Bar<ref
name="foo" />
+ // Foo<ref name="bar" /> Baz<ref group="g1"
name=":0">Quux</ref> Whee<ref name="bar">[[Bar]]</ref> Yay<ref group="g1">No
name</ref> Quux<ref name="bar">Different content</ref> Foo<ref group="g1">No
name</ref> Bar<ref name="foo" />
// <references><ref name="foo">Ref in refs</ref></references>
'html':
'<body>' +
@@ -843,7 +843,7 @@
'<a
href="#cite_note-bar-1">[1]</a>' +
'</span>' +
' Baz' +
- '<span about="#mwt2" class="reference"
data-mw="{"name":"ref","body":{"html":"Quux"},"attrs":{"name":"quux"}}"
id="cite_ref-quux-2-0" rel="dc:references" typeof="mw:Extension/ref"
data-parsoid="{}">' +
+ '<span about="#mwt2" class="reference"
data-mw="{"name":"ref","body":{"html":"Quux"},"attrs":{"group":"g1","name":":0"}}"
id="cite_ref-quux-2-0" rel="dc:references" typeof="mw:Extension/ref"
data-parsoid="{}">' +
'</span>' +
' Whee' +
'<span about="#mwt3" class="reference"
data-mw="{"name":"ref","body":{"html":"'
+
@@ -871,7 +871,7 @@
'attributes': {
'listIndex': 0,
'listGroup': 'mwReference/',
- 'listKey': 'bar',
+ 'listKey': 'literal/bar',
'refGroup': '',
'mw': { 'name': 'ref', 'attrs': {
'name': 'bar' } },
'originalMw':
'{"name":"ref","attrs":{"name":"bar"}}',
@@ -905,11 +905,11 @@
'type': 'mwReference',
'attributes': {
'listIndex': 1,
- 'listGroup': 'mwReference/',
- 'listKey': 'quux',
- 'refGroup': '',
- 'mw': { 'name': 'ref', 'body': {
'html': 'Quux' }, 'attrs': { 'name': 'quux' } },
- 'originalMw':
'{"name":"ref","body":{"html":"Quux"},"attrs":{"name":"quux"}}',
+ 'listGroup': 'mwReference/g1',
+ 'listKey': 'literal/:0',
+ 'refGroup': 'g1',
+ 'mw': { 'name': 'ref', 'body': {
'html': 'Quux' }, 'attrs': { 'group': 'g1', 'name': ':0' } },
+ 'originalMw':
'{"name":"ref","body":{"html":"Quux"},"attrs":{"group":"g1","name":":0"}}',
'childDomElements': [],
'contentsUsed': true
},
@@ -918,7 +918,7 @@
'values': {
'about': '#mwt2',
'class': 'reference',
- 'data-mw':
'{"name":"ref","body":{"html":"Quux"},"attrs":{"name":"quux"}}',
+ 'data-mw':
'{"name":"ref","body":{"html":"Quux"},"attrs":{"group":"g1","name":":0"}}',
'data-parsoid': '{}',
'id':
'cite_ref-quux-2-0',
'rel': 'dc:references',
@@ -934,7 +934,7 @@
'attributes': {
'listIndex': 0,
'listGroup': 'mwReference/',
- 'listKey': 'bar',
+ 'listKey': 'literal/bar',
'refGroup': '',
'mw': { 'name': 'ref', 'body': {
'html': '<a rel="mw:WikiLink" href="./Bar">Bar</a>' }, 'attrs': { 'name': 'bar'
} },
'originalMw':
'{"name":"ref","body":{"html":"<a rel=\\"mw:WikiLink\\"
href=\\"./Bar\\">Bar</a>"},"attrs":{"name":"bar"}}',
@@ -962,7 +962,7 @@
'attributes': {
'listIndex': 2,
'listGroup': 'mwReference/g1',
- 'listKey': ':2',
+ 'listKey': 'auto/0',
'refGroup': 'g1',
'mw': { 'name': 'ref', 'body': {
'html': 'No name' }, 'attrs': { 'group': 'g1' } },
'originalMw':
'{"name":"ref","body":{"html":"No name"},"attrs":{"group":"g1"}}',
@@ -990,7 +990,7 @@
'attributes': {
'listIndex': 0,
'listGroup': 'mwReference/',
- 'listKey': 'bar',
+ 'listKey': 'literal/bar',
'refGroup': '',
'mw': { 'name': 'ref', 'body': {
'html': 'Different content' }, 'attrs': { 'name': 'bar' } },
'originalMw':
'{"name":"ref","body":{"html":"Different content"},"attrs":{"name":"bar"}}',
@@ -1018,7 +1018,7 @@
'attributes': {
'listGroup': 'mwReference/',
'listIndex': 3,
- 'listKey': 'foo',
+ 'listKey': 'literal/foo',
'refGroup': '',
'mw': { 'name': 'ref', 'attrs': {
'name': 'foo' } },
'originalMw':
'{"name":"ref","attrs":{"name":"foo"}}',
@@ -1066,7 +1066,7 @@
'contentsUsed': true,
'listGroup': 'mwReference/',
'listIndex': 3,
- 'listKey': 'foo',
+ 'listKey': 'literal/foo',
'mw': { 'name': 'ref', 'attrs': {
'name': 'foo' }, 'body': { 'html': 'Ref in refs' } },
'originalMw':
'{"name":"ref","body":{"html":"Ref in refs"},"attrs":{"name":"foo"}}',
'refGroup': ''
@@ -1172,7 +1172,7 @@
'contentsUsed': true,
'listGroup': 'mwReference/',
'listIndex': 0,
- 'listKey': ':0',
+ 'listKey': 'auto/0',
'mw': {
'attrs': {},
'body': {
diff --git a/modules/ve-mw/ui/dialogs/ve.ui.MWReferenceDialog.js
b/modules/ve-mw/ui/dialogs/ve.ui.MWReferenceDialog.js
index 7030907..70bbfcf 100644
--- a/modules/ve-mw/ui/dialogs/ve.ui.MWReferenceDialog.js
+++ b/modules/ve-mw/ui/dialogs/ve.ui.MWReferenceDialog.js
@@ -199,7 +199,7 @@
keyIndex = internalList.getKeyIndex(
this.ref.listGroup, this.ref.listKey );
if ( keyIndex !== undefined ) {
// Resolve name collision by generating
a new list key
- this.ref.listKey =
internalList.getUniqueListKey( listGroup );
+ this.ref.listKey = 'auto/' +
internalList.getNextUniqueNumber();
}
// Update the group name of all references
nodes with the same group and key
txs = [];
@@ -235,7 +235,7 @@
listGroup = 'mwReference/' + refGroup;
// Create new internal item
this.ref = {
- 'listKey':
internalList.getUniqueListKey( listGroup ),
+ 'listKey': 'auto/' +
internalList.getNextUniqueNumber(),
'listGroup': listGroup,
'refGroup': refGroup
};
diff --git a/modules/ve/dm/ve.dm.InternalList.js
b/modules/ve/dm/ve.dm.InternalList.js
index e69a969..90d5e37 100644
--- a/modules/ve/dm/ve.dm.InternalList.js
+++ b/modules/ve/dm/ve.dm.InternalList.js
@@ -26,6 +26,7 @@
this.groupsChanged = [];
this.keyIndexes = {};
this.keys = [];
+ this.nextUniqueNumber = 0;
// Event handlers
if ( doc ) {
@@ -43,17 +44,6 @@
* @event update
* @param {string[]} groupsChanged List of groups changed since the last
transaction
*/
-
-/* Static methods */
-
-/**
- * Is a specific key an automatically generated unique key
- * @param {string} listKey List key
- * @returns {boolean} The key is an automatically generated unique key
- */
-ve.dm.InternalList.static.isUniqueListKey = function( listKey ) {
- return ( /^:[0-9]+$/ ).test( listKey );
-};
/* Methods */
@@ -173,23 +163,39 @@
/**
* Get a unique list key for a given group.
*
- * Generated list keys begin with ":", so that they are obviously different
from hand-coded ones.
+ * The returned list key is added to the list of unique list keys used in this
group so that it
+ * won't be allocated again. It will also be associated to oldListKey so that
if the same oldListKey
+ * is passed in again later, the previously allocated name will be returned.
*
* @method
* @param {string} groupName Name of the group
- * @returns {string} Unique list key
+ * @param {string} oldListKey Current list key to associate the generated list
key with
+ * @param {string} prefix Prefix to distinguish generated keys from
non-generated ones
+ * @returns {string} Generated unique list key, or existing unique key
associated with oldListKey
*/
-ve.dm.InternalList.prototype.getUniqueListKey = function ( groupName ) {
+ve.dm.InternalList.prototype.getUniqueListKey = function ( groupName,
oldListKey, prefix ) {
var group = this.getNodeGroup( groupName ),
num = 0;
- if ( group ) {
- while ( group.keyedNodes[':' + num ] ) {
- num++;
- }
+ if ( group.uniqueListKeys[oldListKey] !== undefined ) {
+ return group.uniqueListKeys[oldListKey];
}
- return ':' + num;
+ while ( group.keyedNodes[prefix + num] ||
group.uniqueListKeysInUse[prefix + num] ) {
+ num++;
+ }
+
+ group.uniqueListKeys[oldListKey] = prefix + num;
+ group.uniqueListKeysInUse[prefix + num] = true;
+ return prefix + num;
+};
+
+/**
+ * Get the next number in a monotonically increasing series.
+ * @returns {number} One higher than the return value of the previous call, or
0 on the first call
+ */
+ve.dm.InternalList.prototype.getNextUniqueNumber = function () {
+ return this.nextUniqueNumber++;
};
/**
@@ -294,7 +300,9 @@
group = this.nodes[groupName] = {
'keyedNodes': {},
'firstNodes': [],
- 'indexOrder': []
+ 'indexOrder': [],
+ 'uniqueListKeys': {},
+ 'uniqueListKeysInUse': {}
};
}
keyedNodes = group.keyedNodes[key];
--
To view, visit https://gerrit.wikimedia.org/r/85781
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I726fb83e6fb66ffec643d996768a854ec9474b3d
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: wmf/1.22wmf18
Gerrit-Owner: Catrope <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
