[MediaWiki-commits] [Gerrit] Followup 80dd016: avoid using XMLSerializer when not absolut... - change (VisualEditor/VisualEditor)

[jenkins-bot (Code Review)]

jenkins-bot has submitted this change and it was merged.

Change subject: Followup 80dd016: avoid using XMLSerializer when not absolutely 
necessary
......................................................................


Followup 80dd016: avoid using XMLSerializer when not absolutely necessary

Because XMLSerializer is broken in Firefox: it URL-encodes hrefs.

Also move brokenness detection out of transformAttributes and into
parse/serializeXhtml.

Change-Id: I917944a16786040c57969c611f82192de29ee57b
---
M src/ve.js
1 file changed, 22 insertions(+), 14 deletions(-)

Approvals:
  Jforrester: Looks good to me, but someone else must approve
  Esanders: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/src/ve.js b/src/ve.js
index 8272d13..532bcc8 100644
--- a/src/ve.js
+++ b/src/ve.js
@@ -918,13 +918,12 @@
        /**
         * Helper function for #parseXhtml and #serializeXhtml.
         *
-        * Detect whether the browser normalizes style attributes (IE does this)
-        * and if so, map broken attributes to attributes prefixed with data-ve-
+        * Map attributes that are broken in IE to attributes prefixed with 
data-ve-
         * or vice versa.
         *
         * @param {string} html HTML string. Must also be valid XML
         * @param {boolean} unmask Map the masked attributes back to their 
originals
-        * @returns {string} HTML string, possibly modified to mask broken 
attributes
+        * @returns {string} HTML string modified to mask/unmask broken 
attributes
         */
        ve.transformStyleAttributes = function ( html, unmask ) {
                var xmlDoc, fromAttr, toAttr, i, len,
@@ -932,15 +931,6 @@
                                'style', // IE normalizes 'color:#ffd' to 
'color: rgb(255, 255, 221);'
                                'bgcolor' // IE normalizes '#FFDEAD' to 
'#ffdead'
                        ];
-
-               // Feature-detect style attribute breakage in IE
-               if ( ve.isStyleAttributeBroken === undefined ) {
-                       ve.isStyleAttributeBroken = ve.normalizeAttributeValue( 
'style', 'color:#ffd' ) !== 'color:#ffd';
-               }
-               if ( !ve.isStyleAttributeBroken ) {
-                       // Nothing to do
-                       return html;
-               }
 
                // Parse the HTML into an XML DOM
                xmlDoc = new DOMParser().parseFromString( html, 'text/xml' );
@@ -990,7 +980,14 @@
         * @return {HTMLDocument} HTML DOM
         */
        ve.parseXhtml = function ( html ) {
-               return ve.createDocumentFromHtml( ve.transformStyleAttributes( 
html, false ) );
+               // Feature-detect style attribute breakage in IE
+               if ( ve.isStyleAttributeBroken === undefined ) {
+                       ve.isStyleAttributeBroken = ve.normalizeAttributeValue( 
'style', 'color:#ffd' ) !== 'color:#ffd';
+               }
+               if ( ve.isStyleAttributeBroken ) {
+                       html = ve.transformStyleAttributes( html, false );
+               }
+               return ve.createDocumentFromHtml( html );
        };
 
        /**
@@ -1001,7 +998,18 @@
         * @return {string} Serialized HTML string
         */
        ve.serializeXhtml = function ( doc ) {
-               var xml = new XMLSerializer().serializeToString( 
ve.fixupPreBug( doc.documentElement ) );
+               var xml;
+               // Feature-detect style attribute breakage in IE
+               if ( ve.isStyleAttributeBroken === undefined ) {
+                       ve.isStyleAttributeBroken = ve.normalizeAttributeValue( 
'style', 'color:#ffd' ) !== 'color:#ffd';
+               }
+               if ( !ve.isStyleAttributeBroken ) {
+                       // Use outerHTML if possible because in Firefox, 
XMLSerializer URL-encodes
+                       // hrefs but outerHTML doesn't
+                       return ve.properOuterHtml( doc.documentElement );
+               }
+
+               xml = new XMLSerializer().serializeToString( ve.fixupPreBug( 
doc.documentElement ) );
                // HACK: strip out xmlns
                xml = xml.replace( '<html 
xmlns="http://www.w3.org/1999/xhtml";', '<html' );
                return ve.transformStyleAttributes( xml, true );

-- 
To view, visit https://gerrit.wikimedia.org/r/160809
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I917944a16786040c57969c611f82192de29ee57b
Gerrit-PatchSet: 2
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <roan.katt...@gmail.com>
Gerrit-Reviewer: Catrope <roan.katt...@gmail.com>
Gerrit-Reviewer: Esanders <esand...@wikimedia.org>
Gerrit-Reviewer: Jforrester <jforres...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits