Bug 176980

Summary: On iOS, getData can't get text set by setData during copy event
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: HTML EditingAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, darin, kaspern, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
WIP
none
Fixes the bug by refactoring
none
Patch for landing none

Description Ryosuke Niwa 2017-09-14 23:06:13 PDT
On iOS, calling getData('text/plain') after calling setData('text/plain', 'foo') doesn't return 'foo'.
Comment 1 Radar WebKit Bug Importer 2017-09-14 23:20:36 PDT
<rdar://problem/34453915>
Comment 2 Ryosuke Niwa 2017-09-15 00:10:13 PDT
Created attachment 320872 [details]
WIP
Comment 3 Ryosuke Niwa 2017-09-15 02:22:40 PDT
Let me fix this myself. I need to apply one more fix to dataTransfer.types, which always returns the list of all supported types at the moment :(
Comment 4 Ryosuke Niwa 2017-09-18 22:55:06 PDT
Created attachment 321183 [details]
Fixes the bug by refactoring
Comment 5 Darin Adler 2017-09-19 09:29:50 PDT
Comment on attachment 321183 [details]
Fixes the bug by refactoring

View in context: https://bugs.webkit.org/attachment.cgi?id=321183&action=review

> Source/WebCore/platform/Pasteboard.h:189
> +    virtual void commitToPasteboard(Pasteboard&) { ASSERT_NOT_REACHED(); }

Can we make this pure virtual instead? I’m sure there is some reason we can’t, but failing at runtime instead of compile time is not a good pattern.
Comment 6 Ryosuke Niwa 2017-09-19 12:14:49 PDT
(In reply to Darin Adler from comment #5)
> Comment on attachment 321183 [details]
> Fixes the bug by refactoring
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=321183&action=review
> 
> > Source/WebCore/platform/Pasteboard.h:189
> > +    virtual void commitToPasteboard(Pasteboard&) { ASSERT_NOT_REACHED(); }
> 
> Can we make this pure virtual instead? I’m sure there is some reason we
> can’t, but failing at runtime instead of compile time is not a good pattern.

So Pasteboard is the "real" pasteboard we use to write to pasteboard so we can't quite do that. I think the cleaner approach is to only add this to StaticPasteboard and just dynamic_cast dataTransfer->pasteboard(). I was debating that and just adding a virtual function but I'd just add a dynamic_cast instead.
Comment 7 Ryosuke Niwa 2017-09-19 12:17:53 PDT
Well, an even cleaner approach is if there was a local variable referencing to a StaticPasteboard in dispatchClipboardEvent itself but that's a bit tricky because StaticPasteboard is not ref-counted (uses unique_ptr).
Comment 8 Ryosuke Niwa 2017-09-19 12:34:28 PDT
Created attachment 321232 [details]
Patch for landing
Comment 9 Ryosuke Niwa 2017-09-19 12:38:08 PDT
Comment on attachment 321232 [details]
Patch for landing

Wait for EWS.
Comment 10 WebKit Commit Bot 2017-09-19 14:50:07 PDT
Comment on attachment 321232 [details]
Patch for landing

Clearing flags on attachment: 321232

Committed r222228: <http://trac.webkit.org/changeset/222228>
Comment 11 WebKit Commit Bot 2017-09-19 14:50:09 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Wenson Hsieh 2017-11-09 16:09:26 PST
*** Bug 179439 has been marked as a duplicate of this bug. ***