Esanders has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/173256

Change subject: Safari copy-paste fix
......................................................................

Safari copy-paste fix

Always place a nbsp; in the clipboard-key span as otherwise WebKit
browsers won't select it.

Fix clipboard-key detection to be recursive when using pasteTarget.

Fix getClipboardHash to be recursive.

Try to avoid https://code.google.com/p/chromium/issues/detail?id=318925
which causes spaces to be stripped if a newline starts with a node, by
letting the pasteTarget be wider.

Bug: 71718

Change-Id: Iba35ab3a2b382fe8d166bde3f6d6ca16ef40ed12
---
M src/ce/styles/ve.ce.Surface.css
M src/ce/ve.ce.Surface.js
M tests/ce/ve.ce.Surface.test.js
3 files changed, 35 insertions(+), 25 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor 
refs/changes/56/173256/1

diff --git a/src/ce/styles/ve.ce.Surface.css b/src/ce/styles/ve.ce.Surface.css
index b5ce8cf..5e8ea14 100644
--- a/src/ce/styles/ve.ce.Surface.css
+++ b/src/ce/styles/ve.ce.Surface.css
@@ -26,14 +26,13 @@
        position: fixed;
        top: 1em; /* Hack: Stop the viewport scrolling when the paste target is 
typed into */
        left: 0;
-       width: 1px;
+       /* Try to avoid wrapping by not setting a width because of 
https://code.google.com/p/chromium/issues/detail?id=318925 */
        height: 1px;
        opacity: 0;
        overflow: hidden;
 }
 
 .ve-ce-surface-paste * {
-       width: 1px !important;
        height: 1px !important;
 }
 
diff --git a/src/ce/ve.ce.Surface.js b/src/ce/ve.ce.Surface.js
index 59a5907..9d453f1 100644
--- a/src/ce/ve.ce.Surface.js
+++ b/src/ce/ve.ce.Surface.js
@@ -241,18 +241,21 @@
  * When pasting, browsers normalize HTML to varying degrees.
  * This hash creates a comparable string for validating clipboard contents.
  *
- * @param {jQuery} $elements Clipboard HTML elements
+ * @param {HTMLNode[]} elements Clipboard HTML elements
  * @returns {string} Hash
  */
-ve.ce.Surface.static.getClipboardHash = function ( $elements ) {
-       var hash = '';
+ve.ce.Surface.static.getClipboardHash = function ( elements ) {
+       var i, l, element, hash = '';
        // Collect text contents, or just node name for content-less nodes.
-       $elements.each( function () {
+       for ( i = 0, l = elements.length; i < l; i++ ) {
+               element = elements[i];
                // Only use node types which are know to copy (e.g. not comment 
nodes)
-               if ( this.nodeType === Node.TEXT_NODE || this.nodeType === 
Node.ELEMENT_NODE ) {
-                       hash += this.textContent || '<' + this.nodeName + '>';
+               if ( element.nodeType === Node.TEXT_NODE ) {
+                       hash += element.textContent;
+               } else if ( element.nodeType === Node.ELEMENT_NODE ) {
+                       hash += '<' + element.nodeName + '>' + 
this.getClipboardHash( element.childNodes );
                }
-       } );
+       }
        // Whitespace may be added/removed, so strip it all
        return hash.replace( /\s/gm, '' );
 };
@@ -1340,7 +1343,7 @@
        } else {
                clipboardItem.hash = this.constructor.static.getClipboardHash( 
this.$pasteTarget.contents() );
                this.$pasteTarget.prepend(
-                       this.$( '<span>' ).attr( 'data-ve-clipboard-key', 
this.clipboardId + '-' + clipboardIndex )
+                       this.$( '<span>' ).attr( 'data-ve-clipboard-key', 
this.clipboardId + '-' + clipboardIndex ).html( '&nbsp;' )
                );
 
                // If direct clipboard editing is not allowed, we must use the 
pasteTarget to
@@ -1561,23 +1564,31 @@
                $( this ).attr( attrs );
                this.removeAttribute( 'data-ve-attributes' );
        } );
-
+beforePasteData.custom = '';
+beforePasteData.html = '';
        // Find the clipboard key
        if ( beforePasteData.custom ) {
                clipboardKey = beforePasteData.custom;
        } else {
-               $elements = beforePasteData.html ? this.$( $.parseHTML( 
beforePasteData.html ) ) : this.$pasteTarget.contents();
+               if ( beforePasteData.html ) {
+                       $elements = this.$( $.parseHTML( beforePasteData.html ) 
);
 
-               // Try to find the clipboard key hidden in the HTML
-               $elements = $elements.filter( function () {
-                       var val = this.getAttribute && this.getAttribute( 
'data-ve-clipboard-key' );
-                       if ( val ) {
-                               clipboardKey = val;
-                               // Remove the clipboard key span once read
-                               return false;
-                       }
-                       return true;
-               } );
+                       // Try to find the clipboard key hidden in the HTML
+                       $elements = $elements.filter( function () {
+                               var val = this.getAttribute && 
this.getAttribute( 'data-ve-clipboard-key' );
+                               if ( val ) {
+                                       clipboardKey = val;
+                                       // Remove the clipboard key span once 
read
+                                       return false;
+                               }
+                               return true;
+                       } );
+               } else {
+                       // HTML in pasteTarget my get wrapped, so use the 
recursive $.find to looke for the clipboard key
+                       clipboardKey = this.$pasteTarget.find( 
'span[data-ve-clipboard-key]' ).data( 've-clipboard-key' );
+                       // $elements is used by getClipboardHash so generate it 
too
+                       $elements = this.$pasteTarget.contents();
+               }
        }
 
        // If we have a clipboard key, validate it and fetch data
@@ -1591,7 +1602,7 @@
                        // hasn't been modified in another editor before being 
pasted back.
                        if ( beforePasteData.custom ||
                                this.clipboard[clipboardIndex].hash ===
-                                       
this.constructor.static.getClipboardHash( $elements )
+                                       
this.constructor.static.getClipboardHash( $elements.toArray() )
                        ) {
                                slice = this.clipboard[clipboardIndex].slice;
                        }
diff --git a/tests/ce/ve.ce.Surface.test.js b/tests/ce/ve.ce.Surface.test.js
index 82fb576..37ec4e4 100644
--- a/tests/ce/ve.ce.Surface.test.js
+++ b/tests/ce/ve.ce.Surface.test.js
@@ -458,9 +458,9 @@
 QUnit.test( 'getClipboardHash', 1, function ( assert ) {
        assert.strictEqual(
                ve.ce.Surface.static.getClipboardHash(
-                       $( $.parseHTML( '  <p class="foo"> Bar </p>\n\t<span 
class="baz"></span> Quux <h1><span></span>Whee</h1>' ) )
+                       $( $.parseHTML( '  <p class="foo"> B<b>a</b>r 
</p>\n\t<span class="baz"></span> Quux <h1><span></span>Whee</h1>' ) ).toArray()
                ),
-               'Bar<SPAN>QuuxWhee',
+               '<P>B<B>ar<SPAN>Quux<H1><SPAN>Whee',
                'Simple usage'
        );
 } );

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iba35ab3a2b382fe8d166bde3f6d6ca16ef40ed12
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Esanders <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to