jenkins-bot has submitted this change and it was merged.
Change subject: Simplify getClipboardHash
......................................................................
Simplify getClipboardHash
In WebKit browsers pasting can re-order and add spans, so getClipboardHash
was suggesting the paste data had changed when it hadn't. This is not
a problem in Chrome and Firefox where we have the clipboard API path,
but Safari doesn't support this.
Additionally, context markers also broke clipboard hash comparison in
browsers where the clipboard API path isn't used, even if spans weren't
being reordered or added. So detection of internal paste was completely
broken in IE as well as in Safari.
Simplify getClipboardHash to just return plain text without whitespace.
Bug: T71494
Change-Id: Ice5199e4db996b577d60e7df9ccb730300c9ceee
---
M src/ce/ve.ce.Surface.js
M tests/ce/ve.ce.Surface.test.js
2 files changed, 17 insertions(+), 22 deletions(-)
Approvals:
Catrope: Looks good to me, approved
Jforrester: Looks good to me, approved
jenkins-bot: Verified
diff --git a/src/ce/ve.ce.Surface.js b/src/ce/ve.ce.Surface.js
index 6cb99d5..8ac6664 100644
--- a/src/ce/ve.ce.Surface.js
+++ b/src/ce/ve.ce.Surface.js
@@ -252,23 +252,18 @@
* When pasting, browsers normalize HTML to varying degrees.
* This hash creates a comparable string for validating clipboard contents.
*
- * @param {Node[]} nodes Clipboard HTML nodes
+ * @param {jQuery} $elements Clipboard HTML
+ * @param {Object} [beforePasteData] Paste information, including leftText and
rightText to strip
* @returns {string} Hash
*/
-ve.ce.Surface.static.getClipboardHash = function ( nodes ) {
- var i, l, node, hash = '';
- // Collect text contents, or just node name for content-less nodes.
- for ( i = 0, l = nodes.length; i < l; i++ ) {
- node = nodes[i];
- // Only use node types which are know to copy (e.g. not comment
nodes)
- if ( node.nodeType === Node.TEXT_NODE ) {
- hash += node.textContent;
- } else if ( node.nodeType === Node.ELEMENT_NODE ) {
- hash += '<' + node.nodeName + '>' +
this.getClipboardHash( node.childNodes );
- }
- }
- // Whitespace may be added/removed, so strip it all
- return hash.replace( /\s/gm, '' );
+ve.ce.Surface.static.getClipboardHash = function ( $elements, beforePasteData
) {
+ beforePasteData = beforePasteData || {};
+ return $elements.text().slice(
+ beforePasteData.leftText ? beforePasteData.leftText.length : 0,
+ beforePasteData.rightText ? -beforePasteData.rightText.length :
undefined
+ )
+ // Whitespace may be modified (e.g. ' ' to ' '), so strip it all
+ .replace( /\s/gm, '' );
};
/* Methods */
@@ -1814,7 +1809,7 @@
* @param {jQuery.Event} e Paste event
*/
ve.ce.Surface.prototype.afterPaste = function () {
- var clipboardKey, clipboardId, clipboardIndex, range,
+ var clipboardKey, clipboardId, clipboardIndex, clipboardHash, range,
$elements, parts, pasteData, slice, tx, internalListRange,
data, doc, htmlDoc, $images, i,
context, left, right, contextRange,
@@ -1874,11 +1869,12 @@
}
return true;
} );
+ clipboardHash =
this.constructor.static.getClipboardHash( $elements );
} else {
// HTML in pasteTarget my get wrapped, so use the
recursive $.find to look 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();
+ // Pass beforePasteData so context gets stripped
+ clipboardHash =
this.constructor.static.getClipboardHash( this.$pasteTarget, beforePasteData );
}
}
@@ -1895,8 +1891,7 @@
// equal to the hash of the pasted HTML to assert that
the HTML
// hasn't been modified in another editor before being
pasted back.
if ( beforePasteData.custom ||
- this.clipboard[clipboardIndex].hash ===
-
this.constructor.static.getClipboardHash( $elements.toArray() )
+ this.clipboard[clipboardIndex].hash ===
clipboardHash
) {
slice = this.clipboard[clipboardIndex].slice;
}
diff --git a/tests/ce/ve.ce.Surface.test.js b/tests/ce/ve.ce.Surface.test.js
index 3d2687a..3dfd74d 100644
--- a/tests/ce/ve.ce.Surface.test.js
+++ b/tests/ce/ve.ce.Surface.test.js
@@ -601,9 +601,9 @@
QUnit.test( 'getClipboardHash', 1, function ( assert ) {
assert.strictEqual(
ve.ce.Surface.static.getClipboardHash(
- $( $.parseHTML( ' <p class="foo"> B<b>a</b>r
</p>\n\t<span class="baz"></span> Quux <h1><span></span>Whee</h1>' ) ).toArray()
+ $( ' <p class="foo"> B<b>a</b>r </p>\n\t<span
class="baz"></span> Quux <h1><span></span>Whee</h1>' )
),
- '<P>B<B>ar<SPAN>Quux<H1><SPAN>Whee',
+ 'BarQuuxWhee',
'Simple usage'
);
} );
--
To view, visit https://gerrit.wikimedia.org/r/200299
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ice5199e4db996b577d60e7df9ccb730300c9ceee
Gerrit-PatchSet: 3
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Esanders <esand...@wikimedia.org>
Gerrit-Reviewer: Catrope <roan.katt...@gmail.com>
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
