Catrope has uploaded a new change for review.
https://gerrit.wikimedia.org/r/70109
Change subject: Preserve data-mw verbatim if unchanged
......................................................................
Preserve data-mw verbatim if unchanged
Previously we would reserialize the JSON blob in data-mw even if we
didn't change it, which potentially reordered keys and caused a DOM
diff.
Bug: 50066
Change-Id: If0a5bcc67d3a172de0e8839cfda11efacfbf36ff
---
M modules/ve/dm/nodes/ve.dm.MWReferenceListNode.js
M modules/ve/dm/nodes/ve.dm.MWReferenceNode.js
M modules/ve/dm/nodes/ve.dm.MWTransclusionNode.js
M modules/ve/test/dm/ve.dm.example.js
4 files changed, 45 insertions(+), 9 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor
refs/changes/09/70109/1
diff --git a/modules/ve/dm/nodes/ve.dm.MWReferenceListNode.js
b/modules/ve/dm/nodes/ve.dm.MWReferenceListNode.js
index 2e3a82c..33d03e1 100644
--- a/modules/ve/dm/nodes/ve.dm.MWReferenceListNode.js
+++ b/modules/ve/dm/nodes/ve.dm.MWReferenceListNode.js
@@ -43,6 +43,7 @@
'type': this.name,
'attributes': {
'mw': mwData,
+ 'origMw': mwDataJSON,
'about': domElements[0].getAttribute( 'about' ),
'domElements': ve.copyArray( domElements ),
'refGroup': refGroup,
@@ -78,7 +79,14 @@
el.setAttribute( 'about', attribs.about );
}
el.setAttribute( 'typeof', 'mw:Extension/references' );
- el.setAttribute( 'data-mw', JSON.stringify( mwData ) );
+
+ // If mwAttr and origMw are the same, use origMw to prevent
reserialization.
+ // Reserialization has the potential to reorder keys and so change the
DOM unnecessarily
+ if ( ve.compare( mwData, JSON.parse( attribs.origMw ) ) ) {
+ el.setAttribute( 'data-mw', attribs.origMw );
+ } else {
+ el.setAttribute( 'data-mw', JSON.stringify( mwData ) );
+ }
return els;
};
diff --git a/modules/ve/dm/nodes/ve.dm.MWReferenceNode.js
b/modules/ve/dm/nodes/ve.dm.MWReferenceNode.js
index 36a25af..97ac4bf 100644
--- a/modules/ve/dm/nodes/ve.dm.MWReferenceNode.js
+++ b/modules/ve/dm/nodes/ve.dm.MWReferenceNode.js
@@ -56,6 +56,7 @@
'type': this.name,
'attributes': {
'mw': mwData,
+ 'origMw': mwDataJSON,
'about': about,
'listIndex': listIndex,
'listGroup': listGroup,
@@ -124,7 +125,13 @@
delete mwAttr.attrs.refGroup;
}
- span.setAttribute( 'data-mw', JSON.stringify( mwAttr ) );
+ // If mwAttr and origMw are the same, use origMw to prevent
reserialization.
+ // Reserialization has the potential to reorder keys and so change the
DOM unnecessarily
+ if ( ve.compare( mwAttr, JSON.parse( dataElement.attributes.origMw ) )
) {
+ span.setAttribute( 'data-mw', dataElement.attributes.origMw );
+ } else {
+ span.setAttribute( 'data-mw', JSON.stringify( mwAttr ) );
+ }
return [ span ];
};
diff --git a/modules/ve/dm/nodes/ve.dm.MWTransclusionNode.js
b/modules/ve/dm/nodes/ve.dm.MWTransclusionNode.js
index 5661320..f8574f7 100644
--- a/modules/ve/dm/nodes/ve.dm.MWTransclusionNode.js
+++ b/modules/ve/dm/nodes/ve.dm.MWTransclusionNode.js
@@ -60,14 +60,16 @@
ve.dm.MWTransclusionNode.static.toDataElement = function ( domElements,
converter ) {
var dataElement, index,
- mw = JSON.parse( domElements[0].getAttribute( 'data-mw' ) ),
+ mwDataJSON = domElements[0].getAttribute( 'data-mw' ),
+ mwData = mwDataJSON ? JSON.parse( mwDataJSON ) : {},
isInline = this.isHybridInline( domElements, converter ),
type = isInline ? 'mwTransclusionInline' :
'mwTransclusionBlock';
dataElement = {
'type': type,
'attributes': {
- 'mw': mw
+ 'mw': mwData,
+ 'origMw': mwDataJSON
}
};
@@ -79,13 +81,17 @@
ve.dm.MWTransclusionNode.static.toDomElements = function ( dataElement, doc,
converter ) {
var span,
- index = converter.getStore().indexOfHash( ve.getHash(
this.getHashObject( dataElement ) ) );
+ index = converter.getStore().indexOfHash( ve.getHash(
this.getHashObject( dataElement ) ) ),
+ origMwData = JSON.parse( dataElement.attributes.origMw );
// If the transclusion is unchanged just send back the
// original DOM elements so selser can skip over it
- if ( index === dataElement.attributes.originalIndex ) {
+ if (
+ index === dataElement.attributes.originalIndex ||
+ ve.compare( dataElement.attributes.mw, origMwData )
+ ) {
// The object in the store is also used for CE rendering so
return a copy
- return ve.copyArray( converter.getStore().value( index ) );
+ return ve.copyDomElements( converter.getStore().value( index ),
doc );
} else {
span = doc.createElement( 'span' );
// All we need to send back to Parsoid is the original
transclusion marker, with a
diff --git a/modules/ve/test/dm/ve.dm.example.js
b/modules/ve/test/dm/ve.dm.example.js
index 83278d1..e5c30c7 100644
--- a/modules/ve/test/dm/ve.dm.example.js
+++ b/modules/ve/test/dm/ve.dm.example.js
@@ -857,7 +857,7 @@
ve.dm.example.MWInlineImageHtml = '<span typeof="mw:Image"
data-parsoid="{"tsr":[0,24],"optList":[{"ck":"width","ak":"500px"}],"cacheKey":"[[Image:Wiki.png|500px]]","img":{"h":155,"w":135,"wdset":true},"dsr":[0,24,null,null]}"><a
href="./File:Wiki.png"
data-parsoid="{"a":{"href":"./File:Wiki.png"}}"><img
resource="./File:Wiki.png"
src="http://upload.wikimedia.org/wikipedia/en/b/bc/Wiki.png"; height="155"
width="135"
data-parsoid="{"a":{"resource":"./File:Wiki.png","width":"135"},"sa":{"resource":"Image:Wiki.png","width":"500"}}"></a></span>';
ve.dm.example.MWTransclusion = {
- 'blockSpan': '<span about="#mwt1" typeof="mw:Transclusion"
data-mw="{"id":"mwt1","target":{"wt":"Test"},"params":{"1":{"wt":"Hello,
world!"}}}"
data-parsoid="{"tsr":[18,40],"src":"{{Test|Hello,
world!}}","dsr":[18,40,null,null]}"></span>',
+ 'blockSpan': '<span about="#mwt1" typeof="mw:Transclusion"
data-mw="{"target":{"wt":"Test"},"params":{"1":{"wt":"Hello,
world!"}},"id":"mwt1"}"
data-parsoid="{"tsr":[18,40],"src":"{{Test|Hello,
world!}}","dsr":[18,40,null,null]}"></span>',
'blockSpanModified': '<span about="#mwt1" typeof="mw:Transclusion"
data-mw="{"id":"mwt1","target":{"wt":"Test"},"params":{"1":{"wt":"Hello,
globe!"}}}"
data-parsoid="{"tsr":[18,40],"src":"{{Test|Hello,
world!}}","dsr":[18,40,null,null]}"></span>',
'blockContent': '<p about="#mwt1" data-parsoid="{}">Hello, world!</p>',
'blockData': {
@@ -870,12 +870,13 @@
'1': { 'wt': 'Hello, world!' }
}
},
+ 'origMw':
'{\"target\":{\"wt\":\"Test\"},\"params\":{\"1\":{\"wt\":\"Hello,
world!\"}},\"id\":\"mwt1\"}',
'originalIndex': 0
},
'htmlAttributes': [
{ 'values': {
'about': '#mwt1',
- 'data-mw':
'{\"id\":\"mwt1\",\"target\":{\"wt\":\"Test\"},\"params\":{\"1\":{\"wt\":\"Hello,
world!\"}}}',
+ 'data-mw':
'{\"target\":{\"wt\":\"Test\"},\"params\":{\"1\":{\"wt\":\"Hello,
world!\"}},\"id\":\"mwt1\"}',
'data-parsoid':
'{\"tsr\":[18,40],\"src\":\"{{Test|Hello,
world!}}\",\"dsr\":[18,40,null,null]}',
'typeof': 'mw:Transclusion'
} },
@@ -899,6 +900,7 @@
'1': { 'wt': '1,234' }
}
},
+ 'origMw':
'{\"id\":\"mwt1\",\"target\":{\"wt\":\"Inline\"},\"params\":{\"1\":{\"wt\":\"1,234\"}}}',
'originalIndex': 0
},
'htmlAttributes': [ { 'values': {
@@ -919,6 +921,7 @@
'1': { 'wt': '5,678' }
}
},
+ 'origMw':
'{\"id\":\"mwt1\",\"target\":{\"wt\":\"Inline\"},\"params\":{\"1\":{\"wt\":\"5,678\"}}}',
'originalIndex': 0
},
'htmlAttributes': [
@@ -1148,6 +1151,7 @@
'listKey': 'bar',
'refGroup': '',
'mw': { 'body': { 'html': '' },
'attrs': { 'name': 'bar' } },
+ 'origMw':
'{"body":{"html":""},"attrs":{"name":"bar"}}',
'contentsUsed': false
},
'htmlAttributes': [
@@ -1182,6 +1186,7 @@
'listKey': 'quux',
'refGroup': '',
'mw': { 'body': { 'html': 'Quux' },
'attrs': { 'name': 'quux' } },
+ 'origMw':
'{"body":{"html":"Quux"},"attrs":{"name":"quux"}}',
'contentsUsed': true
},
'htmlAttributes': [
@@ -1216,6 +1221,7 @@
'listKey': 'bar',
'refGroup': '',
'mw': { 'body': { 'html': 'Bar' },
'attrs': { 'name': 'bar' } },
+ 'origMw':
'{"body":{"html":"Bar"},"attrs":{"name":"bar"}}',
'contentsUsed': true
},
'htmlAttributes': [
@@ -1250,6 +1256,7 @@
'listKey': null,
'refGroup': 'g1',
'mw': { 'body': { 'html': 'No name' },
'attrs': { 'group': 'g1' } },
+ 'origMw': '{"body":{"html":"No
name"},"attrs":{"group":"g1"}}',
'contentsUsed': true
},
'htmlAttributes': [
@@ -1283,6 +1290,7 @@
'name': 'references',
'attrs': {}
},
+ 'origMw':
'{"name":"references","attrs":{}}',
'domElements': $(
'<ol class="references"
about="#mwt12" typeof="mw:Extension/references" '+
'data-mw="{"name":"references","attrs":{}}" ' +
@@ -3449,6 +3457,7 @@
'body': { 'html': 'No name 1' },
'name': 'ref'
},
+ 'origMw': '{"name":"ref","body":{"html":"No name
1"},"attrs":{}}',
'refGroup': ''
},
'htmlAttributes': [ { 'values': {
@@ -3478,6 +3487,7 @@
'body': { 'html': 'Bar' },
'name': 'ref'
},
+ 'origMw': '{"body":{"html":""},"attrs":{"name":"bar"}}',
'refGroup': ''
},
'htmlAttributes': [ { 'values': {
@@ -3505,6 +3515,7 @@
'body': { 'html': 'Quux' },
'name': 'ref'
},
+ 'origMw':
'{"name":"ref","body":{"html":"Quux"},"attrs":{"name":"quux"}}',
'refGroup': ''
},
'htmlAttributes': [ { 'values': {
@@ -3531,6 +3542,7 @@
'attrs': { 'name': 'bar' },
'name': 'ref'
},
+ 'origMw': '{"body":{"html":""},"attrs":{"name":"bar"}}',
'refGroup': ''
},
'htmlAttributes': [ { 'values': {
@@ -3560,6 +3572,7 @@
'body': { 'html': 'No name 2' },
'name': 'ref'
},
+ 'origMw': '{"name":"ref","body":{"html":"No name
2"},"attrs":{}}',
'refGroup': ''
},
'htmlAttributes': [ { 'values': {
@@ -3586,6 +3599,7 @@
'body': { 'html': 'No name 3' },
'name': 'ref'
},
+ 'origMw': '{"name":"ref","body":{"html":"No name
3"},"attrs":{}}',
'refGroup': ''
},
'htmlAttributes': [ { 'values': {
@@ -3608,6 +3622,7 @@
'name': 'references',
'attrs': {}
},
+ 'origMw': '{"name":"references","attrs":{}"}',
//'domElements': HTML,
'listGroup': 'mwReference/',
'refGroup': ''
--
To view, visit https://gerrit.wikimedia.org/r/70109
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If0a5bcc67d3a172de0e8839cfda11efacfbf36ff
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
![http://upload.wikimedia.org/wikipedia/en/b/bc/Wiki.png"](https://upload.wikimedia.org/wikipedia/en/b/bc/Wiki.png"){kind=link}