Catrope has uploaded a new change for review.
https://gerrit.wikimedia.org/r/59781
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(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor
refs/changes/81/59781/1
diff --git a/modules/ve/dm/ve.dm.Converter.js b/modules/ve/dm/ve.dm.Converter.js
index 9a49c2e..1d5dbb6 100644
--- a/modules/ve/dm/ve.dm.Converter.js
+++ b/modules/ve/dm/ve.dm.Converter.js
@@ -1099,22 +1099,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 5a13752..e89dbf2 100644
--- a/modules/ve/init/mw/ve.init.mw.Target.js
+++ b/modules/ve/init/mw/ve.init.mw.Target.js
@@ -433,7 +433,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 ),
@@ -463,7 +463,7 @@
'action': 'visualeditor',
'paction': 'diff',
'page': this.pageName,
- '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
},
@@ -551,7 +551,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,
'token': this.editToken,
'format': 'json'
@@ -592,7 +592,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 2a04071..250df3e 100644
--- a/modules/ve/test/dm/ve.dm.example.js
+++ b/modules/ve/test/dm/ve.dm.example.js
@@ -1646,11 +1646,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>'
+ ]
},
'mismatching whitespace data is ignored': {
'html': null,
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: newchange
Gerrit-Change-Id: I541b57e9fd5c9c42d19d0a59f6e29fb43d35c9b6
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
