Bug 176980 - On iOS, getData can't get text set by setData during copy event
Summary: On iOS, getData can't get text set by setData during copy event
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
: 179439 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-09-14 23:06 PDT by Ryosuke Niwa
Modified: 2017-11-09 16:09 PST (History)
7 users (show)

See Also:


Attachments
WIP (5.46 KB, patch)
2017-09-15 00:10 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Fixes the bug by refactoring (32.51 KB, patch)
2017-09-18 22:55 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Patch for landing (32.99 KB, patch)
2017-09-19 12:34 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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. ***