Catrope has uploaded a new change for review.
https://gerrit.wikimedia.org/r/59836
Change subject: Remove html/* attributes in getClonedElement()
......................................................................
Remove html/* attributes in getClonedElement()
Subbu said that cloning of attributes like data-parsoid or typeof would
cause problems for Parsoid.
Also remove the attributes object if it becomes empty, and do the same
for the internal object.
Bug: 47297
Change-Id: I428becf95c70d0ed8af5b0c408e3966dc47fd8c3
---
M modules/ve/dm/ve.dm.Node.js
M modules/ve/test/dm/ve.dm.Node.test.js
2 files changed, 162 insertions(+), 2 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor
refs/changes/36/59836/1
diff --git a/modules/ve/dm/ve.dm.Node.js b/modules/ve/dm/ve.dm.Node.js
index dd7a766..ad80a28 100644
--- a/modules/ve/dm/ve.dm.Node.js
+++ b/modules/ve/dm/ve.dm.Node.js
@@ -209,14 +209,27 @@
* Get a clone of the node's document data element.
*
* The attributes object will be deep-copied, and the .internal.generated
property will be removed,
- * if present.
+ * if present. Any html/* attributes will be removed as well.
*
* @returns {Object} Cloned element object
*/
ve.dm.Node.prototype.getClonedElement = function () {
- var clone = ve.copyObject( this.element );
+ var attr, clone = ve.copyObject( this.element );
if ( clone.internal ) {
delete clone.internal.generated;
+ if ( ve.isEmptyObject( clone.internal ) ) {
+ delete clone.internal;
+ }
+ }
+ if ( this.element.attributes ) {
+ for ( attr in this.element.attributes ) {
+ if ( attr.substr( 0, 5 ) === 'html/' ) {
+ delete clone.attributes[attr];
+ }
+ }
+ }
+ if ( ve.isEmptyObject( clone.attributes ) ) {
+ delete clone.attributes;
}
return clone;
};
diff --git a/modules/ve/test/dm/ve.dm.Node.test.js
b/modules/ve/test/dm/ve.dm.Node.test.js
index ce2e204..bb2129e 100644
--- a/modules/ve/test/dm/ve.dm.Node.test.js
+++ b/modules/ve/test/dm/ve.dm.Node.test.js
@@ -110,3 +110,150 @@
assert.strictEqual( node2.canBeMergedWith( node1 ), false, 'different
level, different type' );
assert.strictEqual( node2.canBeMergedWith( node4 ), false, 'same level,
different type' );
} );
+
+QUnit.test( 'getClonedElement', function ( assert ) {
+ var i, node,
+ cases = [
+ {
+ 'original': {
+ 'type': 'foo'
+ },
+ 'clone': {
+ 'type': 'foo'
+ },
+ 'msg': 'Simple element is cloned verbatim'
+ },
+ {
+ 'original': {
+ 'type': 'foo',
+ 'attributes': {
+ 'bar': 'baz'
+ }
+ },
+ 'clone': {
+ 'type': 'foo',
+ 'attributes': {
+ 'bar': 'baz'
+ }
+ },
+ 'msg': 'Element with simple attributes is
cloned verbatim'
+ },
+ {
+ 'original': {
+ 'type': 'foo',
+ 'attributes': {
+ 'bar': 'baz',
+ 'html/0/typeof': 'Foo',
+ 'html/0/href': 'Bar'
+ }
+ },
+ 'clone': {
+ 'type': 'foo',
+ 'attributes': {
+ 'bar': 'baz'
+ }
+ },
+ 'msg': 'html/* attributes are removed from
clone'
+ },
+ {
+ 'original': {
+ 'type': 'foo',
+ 'attributes': {
+ 'html/0/typeof': 'Foo'
+ }
+ },
+ 'clone': {
+ 'type': 'foo'
+ },
+ 'msg': 'attributes object containing only
html/* attributes is removed from clone'
+ },
+ {
+ 'original': {
+ 'type': 'foo',
+ 'attributes': {
+ 'html': '<foo></foo>'
+ }
+ },
+ 'clone': {
+ 'type': 'foo',
+ 'attributes': {
+ 'html': '<foo></foo>'
+ }
+ },
+ 'msg': 'html attribute (without slash) is not
removed'
+ },
+ {
+ 'original': {
+ 'type': 'foo',
+ 'internal': {
+ 'generated': 'wrapper',
+ 'whitespace': [ undefined, ' ' ]
+ }
+ },
+ 'clone': {
+ 'type': 'foo',
+ 'internal': {
+ 'whitespace': [ undefined, ' ' ]
+ }
+ },
+ 'msg': 'internal.generated property is removed
from clone'
+ },
+ {
+ 'original': {
+ 'type': 'foo',
+ 'internal': {
+ 'generated': 'wrapper'
+ }
+ },
+ 'clone': {
+ 'type': 'foo'
+ },
+ 'msg': 'internal property is removed if it only
contained .generated'
+ },
+ {
+ 'original': {
+ 'type': 'foo',
+ 'internal': {
+ 'generated': 'wrapper'
+ },
+ 'attributes': {
+ 'html/0/typeof': 'Foo',
+ 'html/1/href': 'Bar'
+ }
+ },
+ 'clone': {
+ 'type': 'foo'
+ },
+ 'msg': 'internal and attribute properties are
both removed'
+ },
+ {
+ 'original': {
+ 'type': 'foo',
+ 'internal': {
+ 'generated': 'wrapper',
+ 'whitespace': [ undefined, ' '
],
+ },
+ 'attributes': {
+ 'html/0/typeof': 'Foo',
+ 'bar': 'baz'
+ }
+ },
+ 'clone': {
+ 'type': 'foo',
+ 'internal': {
+ 'whitespace': [ undefined, ' '
],
+ },
+ 'attributes': {
+ 'bar': 'baz'
+ }
+ },
+ 'msg': 'internal.generated and
attributes.html/* are both removed'
+ }
+ ];
+ QUnit.expect( cases.length );
+
+ for ( i = 0; i < cases.length; i++ ) {
+ node = new ve.dm.NodeStub( 0, cases[i].original );
+ assert.deepEqual( node.getClonedElement(), cases[i].clone,
cases[i].msg );
+ }
+} );
--
To view, visit https://gerrit.wikimedia.org/r/59836
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I428becf95c70d0ed8af5b0c408e3966dc47fd8c3
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
