Subramanya Sastry has uploaded a new change for review. https://gerrit.wikimedia.org/r/179787
Change subject: Fix test that checks if a DOM node is original or new content ...................................................................... Fix test that checks if a DOM node is original or new content * The existing test was looking at the presence / absence of the data-parsoid attribute. However, once we stripped data-parsoid from template content in 320caa30, the original test was no longer sufficient. This caused false roundtrip diffs in roundtrip-testing. Try "node roundtrip-test.js --prefix kowiki 민아" vs. looking at http://localhost:8000/_rt/kowiki/민아 The former runs in roundtrip-test mode (no-edits) and the latter runs in edit mode. In roundtrip-test mode, a mw:Placeholder/StrippedTag meta without a data-parsoid attribute was changing the newline constraints since the meta was being treated as new content without the data-parsoid attribute. * While we should fix the newline constraints code separately to look at the meta typeof, this patch fixes the isNewElt fix to be more robust -- it now tests the newness of the first top-level node of templated/extension content (which doesn't gets its data-parsoid stripped). * This fixes the roundtrip-test.js output on the kowiki page above. Change-Id: Ibaf3f544f34d16b3f8ae76bc1b2a4e563e438988 --- M lib/mediawiki.DOMUtils.js 1 file changed, 22 insertions(+), 8 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/parsoid refs/changes/87/179787/1 diff --git a/lib/mediawiki.DOMUtils.js b/lib/mediawiki.DOMUtils.js index 578ea81..167c8ef 100644 --- a/lib/mediawiki.DOMUtils.js +++ b/lib/mediawiki.DOMUtils.js @@ -636,8 +636,21 @@ return dp.stx === 'html'; }, - isNewElt: function(n) { - return n.getAttribute('data-parsoid') === null; + /** + * This tests whether a DOM node is a new node added during an edit session + * or an existing node from parsed wikitext. + * + * As written, this function can only be used on non-template/extension content + * or on the top-level nodes of template/extension content. This test will + * return the wrong results on non-top-level nodes of template/extension content. + * + * @param {Node} node + */ + isNewElt: function(node) { + // For template/extension content, newness should be + // checked on the encapsulation wrapper node. + node = this.findFirstEncapsulationWrapperNode(node) || node; + return node.getAttribute('data-parsoid') === null; }, /** @@ -793,8 +806,10 @@ * @param {Node} node */ isTplOrExtToplevelNode: function(node) { - if (isElt(node)) { + if (this.isElt(node)) { var about = node.getAttribute('about'); + // SSS FIXME: Verify that our DOM spec clarifies this + // expectation on about-ids and that our clients respect this. return about && Util.isParsoidObjectId(about); } else { return false; @@ -1339,15 +1354,14 @@ * Find the first wrapper element of encapsulated content. */ findFirstEncapsulationWrapperNode: function ( node ) { - var about = node.getAttribute('about'); - if ( !about ) { + if (!this.isTplOrExtToplevelNode(node)) { return null; } + var about = node.getAttribute('about'); while ( !this.isFirstEncapsulationWrapperNode( node ) ) { - node = node.previousSibling; - if ( !node || !isElt(node) || - node.getAttribute('about') !== about ) { + node = this.previousNonDeletedSibling(node); + if ( !node || !isElt(node) || node.getAttribute('about') !== about ) { return null; } } -- To view, visit https://gerrit.wikimedia.org/r/179787 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ibaf3f544f34d16b3f8ae76bc1b2a4e563e438988 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/services/parsoid Gerrit-Branch: master Gerrit-Owner: Subramanya Sastry <[email protected]> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
