jenkins-bot has submitted this change and it was merged. Change subject: Bug 53071: Handle about-less images better ......................................................................
Bug 53071: Handle about-less images better * Only transfer about if there is a non-null about attribute in unpackDOMFragments * Don't consume siblings if there is no about in getAboutSiblings I am not 100% sure that this is indeed the root cause for bug 53071. A scenario I can think of is this: 1) getAboutSiblings returned sibling nodes without about attributes 2) for some reason that is unclear to me those were *not* assigned about="null" attributes in unpackDOMFragments on reuse 3) the serializer faithfully serialized out the new content The diffs in https://it.wikipedia.org/w/index.php?title=Trieste&diff=prev&oldid=61046913 would support this, as the entire image link is duplicated. The diff in https://it.wikipedia.org/w/index.php?title=Dalmine&oldid=61238456 however looks like an incomplete image duplication. There are several figure-related DSR errors when parsing that page of this pattern: WARNING: DSR inconsistency: cs/s mismatch for node: FIGURE s: 22704; cs: 22817 WARNING: DSR inconsistency: cs/s mismatch for node: FIGURE s: 18478; cs: 18586 Those warnings are still there when parsing the page with fragment reuse enabled, so this might be a different bug. Change-Id: I5a273aa39904230cae8500e5ada78c5e630e443d --- M js/lib/dom.t.unpackDOMFragments.js M js/lib/mediawiki.DOMUtils.js 2 files changed, 11 insertions(+), 3 deletions(-) Approvals: Subramanya Sastry: Looks good to me, approved jenkins-bot: Verified diff --git a/js/lib/dom.t.unpackDOMFragments.js b/js/lib/dom.t.unpackDOMFragments.js index 2c3332b..a88a924 100644 --- a/js/lib/dom.t.unpackDOMFragments.js +++ b/js/lib/dom.t.unpackDOMFragments.js @@ -83,8 +83,8 @@ // get rid of the wrapper sibling (simplifies logic below) var sibling = node.nextSibling; - if (sibling && DU.isElt(sibling) && - sibling.getAttribute('about') === node.getAttribute('about')) + if (about !== null && sibling && DU.isElt(sibling) && + sibling.getAttribute('about') === about) { // remove optional second element added by wrapper tokens lastNode = sibling; @@ -128,7 +128,9 @@ while (firstChild) { // Transfer the about attribute so that it is still unique in // the page - firstChild.setAttribute('about', about); + if (about !== null) { + firstChild.setAttribute('about', about); + } // Load data-parsoid for all children DU.loadDataParsoid(firstChild); parentNode.insertBefore(firstChild, node); diff --git a/js/lib/mediawiki.DOMUtils.js b/js/lib/mediawiki.DOMUtils.js index ba4917e..0a930b6 100644 --- a/js/lib/mediawiki.DOMUtils.js +++ b/js/lib/mediawiki.DOMUtils.js @@ -944,6 +944,10 @@ getAboutSiblings: function(node, about) { var nodes = [node]; + if (!about) { + return nodes; + } + node = node.nextSibling; while (node && ( this.isElt(node) && node.getAttribute('about') === about || @@ -1034,6 +1038,7 @@ { DU.loadDataParsoid(node); nodes = DU.getAboutSiblings(node, about); + var key; if (/(?:^|\s)mw:Transclusion(?=$|\s)/.test(typeOf)) { expAccum = expansions.transclusions; @@ -1048,6 +1053,7 @@ // transclusion output. key = node.data.parsoid.cacheKey; } + //console.log(key); if (key) { expAccum[key] = { -- To view, visit https://gerrit.wikimedia.org/r/82676 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I5a273aa39904230cae8500e5ada78c5e630e443d Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Parsoid Gerrit-Branch: master Gerrit-Owner: GWicke <[email protected]> Gerrit-Reviewer: Subramanya Sastry <[email protected]> Gerrit-Reviewer: jenkins-bot _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
