GWicke has uploaded a new change for review.
https://gerrit.wikimedia.org/r/66348
Change subject: Simplify and document the DOMTraverser return value protocol
......................................................................
Simplify and document the DOMTraverser return value protocol
Simplify the handler return value protocol to use only two cases, and document
the expectations.
Handlers can return
* the next node to process
* aborts processing for current node
* can also be null, so .nextSibling works even on lastChild
* other falsy values / no return
* continue processing for current node
Rationale:
* Calling nextSibling in a handler to return the next node can return null
when the last child is replaced. In any case, processing for the current
node should be terminated. Handling null as equivalent to a node in the
traverser keeps the handler code simple and reduces the probability of bugs
there.
* For the 'continue processing' case, we could require a specific falsy value
or true if we wanted, but for now accepting undefined looks like a simple
solution for handlers. Requiring handlers to return false triggers the wrong
intuition as this normally implies 'abort'. Requiring a true return instead
looks like a strong alternative worth considering.
Change-Id: I3028aa53e18cdd37e8fe6b7babd62303cb62ecf3
---
M js/lib/mediawiki.DOMPostProcessor.js
1 file changed, 22 insertions(+), 27 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Parsoid
refs/changes/48/66348/1
diff --git a/js/lib/mediawiki.DOMPostProcessor.js
b/js/lib/mediawiki.DOMPostProcessor.js
index ae382b5..622322b 100644
--- a/js/lib/mediawiki.DOMPostProcessor.js
+++ b/js/lib/mediawiki.DOMPostProcessor.js
@@ -115,7 +115,7 @@
if ( this.handlers[ix].name === null ||
this.handlers[ix].name === name ) {
result = this.handlers[ix].run( node );
- if ( result || result === false ) {
+ if ( result || result === null ) {
// abort processing for this node
return result;
}
@@ -125,6 +125,10 @@
/**
* Traverse the DOM and fire the handlers that are registered
+ *
+ * Handlers can return
+ * - the next node to process (can also be null, aborts processing for current
node)
+ * - other falsy values / no return (continue processing for current node)
*/
DOMTraverser.prototype.traverse = function ( node ) {
if (node.nodeType === node.DOCUMENT_NODE) {
@@ -139,34 +143,24 @@
nextChild = child.nextSibling;
result = this.callHandlers( child );
- // Handlers can return
- // - the next node to process (aborts processing for current
node)
- // - false for the next node (aborts processing for current
node)
- // - undefined / no return (continue processing for current
node)
- if ( result ) {
- // Continue to work on the returned node
+ if ( result || result === null ) {
+ // Continue to work on the returned node, if any
child = result;
} else {
- if ( result === undefined ) {
- // handle children
- if (child.parentNode === null) {
- // TODO gwicke: Throw an exception and
pinpoint the faulty
- // handler.
-
console.error('DOMPostProcessor.traverse: null parentNode! ' +
- 'Bug in handlers on ' +
child.outerHTML);
- child = nextChild;
- } else {
- if ( DU.isElt(child) &&
- child.childNodes.length
> 0 )
- {
- this.traverse( child );
- }
- child = child.nextSibling;
- }
- } else {
- // Move on to the next child, as determined
before running
- // handlers.
+ // handle children
+ if (child.parentNode === null) {
+ // TODO gwicke: Throw an exception and pinpoint
the faulty
+ // handler.
+ console.error('DOMPostProcessor.traverse: null
parentNode! ' +
+ 'Bug in handlers on ' +
child.outerHTML);
child = nextChild;
+ } else {
+ if ( DU.isElt(child) &&
+ child.childNodes.length > 0 )
+ {
+ this.traverse( child );
+ }
+ child = child.nextSibling;
}
}
}
@@ -2377,9 +2371,10 @@
metaType.match(/\bmw:(StartTag|EndTag|Extension\/(?:ref|references)\/Marker|TSRMarker)\/?[^\s]*\b/)
&&
!node.getAttribute("property"))
{
+ var nextNode = node.nextSibling;
deleteNode(node);
// stop the traversal, since this node is no longer in the DOM.
- return false;
+ return nextNode;
}
}
--
To view, visit https://gerrit.wikimedia.org/r/66348
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3028aa53e18cdd37e8fe6b7babd62303cb62ecf3
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Parsoid
Gerrit-Branch: master
Gerrit-Owner: GWicke <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
