jenkins-bot has submitted this change and it was merged.
Change subject: Factor the <pre> newline hack out of the converter into
ve.properInnerHTML()
......................................................................
Factor the <pre> newline hack out of the converter into ve.properInnerHTML()
Also add detection for whether the browser is actually broken (most are,
but some, like Opera, aren't), treat <textarea> and <listing> in addition
to <pre>, and fix a bug where the function would crash if the <pre> was
empty (because .firstChild was undefined/null).
Change-Id: I541b57e9fd5c9c42d19d0a59f6e29fb43d35c9b6
---
M modules/ve/dm/ve.dm.Converter.js
M modules/ve/init/mw/ve.init.mw.Target.js
M modules/ve/test/dm/ve.dm.example.js
M modules/ve/ve.js
4 files changed, 48 insertions(+), 25 deletions(-)
Approvals:
Esanders: Looks good to me, approved
jenkins-bot: Verified
diff --git a/modules/ve/dm/ve.dm.Converter.js b/modules/ve/dm/ve.dm.Converter.js
index 66191be..93ab24a 100644
--- a/modules/ve/dm/ve.dm.Converter.js
+++ b/modules/ve/dm/ve.dm.Converter.js
@@ -1145,22 +1145,6 @@
}
delete container.lastOuterPost;
}
-
- // Workaround for bug 42469: if a <pre> starts with a newline, that
means .innerHTML will
- // screw up and stringify it with one fewer newline. Work around this
by adding a newline.
- // If we don't see a leading newline, we still don't know if the
original HTML was
- // <pre>Foo</pre> or <pre>\nFoo</pre> , but that's a syntactic
difference, not a semantic
- // one, and handling that is Parsoid's job.
- $( container ).find( 'pre' ).each( function() {
- var matches;
- if ( this.firstChild.nodeType === Node.TEXT_NODE ) {
- matches = this.firstChild.data.match( /^(\r\n|\r|\n)/ );
- if ( matches && matches[1] ) {
- // Prepend a newline exactly like the one we saw
- this.firstChild.insertData( 0, matches[1] );
- }
- }
- } );
};
/* Initialization */
diff --git a/modules/ve/init/mw/ve.init.mw.Target.js
b/modules/ve/init/mw/ve.init.mw.Target.js
index 30fd921..a51582f 100644
--- a/modules/ve/init/mw/ve.init.mw.Target.js
+++ b/modules/ve/init/mw/ve.init.mw.Target.js
@@ -437,7 +437,7 @@
'oldid': this.oldid,
'basetimestamp': this.baseTimeStamp,
'starttimestamp': this.startTimeStamp,
- 'html': doc.body.innerHTML, // TODO make this send the
whole document in the future
+ 'html': ve.properInnerHTML( doc.body ), // TODO make
this send the whole document in the future
'token': this.editToken,
'summary': options.summary,
'minor': Number( options.minor ),
@@ -468,7 +468,7 @@
'paction': 'diff',
'page': this.pageName,
'oldid': this.oldid,
- 'html': doc.body.innerHTML, // TODO make this send the
whole document in the future
+ 'html': ve.properInnerHTML( doc.body ), // TODO make
this send the whole document in the future
// TODO: API required editToken, though not relevant
for diff
'token': this.editToken
},
@@ -556,7 +556,7 @@
'data': {
'action': 'visualeditor',
'paction': 'serialize',
- 'html': doc.body.innerHTML, // TODO make this send the
whole document in the future
+ 'html': ve.properInnerHTML( doc.body ), // TODO make
this send the whole document in the future
'page': this.pageName,
'oldid': this.oldid,
'token': this.editToken,
@@ -598,7 +598,7 @@
ve.createDocumentFromHTML( '<body>' +
this.originalHtml + '</body>' )
),
'editedData': editedData,
- 'editedHtml': ve.dm.converter.getDomFromData( store,
editedData ).body.innerHTML,
+ 'editedHtml': ve.properInnerHTML(
ve.dm.converter.getDomFromData( store, editedData ).body ),
'wiki': mw.config.get( 'wgDBname' )
};
$.post(
diff --git a/modules/ve/test/dm/ve.dm.example.js
b/modules/ve/test/dm/ve.dm.example.js
index 21d4343..e0ac641 100644
--- a/modules/ve/test/dm/ve.dm.example.js
+++ b/modules/ve/test/dm/ve.dm.example.js
@@ -1645,11 +1645,7 @@
'\n',
'\n',
{ 'type': '/preformatted' }
- ],
- // pre newline hack
- // TODO we should test this using a better, more
.innerHTML-based mechanism for
- // comparing DOM trees
- 'normalizedHtml':
'<body>\n<pre>\n\n\n\nFoo\n\n\nBar\n\n\n\n</pre>\n\n\n\n\n</body>'
+ ]
},
'whitespace preservation in table cell starting with text and ending
with annotation': {
'html': '<body><table><tbody><tr><td>Foo
<b>Bar</b></td></tr></tbody></table></body>',
diff --git a/modules/ve/ve.js b/modules/ve/ve.js
index 720e224..00cd8f0 100644
--- a/modules/ve/ve.js
+++ b/modules/ve/ve.js
@@ -881,6 +881,49 @@
return newDocument;
};
+ /**
+ * Get the actual inner HTML of a DOM node.
+ *
+ * In most browsers, .innerHTML is broken and eats newlines in <pre>s,
see
+ * https://bugzilla.mozilla.org/show_bug.cgi?id=838954 . This function
detects this behavior
+ * and works around it, to the extent possible. <pre>\nFoo</pre> will
become <pre>Foo</pre>
+ * if the browser is broken, but newlines are preserved in all other
cases.
+ *
+ * @param {HTMLElement} element HTML element to get inner HTML of
+ * @returns {string} Inner HTML
+ */
+ ve.properInnerHTML = function ( element ) {
+ var div, $element;
+ if ( ve.isPreInnerHTMLBroken === undefined ) {
+ // Test whether newlines in <pre> are serialized back
correctly
+ div = document.createElement( 'div' );
+ div.innerHTML = '<pre>\n\n</pre>';
+ ve.isPreInnerHTMLBroken = div.innerHTML ===
'<pre>\n</pre>';
+ }
+
+ if ( !ve.isPreInnerHTMLBroken ) {
+ return element.innerHTML;
+ }
+
+ // Workaround for bug 42469: if a <pre> starts with a newline,
that means .innerHTML will
+ // screw up and stringify it with one fewer newline. Work
around this by adding a newline.
+ // If we don't see a leading newline, we still don't know if
the original HTML was
+ // <pre>Foo</pre> or <pre>\nFoo</pre> , but that's a syntactic
difference, not a semantic
+ // one, and handling that is Parsoid's job.
+ $element = $( element ).clone();
+ $element.find( 'pre, textarea, listing' ).each( function() {
+ var matches;
+ if ( this.firstChild && this.firstChild.nodeType ===
Node.TEXT_NODE ) {
+ matches = this.firstChild.data.match(
/^(\r\n|\r|\n)/ );
+ if ( matches && matches[1] ) {
+ // Prepend a newline exactly like the
one we saw
+ this.firstChild.insertData( 0,
matches[1] );
+ }
+ }
+ } );
+ return $element.get( 0 ).innerHTML;
+ };
+
// Based on the KeyEvent DOM Level 3 (add more as you need them)
//
http://www.w3.org/TR/2001/WD-DOM-Level-3-Events-20010410/DOM3-Events.html#events-Events-KeyEvent
ve.Keys = window.KeyEvent || {
--
To view, visit https://gerrit.wikimedia.org/r/59781
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I541b57e9fd5c9c42d19d0a59f6e29fb43d35c9b6
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <roan.katt...@gmail.com>
Gerrit-Reviewer: Esanders <esand...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
