Catrope has uploaded a new change for review.
https://gerrit.wikimedia.org/r/58259
Change subject: Fix ModelRegistry bugs
......................................................................
Fix ModelRegistry bugs
* In matchTypeRegExps(), skip string types
** Didn't break because we currently have mixes of strings and regexes
* Combine types from rel, typeof and property rather than picking one
** Added test case in ve.dm.example that resembles actual Parsoid output
* If the element has extension-specific types, not only restrict type
matching to extension-specific types, but also require that *all* types
present on the element be matched
Change-Id: Iacf3851a0ca9081d2c813b42435484a47cec6230
---
M modules/ve/dm/ve.dm.ModelRegistry.js
M modules/ve/test/dm/ve.dm.Converter.test.js
M modules/ve/test/dm/ve.dm.ModelRegistry.test.js
M modules/ve/test/dm/ve.dm.example.js
4 files changed, 91 insertions(+), 14 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor
refs/changes/59/58259/1
diff --git a/modules/ve/dm/ve.dm.ModelRegistry.js
b/modules/ve/dm/ve.dm.ModelRegistry.js
index 9ab0e0b..1647fbd 100644
--- a/modules/ve/dm/ve.dm.ModelRegistry.js
+++ b/modules/ve/dm/ve.dm.ModelRegistry.js
@@ -174,9 +174,8 @@
*/
ve.dm.ModelRegistry.prototype.matchElement = function ( element ) {
var i, name, model, matches, winner, types, elementExtSpecificTypes,
matchTypes,
+ hasExtSpecificTypes,
tag = element.nodeName.toLowerCase(),
- typeAttr = element.getAttribute( 'typeof' ) ||
element.getAttribute( 'rel' ) ||
- element.getAttribute( 'property' ),
reg = this;
function byRegistrationOrderDesc( a, b ) {
@@ -189,6 +188,7 @@
types = reg.registry[models[i]].static.matchRdfaTypes;
for ( j = 0; j < types.length; j++ ) {
if (
+ types[j] instanceof RegExp &&
type.match( types[j] ) &&
(
reg.registry[models[i]].static.matchTagNames === null ||
@@ -202,18 +202,51 @@
return matches;
}
- function matchWithFunc( types, tag ) {
+ function matchesAllTypes( types, name ) {
+ var i, j, haveMatch, matchTypes =
reg.registry[name].static.matchRdfaTypes;
+ for ( i = 0; i < types.length; i++ ) {
+ haveMatch = false;
+ for ( j = 0; j < matchTypes.length; j++ ) {
+ if ( matchTypes[j] instanceof RegExp ) {
+ if ( types[i].match( matchTypes[j] ) ) {
+ haveMatch = true;
+ break;
+ }
+ } else {
+ if ( types[i] === matchTypes[j] ) {
+ haveMatch = true;
+ break;
+ }
+ }
+ }
+ if ( !haveMatch ) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ function matchWithFunc( types, tag, mustMatchAll ) {
var i, queue = [], queue2 = [];
for ( i = 0; i < types.length; i++ ) {
// Queue string matches and regexp matches separately
queue = queue.concat( ve.getProp(
reg.modelsByTypeAndTag, 1, types[i], tag ) || [] );
queue2 = queue2.concat( matchTypeRegExps( types[i],
tag, true ) );
}
+ if ( mustMatchAll ) {
+ // Filter out matches that don't match all types
+ queue = ve.filterArray( queue, function ( name ) {
return matchesAllTypes( types, name ); } );
+ queue2 = ve.filterArray( queue2, function ( name ) {
return matchesAllTypes( types, name ); } );
+ }
// Try string matches first, then regexp matches
queue.sort( byRegistrationOrderDesc );
queue2.sort( byRegistrationOrderDesc );
queue = queue.concat( queue2 );
for ( i = 0; i < queue.length; i++ ) {
+ if ( mustMatchAll && !matchesAllTypes( types, queue[i]
) ) {
+ // Skip matches that don't match all types if
that's required
+ continue;
+ }
if ( reg.registry[queue[i]].static.matchFunction(
element ) ) {
return queue[i];
}
@@ -221,16 +254,25 @@
return null;
}
- function matchWithoutFunc( types, tag ) {
+ function matchWithoutFunc( types, tag, mustMatchAll ) {
var i, queue = [], queue2 = [], winningName = null;
for ( i = 0; i < types.length; i++ ) {
// Queue string and regexp matches separately
queue = queue.concat( ve.getProp(
reg.modelsByTypeAndTag, 0, types[i], tag ) || [] );
queue2 = queue2.concat( matchTypeRegExps( types[i],
tag, false ) );
}
+ if ( mustMatchAll ) {
+ // Filter out matches that don't match all types
+ queue = ve.filterArray( queue, function ( name ) {
return matchesAllTypes( types, name ); } );
+ queue2 = ve.filterArray( queue2, function ( name ) {
return matchesAllTypes( types, name ); } );
+ }
// Only try regexp matches if there are no string matches
queue = queue.length > 0 ? queue : queue2;
for ( i = 0; i < queue.length; i++ ) {
+ if ( mustMatchAll && !matchesAllTypes( types, queue[i]
) ) {
+ // Skip matches that don't match all types if
that's required
+ continue;
+ }
if (
winningName === null ||
reg.registrationOrder[winningName] <
reg.registrationOrder[queue[i]]
@@ -241,14 +283,24 @@
return winningName;
}
- types = typeAttr ? typeAttr.split( ' ' ) : [];
+ types = [];
+ if ( element.getAttribute( 'rel' ) ) {
+ types = types.concat( element.getAttribute( 'rel' ).split( ' '
) );
+ }
+ if ( element.getAttribute( 'typeof' ) ) {
+ types = types.concat( element.getAttribute( 'typeof' ).split( '
' ) );
+ }
+ if ( element.getAttribute( 'property' ) ) {
+ types = types.concat( element.getAttribute( 'property' ).split(
' ' ) );
+ }
elementExtSpecificTypes = ve.filterArray( types, ve.bind(
this.isExtensionSpecificType, this ) );
+ hasExtSpecificTypes = elementExtSpecificTypes.length !== 0;
// If the element has extension-specific types, only use those for
matching and ignore its
// other types. If it has no extension-specific types, use all of its
types.
- matchTypes = elementExtSpecificTypes.length === 0 ? types :
elementExtSpecificTypes;
+ matchTypes = hasExtSpecificTypes ? elementExtSpecificTypes : types;
if ( types.length ) {
// func+tag+type match
- winner = matchWithFunc( matchTypes, tag );
+ winner = matchWithFunc( matchTypes, tag, hasExtSpecificTypes );
if ( winner !== null ) {
return winner;
}
@@ -256,14 +308,14 @@
// func+type match
// Only look at rules with no tag specified; if a rule does
specify a tag, we've
// either already processed it above, or the tag doesn't match
- winner = matchWithFunc( matchTypes, '' );
+ winner = matchWithFunc( matchTypes, '', hasExtSpecificTypes );
if ( winner !== null ) {
return winner;
}
}
// Do not check for type-less matches if the element has
extension-specific types
- if ( elementExtSpecificTypes.length === 0 ) {
+ if ( !hasExtSpecificTypes ) {
// func+tag match
matches = ve.getProp( this.modelsByTag, 1, tag ) || [];
// No need to sort because individual arrays in modelsByTag are
already sorted
@@ -294,7 +346,7 @@
}
// tag+type
- winner = matchWithoutFunc( matchTypes, tag );
+ winner = matchWithoutFunc( matchTypes, tag, hasExtSpecificTypes );
if ( winner !== null ) {
return winner;
}
@@ -302,7 +354,7 @@
// type only
// Only look at rules with no tag specified; if a rule does specify a
tag, we've
// either already processed it above, or the tag doesn't match
- winner = matchWithoutFunc( matchTypes, '' );
+ winner = matchWithoutFunc( matchTypes, '', hasExtSpecificTypes );
if ( winner !== null ) {
return winner;
}
diff --git a/modules/ve/test/dm/ve.dm.Converter.test.js
b/modules/ve/test/dm/ve.dm.Converter.test.js
index 66f968e..92d16fd 100644
--- a/modules/ve/test/dm/ve.dm.Converter.test.js
+++ b/modules/ve/test/dm/ve.dm.Converter.test.js
@@ -38,7 +38,7 @@
}
} );
-QUnit.test( 'getDataFromDom', 50, function ( assert ) {
+QUnit.test( 'getDataFromDom', 51, function ( assert ) {
var msg,
store = new ve.dm.IndexValueStore(),
cases = ve.copyObject( ve.dm.example.domToDataCases );
@@ -60,7 +60,7 @@
}
} );
-QUnit.test( 'getDomFromData', 54, function ( assert ) {
+QUnit.test( 'getDomFromData', 55, function ( assert ) {
var msg,
store = new ve.dm.IndexValueStore(),
cases = ve.copyObject( ve.dm.example.domToDataCases );
diff --git a/modules/ve/test/dm/ve.dm.ModelRegistry.test.js
b/modules/ve/test/dm/ve.dm.ModelRegistry.test.js
index 949fe2a..f1cd6d8 100644
--- a/modules/ve/test/dm/ve.dm.ModelRegistry.test.js
+++ b/modules/ve/test/dm/ve.dm.ModelRegistry.test.js
@@ -101,7 +101,7 @@
/* Tests */
-QUnit.test( 'matchElement', 20, function ( assert ) {
+QUnit.test( 'matchElement', 23, function ( assert ) {
var registry = new ve.dm.ModelRegistry(), element;
element = document.createElement( 'a' );
assert.deepEqual( registry.matchElement( element ), null,
'matchElement() returns null if registry empty' );
@@ -163,4 +163,11 @@
assert.deepEqual( registry.matchElement( element ), 'stub-regexp',
'RegExp type match for extension-specific type' );
element.setAttribute( 'rel', 'mw:abbr' );
assert.deepEqual( registry.matchElement( element ), 'stub-abbr',
'String match overrides RegExp match for extension-specific type' );
+ element.setAttribute( 'rel', 'mw:abbr mw:foo' );
+ assert.deepEqual( registry.matchElement( element ), 'stub-regexp',
'Additional extension-specific type (mw:foo) breaks string match, throws back
to regexp match' );
+ element.setAttribute( 'rel', 'mw:abbr foo' );
+ assert.deepEqual( registry.matchElement( element ), null, 'Additional
extension-specific type (foo) breaks match' );
+ element.setAttribute( 'rel', 'mw:abbr' );
+ element.setAttribute( 'typeof', 'foo' );
+ assert.deepEqual( registry.matchElement( element ), null, 'Types split
over two attributes still breaks match' );
} );
diff --git a/modules/ve/test/dm/ve.dm.example.js
b/modules/ve/test/dm/ve.dm.example.js
index 3395486..74ed18b 100644
--- a/modules/ve/test/dm/ve.dm.example.js
+++ b/modules/ve/test/dm/ve.dm.example.js
@@ -1815,6 +1815,24 @@
'<meta typeof="mw:Placeholder" data-parsoid="foobar"
/></body>',
'data': ve.dm.example.withMeta
},
+ 'RDFa types spread across two attributes': {
+ 'html': '<body><link rel="mw:WikiLink/Category"
href="./Category:Foo" about="#mwt1" typeof="mw:Object/Template"></body>',
+ 'data': [
+ {
+ 'type': 'alienMeta',
+ 'attributes': {
+ 'style': 'link',
+ 'key': 'mw:WikiLink/Category',
+ 'value': './Category:Foo',
+ 'html/0/rel': 'mw:WikiLink/Category',
+ 'html/0/href': './Category:Foo',
+ 'html/0/about': '#mwt1',
+ 'html/0/typeof': 'mw:Object/Template'
+ }
+ },
+ { 'type': '/alienMeta' },
+ ]
+ },
'change markers': {
'html': null,
'data': [
--
To view, visit https://gerrit.wikimedia.org/r/58259
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iacf3851a0ca9081d2c813b42435484a47cec6230
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
