Created attachment 314620 [details] Paste event clipboard data inspector The ClipboardData for paste events are identical when performing Paste (⌘V) and Paste and Match Style (⌥⇧⌘V). Because they're indistinguishable, it's impossible to programmatically handle pastes correctly in a rich text editor (like https://github.com/basecamp/trix, which I help maintain). There's no way to infer the desired format and pick an appropriate type. To reproduce, open the attached paste-inspector.html file in Safari and paste as instructed. Note that the logged paste events are identical and both contain "text/html" and "text/plain" types. Chrome and Firefox only return a "text/plain" type when performing Paste and Match Style, which I assume is correct. This issue is also present in previous versions of Safari. Screenshots Safari: https://cl.ly/291S1I3L2B25 Chrome: https://cl.ly/3m3B1P0D0Q2E Firefox: https://cl.ly/3M3v2R2r2T2H
<rdar://problem/33138027>
@Javan: Thanks for reporting this! I'll look into it. FYI: in the meantime, one way you can work around this is to listen for a beforeinput or input event with the inputType "insertFromPaste" instead. When using Paste and Match Style, getting "text/html" from the event's dataTransfer will yield markup representing the unstyled fragment to be inserted into the DOM. So in both cases of pasting and pasting to match style, if you want to know the markup that is about to be inserted and do something custom with it, you can do something like: editor.addEventListener("beforeinput", (event) => { if (event.inputType === "insertFromPaste") { let markupToInsert = event.dataTransfer.getData("text/html"); console.log(`Inserting markup: ${markupToInsert}!`); /* Perform custom handling of the paste. */ event.preventDefault(); } }); ...of course, having the clipboard data explicitly contain just "text/plain" in this case would still be helpful, but this might be a viable alternative.
(In reply to Wenson Hsieh from comment #2) > @Javan: > > Thanks for reporting this! I'll look into it. > > FYI: in the meantime, one way you can work around this is to listen for a > beforeinput or input event with the inputType "insertFromPaste" instead. > When using Paste and Match Style, getting "text/html" from the event's > dataTransfer Cool! I had no idea paste data was available in beforeinput events. 💖
Created attachment 322781 [details] Fixes the bug
Comment on attachment 322781 [details] Fixes the bug Attachment 322781 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/4763530 New failing tests: editing/pasteboard/data-transfer-get-data-on-paste-as-plain-text-when-custom-pasteboard-data-disabled.html
Created attachment 322791 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Created attachment 322794 [details] Fixes builds
Comment on attachment 322794 [details] Fixes builds Attachment 322794 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/4764622 New failing tests: editing/pasteboard/data-transfer-get-data-on-paste-as-plain-text-when-custom-pasteboard-data-disabled.html
Created attachment 322805 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 322794 [details] Fixes builds View in context: https://bugs.webkit.org/attachment.cgi?id=322794&action=review r=me; let's figure out why it's failing EWS. > LayoutTests/editing/pasteboard/data-transfer-get-data-on-paste-as-plain-text-when-custom-pasteboard-data-disabled.html:11 > +description('Tests that pasting as plain text only exposes "text/plain" in the clipboard.'); It doesn't seem like this description is consistent with the test expectations below. > LayoutTests/editing/pasteboard/data-transfer-get-data-on-paste-as-plain-text-when-custom-pasteboard-data-disabled.html:13 > +if (!window.internals || !window.internals.settings) { No braces for single line if statement.
Comment on attachment 322794 [details] Fixes builds View in context: https://bugs.webkit.org/attachment.cgi?id=322794&action=review > LayoutTests/editing/pasteboard/data-transfer-get-data-on-paste-as-plain-text.html:25 > + if (window.testRunner) Nit - we don't need this window.testRunner check (unless you meant to use testRunner.execCommand instead?)
(In reply to Wenson Hsieh from comment #11) > Comment on attachment 322794 [details] > Fixes builds > > View in context: > https://bugs.webkit.org/attachment.cgi?id=322794&action=review > > > LayoutTests/editing/pasteboard/data-transfer-get-data-on-paste-as-plain-text.html:25 > > + if (window.testRunner) > > Nit - we don't need this window.testRunner check (unless you meant to use > testRunner.execCommand instead?) Well, I'm doing that because document.execCommand("PasteAndMatchStyle") would be ignored inside the browser. (In reply to Wenson Hsieh from comment #10) > Comment on attachment 322794 [details] > Fixes builds > > View in context: > https://bugs.webkit.org/attachment.cgi?id=322794&action=review > > r=me; let's figure out why it's failing EWS. > > > LayoutTests/editing/pasteboard/data-transfer-get-data-on-paste-as-plain-text-when-custom-pasteboard-data-disabled.html:11 > > +description('Tests that pasting as plain text only exposes "text/plain" in the clipboard.'); > > It doesn't seem like this description is consistent with the test > expectations below. Fixed. > > LayoutTests/editing/pasteboard/data-transfer-get-data-on-paste-as-plain-text-when-custom-pasteboard-data-disabled.html:13 > > +if (!window.internals || !window.internals.settings) { > > No braces for single line if statement. Oops, fixed.
Comment on attachment 322794 [details] Fixes builds View in context: https://bugs.webkit.org/attachment.cgi?id=322794&action=review >>> LayoutTests/editing/pasteboard/data-transfer-get-data-on-paste-as-plain-text.html:25 >>> + if (window.testRunner) >> >> Nit - we don't need this window.testRunner check (unless you meant to use testRunner.execCommand instead?) > > Well, I'm doing that because document.execCommand("PasteAndMatchStyle") would be ignored inside the browser. > > (In reply to Wenson Hsieh from comment #10) Right, yea. I was confused because the line of code below this uses `document.execCommand`, not `testRunner.execCommand` (which is what I assumed you meant to use).
Hm... somehow text/html isn't included in the list of types when internals.settings.setCustomPasteboardDataEnabled(false) is called regardless of whether we do a regular paste or paste as plain text. So this is a regression independent of this code change (likely from r222595). For now, I'm going to add the following code to addHTMLClipboardTypesForCocoaType but this is a highly indicative or some other bug elsewhere. if ([cocoaType isEqualToString:(NSString *)kUTTypeHTML]) { resultTypes.add(ASCIILiteral("text/html")); return; }
Created attachment 322960 [details] Patch for landing
Comment on attachment 322960 [details] Patch for landing Wait for EWS.
Actually, let's not add that code but rather add a failing test expectation on iOS and file a new bug about that.
(In reply to Ryosuke Niwa from comment #17) > Actually, let's not add that code but rather add a failing test expectation > on iOS and file a new bug about that. Scratch that. Wenson confirmed this is the behavior we had in iOS 11.1.
Created attachment 322966 [details] Fixed the test on iOS
Comment on attachment 322966 [details] Fixed the test on iOS cq+ since mac-debug is 35 commits behind.
Comment on attachment 322966 [details] Fixed the test on iOS Clearing flags on attachment: 322966 Committed r222956: <http://trac.webkit.org/changeset/222956>
All reviewed patches have been landed. Closing bug.
Comment on attachment 322966 [details] Fixed the test on iOS View in context: https://bugs.webkit.org/attachment.cgi?id=322966&action=review > Source/WebCore/ChangeLog:9 > + When performing Paste and Match Style, only expose the plain text by creating a StaticPasteboard with plain text content. Are you sure this is right? While we do want to strip styles like bold, we don’t want to strip images, for example. Maybe it’s always been this way?
(In reply to Darin Adler from comment #23) > Comment on attachment 322966 [details] > Fixed the test on iOS > > View in context: > https://bugs.webkit.org/attachment.cgi?id=322966&action=review > > > Source/WebCore/ChangeLog:9 > > + When performing Paste and Match Style, only expose the plain text by creating a StaticPasteboard with plain text content. > > Are you sure this is right? While we do want to strip styles like bold, we > don’t want to strip images, for example. Maybe it’s always been this way? That's what I thought as well but Safari/WebKit just paste plain text stripping of images, etc... We may want to re-visit this behavior because some other apps on Mac DOES paste images but we probably need to do that independently of this change. This bug is about the DOM API which exposes the pasted content, and doesn't affect what gets pasted by WebKit.