Catrope has uploaded a new change for review.
https://gerrit.wikimedia.org/r/118785
Change subject: Make ContentBranchNode echo suppression work in CBNs that
contain inline nodes
......................................................................
Make ContentBranchNode echo suppression work in CBNs that contain inline nodes
getRenderedContents() would give you the reference-identical original DOM nodes
for inline nodes, which is good because that way the CE nodes retain their
this.$element references to them, but also bad because attaching these nodes
to the temporary wrapper detaches them from the view. With the existing
order of operations, this caused the comparsion to fail and a rerender
to happen, so we just always rerendered CBNs with inline nodes, but a
slightly different order of operations would have made the inline
nodes disappear instead.
Make getRenderedContents() return clones of these nodes, but with a
property pointing back to original. This property is not taken into
account by .isEqualDomNode(), so do the comparison with the clone,
and if we decide to rerender, resolve these clones back to their
originals before rendering into the DOM.
Text replacements in CBNs containing inline nodes (e.g. aliens demo)
now no longer echo changes; without this commit they always echo.
Replacing a character adjacent to an inline node stil echoes, but that's
a separate issue that's also causing replacing the first character in
any CBN to echo.
Change-Id: Idd75cae3c9555f5aca4568666db203b3a3bc98cb
---
M modules/ve/ce/ve.ce.ContentBranchNode.js
1 file changed, 46 insertions(+), 11 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor
refs/changes/85/118785/1
diff --git a/modules/ve/ce/ve.ce.ContentBranchNode.js
b/modules/ve/ce/ve.ce.ContentBranchNode.js
index 95dcafe..5479912 100644
--- a/modules/ve/ce/ve.ce.ContentBranchNode.js
+++ b/modules/ve/ce/ve.ce.ContentBranchNode.js
@@ -44,6 +44,35 @@
*/
ve.ce.ContentBranchNode.static.splitOnEnter = true;
+/* Static Methods */
+
+/**
+ * Append the return value of #getRenderedContents to a DOM element.
+ *
+ * @param {HTMLElement} container DOM element
+ * @param {HTMLElement} wrapper Wrapper returned by #getRenderedContents
+ */
+ve.ce.ContentBranchNode.static.appendRenderedContents = function ( container,
wrapper ) {
+ function resolveOriginals( domElement ) {
+ var i, len, child;
+ for ( i = 0, len = domElement.childNodes.length; i < len; i++ )
{
+ child = domElement.childNodes[i];
+ if ( child.veOrigNode ) {
+ domElement.replaceChild( child.veOrigNode,
child );
+ } else if ( child.childNodes && child.childNodes.length
) {
+ resolveOriginals( child );
+ }
+ }
+ }
+
+ var i, len;
+ /* Resolve references to the original nodes. */
+ resolveOriginals( wrapper );
+ while ( wrapper.firstChild ) {
+ container.appendChild( wrapper.firstChild );
+ }
+};
+
/* Methods */
/**
@@ -91,11 +120,15 @@
/**
* Get an HTML rendering of the contents.
*
+ * If you are actually going to append the result to a DOM, you need to
+ * do this with #appendRenderedContents, which resolves the cloned
+ * nodes returned by this function back to their originals.
+ *
* @method
- * @returns {HTMLElement[]}
+ * @returns {HTMLElement} Wrapper containing rendered contents
*/
ve.ce.ContentBranchNode.prototype.getRenderedContents = function () {
- var i, ilen, j, jlen, item, itemAnnotations, ann,
+ var i, ilen, j, jlen, item, itemAnnotations, ann, clone,
store = this.model.doc.getStore(),
annotationStack = new ve.dm.AnnotationSet( store ),
annotatedHtml = [],
@@ -154,9 +187,13 @@
current.appendChild( doc.createTextNode( buffer
) );
buffer = '';
}
- // DOM equivalent of $( current ).append( itemHtml );
+ // DOM equivalent of $( current ).append( item.clone()
);
for ( j = 0, jlen = item.length; j < jlen; j++ ) {
- current.appendChild( item[j] );
+ // Append a clone so as to not relocate the
original node
+ clone = item[j].cloneNode( true );
+ // Store a reference to the original node in a
property
+ clone.veOrigNode = item[j];
+ current.appendChild( clone );
}
}
}
@@ -164,7 +201,7 @@
current.appendChild( doc.createTextNode( buffer ) );
buffer = '';
}
- return Array.prototype.slice.apply( wrapper.childNodes );
+ return wrapper;
};
@@ -194,8 +231,8 @@
oldWrapper = this.$element[0].cloneNode( true );
newWrapper = this.$element[0].cloneNode( false );
- for ( i = 0, len = rendered.length; i < len; i++ ) {
- newWrapper.appendChild( rendered[i] );
+ while ( rendered.firstChild ) {
+ newWrapper.appendChild( rendered.firstChild );
}
oldWrapper.normalize();
newWrapper.normalize();
@@ -211,10 +248,8 @@
}
}
- // Reattach child nodes with the right annotations
- for ( i = 0, len = rendered.length; i < len; i++ ) {
- this.$element[0].appendChild( rendered[i] );
- }
+ // Reattach nodes
+ this.constructor.static.appendRenderedContents( this.$element[0],
newWrapper );
// Add slugs
this.setupSlugs();
--
To view, visit https://gerrit.wikimedia.org/r/118785
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idd75cae3c9555f5aca4568666db203b3a3bc98cb
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
