WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
174165
DataTransfer shouldn't contain text/html when performing Paste and Match Style
https://bugs.webkit.org/show_bug.cgi?id=174165
Summary
DataTransfer shouldn't contain text/html when performing Paste and Match Style
Javan Makhmali
Reported
2017-07-05 10:59:09 PDT
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
Attachments
Paste event clipboard data inspector
(1.39 KB, text/html)
2017-07-05 10:59 PDT
,
Javan Makhmali
no flags
Details
Fixes the bug
(17.56 KB, patch)
2017-10-04 22:13 PDT
,
Ryosuke Niwa
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews122 for ios-simulator-wk2
(4.01 MB, application/zip)
2017-10-04 23:45 PDT
,
Build Bot
no flags
Details
Fixes builds
(18.55 KB, patch)
2017-10-04 23:55 PDT
,
Ryosuke Niwa
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews122 for ios-simulator-wk2
(4.00 MB, application/zip)
2017-10-05 01:32 PDT
,
Build Bot
no flags
Details
Patch for landing
(19.93 KB, patch)
2017-10-05 18:39 PDT
,
Ryosuke Niwa
no flags
Details
Formatted Diff
Diff
Fixed the test on iOS
(19.42 KB, patch)
2017-10-05 19:16 PDT
,
Ryosuke Niwa
no flags
Details
Formatted Diff
Diff
Show Obsolete
(5)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2017-07-05 11:01:53 PDT
<
rdar://problem/33138027
>
Wenson Hsieh
Comment 2
2017-09-19 08:03:31 PDT
@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.
Javan Makhmali
Comment 3
2017-09-19 08:19:48 PDT
(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. 💖
Ryosuke Niwa
Comment 4
2017-10-04 22:13:34 PDT
Created
attachment 322781
[details]
Fixes the bug
Build Bot
Comment 5
2017-10-04 23:45:47 PDT
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
Build Bot
Comment 6
2017-10-04 23:45:49 PDT
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
Ryosuke Niwa
Comment 7
2017-10-04 23:55:12 PDT
Created
attachment 322794
[details]
Fixes builds
Build Bot
Comment 8
2017-10-05 01:32:27 PDT
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
Build Bot
Comment 9
2017-10-05 01:32:28 PDT
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
Wenson Hsieh
Comment 10
2017-10-05 15:21:30 PDT
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.
Wenson Hsieh
Comment 11
2017-10-05 15:26:33 PDT
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?)
Ryosuke Niwa
Comment 12
2017-10-05 18:15:27 PDT
(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.
Wenson Hsieh
Comment 13
2017-10-05 18:36:35 PDT
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).
Ryosuke Niwa
Comment 14
2017-10-05 18:37:18 PDT
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; }
Ryosuke Niwa
Comment 15
2017-10-05 18:39:59 PDT
Created
attachment 322960
[details]
Patch for landing
Ryosuke Niwa
Comment 16
2017-10-05 18:40:12 PDT
Comment on
attachment 322960
[details]
Patch for landing Wait for EWS.
Ryosuke Niwa
Comment 17
2017-10-05 18:43:14 PDT
Actually, let's not add that code but rather add a failing test expectation on iOS and file a new bug about that.
Ryosuke Niwa
Comment 18
2017-10-05 19:10:37 PDT
(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.
Ryosuke Niwa
Comment 19
2017-10-05 19:16:43 PDT
Created
attachment 322966
[details]
Fixed the test on iOS
Ryosuke Niwa
Comment 20
2017-10-05 20:24:40 PDT
Comment on
attachment 322966
[details]
Fixed the test on iOS cq+ since mac-debug is 35 commits behind.
WebKit Commit Bot
Comment 21
2017-10-05 20:51:39 PDT
Comment on
attachment 322966
[details]
Fixed the test on iOS Clearing flags on attachment: 322966 Committed
r222956
: <
http://trac.webkit.org/changeset/222956
>
WebKit Commit Bot
Comment 22
2017-10-05 20:51:41 PDT
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 23
2017-10-07 13:33:02 PDT
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?
Ryosuke Niwa
Comment 24
2017-10-07 14:12:06 PDT
(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.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug