Bug 116412

Summary: [iOS] Get iOS port off legacy clipboard
Product: WebKit Reporter: Darin Adler <darin>
Component: PlatformAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dbates, ddkilzer
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: Other   
Bug Depends on: 119863    
Bug Blocks: 115980    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
dbates: review+
iOS-specific changes none

Description Darin Adler 2013-05-19 06:42:00 PDT
[iOS] Get iOS part off legacy clipboard
Comment 1 Darin Adler 2013-05-19 06:42:27 PDT
Created attachment 202233 [details]
Patch
Comment 2 WebKit Commit Bot 2013-05-19 06:43:55 PDT
Attachment 202233 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/dom/Clipboard.h', u'Source/WebCore/editing/Editor.cpp', u'Source/WebCore/platform/Pasteboard.h', u'Source/WebCore/platform/ios/ClipboardIOS.h', u'Source/WebCore/platform/ios/ClipboardIOS.mm', u'Source/WebCore/platform/ios/PasteboardIOS.mm']" exit_code: 1
Source/WebCore/editing/Editor.cpp:708:  Do not add platform specific code in WebCore outside of platform.  [build/webcore_platform_layering_violation] [5]
Source/WebCore/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 2 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Darin Adler 2013-05-19 06:46:40 PDT
(In reply to comment #2)
> Source/WebCore/editing/Editor.cpp:708:  Do not add platform specific code in WebCore outside of platform.  [build/webcore_platform_layering_violation] [5]

That guideline is right in spirit, but wrong in detail.
Comment 4 Darin Adler 2013-06-02 22:25:34 PDT
Created attachment 203554 [details]
Patch
Comment 5 Darin Adler 2013-08-13 09:20:11 PDT
Created attachment 208640 [details]
Patch
Comment 6 Darin Adler 2013-08-13 09:20:53 PDT
I need some help testing this in an iOS merge. I think the patch is ready.
Comment 7 Daniel Bates 2013-08-13 17:00:04 PDT
Comment on attachment 208640 [details]
Patch

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

This change looks sane to me. I noticed some minor issues. I am happy to help you merge this change into iOS WebKit and test it. We will likely need to make changes to iOS-specific files that aren't upstream, at the time of writing, before we can build with this change. It should be sufficient to land this change and then merge it into iOS WebKit and test it. Let me know if you would like to make such changes and test this patch before landing it in the WebKit.org repository.

Dave Kilzer may also have insight/suggestions into how best to merge and test this in iOS WebKit.

> Source/WebCore/editing/Editor.cpp:712
> +    clipboard->pasteboard()->setFrame(m_frame);

Notice that Clipboard::pasteboard() returns a reference instead of a pointer: <http://trac.webkit.org/browser/trunk/Source/WebCore/dom/Clipboard.h?rev=153978#L136>. So, we need to write this as:

clipboard->pasteboard().setFrame(m_frame);

> Source/WebCore/platform/ios/PasteboardIOS.mm:514
> +String Pasteboard::readString(const String& type) const

Notice that Pasteboard::readString() isn't declared const in Pasteboard.h, <http://trac.webkit.org/browser/trunk/Source/WebCore/platform/Pasteboard.h#L129>.

> Source/WebCore/platform/ios/PasteboardIOS.mm:605
> +ListHashSet<String> Pasteboard::types() const

Notice that Pasteboard::types() isn't declared const in Pasteboard.h, <http://trac.webkit.org/browser/trunk/Source/WebCore/platform/Pasteboard.h?rev=153978#L127>.
Comment 8 Daniel Bates 2013-08-15 13:46:47 PDT
Created attachment 208847 [details]
iOS-specific changes

Here are the changes we need to make in order to build this patch on iOS.
Comment 9 Darin Adler 2013-08-15 19:36:56 PDT
Just curious, Dan, any testing results yet? Did it work?
Comment 10 Darin Adler 2013-08-15 19:48:53 PDT
Committed r154160: <http://trac.webkit.org/changeset/154160>
Comment 11 Daniel Bates 2013-08-16 15:17:24 PDT
(In reply to comment #9)
> Just curious, Dan, any testing results yet? Did it work?

As far as I can tell copying and pasting works in iOS after this change. I copied and pasted combinations of English, Hebrew, and Chinese text from websites and third-party apps into content editable elements, <textarea>s and <input>s as well as copied and pasted such text between content editable elements, <textarea>s and <input>s.