Bug 173293 - Unable to paste text that was copied from a page into the universal search field
Summary: Unable to paste text that was copied from a page into the universal search field
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: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-06-12 17:37 PDT by Wenson Hsieh
Modified: 2017-06-14 11:58 PDT (History)
8 users (show)

See Also:


Attachments
Patch (19.38 KB, patch)
2017-06-12 17:48 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Patch (18.30 KB, patch)
2017-06-12 19:49 PDT, Wenson Hsieh
rniwa: review+
Details | Formatted Diff | Diff
Patch for landing (holding for wk2r+) (20.13 KB, patch)
2017-06-12 22:49 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Patch (19.08 KB, patch)
2017-06-13 09:16 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2017-06-12 17:37:00 PDT
<rdar://problem/32440918>
Comment 1 Wenson Hsieh 2017-06-12 17:48:30 PDT
Created attachment 312737 [details]
Patch
Comment 2 Wenson Hsieh 2017-06-12 19:49:11 PDT
Created attachment 312745 [details]
Patch
Comment 3 Ryosuke Niwa 2017-06-12 20:14:01 PDT
Comment on attachment 312745 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        copying both rich and plain web content in PlatformPasteboardIOS.mm. This is because "public.content" is no
> +        longer a which UITextView is capable of interpreting and inserting text from in iOS 11.

This is a wordy way of saying that "UITextView no longer supports 'public.content' in iOS 11".

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:5281
> +- (void)executeEditCommand:(NSString *)command completionHandler:(void(^)(BOOL))completionHandler
> +{
> +    _page->executeEditCommand(command, [completion = makeBlockPtr(completionHandler)](WebKit::CallbackBase::Error error) {
> +        completion(error == WebKit::CallbackBase::Error::None);
> +    });
> +}
> +

I don't think you need this. See my comment below.

> Tools/TestWebKitAPI/Tests/ios/UIPasteboardTests.mm:53
> +    __block bool doneExecutingCommand = false;
> +    [webView executeEditCommand:@"copy" completionHandler:^(BOOL success) {
> +        EXPECT_TRUE(success);
> +        doneExecutingCommand = true;
> +    }];

You don't really need this new API. You just need to set the preference to enable copy & paste from JS.
See PasteboardNotification.mm.
Comment 4 Wenson Hsieh 2017-06-12 20:47:20 PDT
Comment on attachment 312745 [details]
Patch

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

Thanks! (I'll probably need a WK2 r+ for this before landing though)

>> Source/WebCore/ChangeLog:11
>> +        longer a which UITextView is capable of interpreting and inserting text from in iOS 11.
> 
> This is a wordy way of saying that "UITextView no longer supports 'public.content' in iOS 11".

Good point -- reworded. (I also had a typo: "public.content" should have been "public.text" here).

>> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:5281
>> +
> 
> I don't think you need this. See my comment below.

👍

>> Tools/TestWebKitAPI/Tests/ios/UIPasteboardTests.mm:53
>> +    }];
> 
> You don't really need this new API. You just need to set the preference to enable copy & paste from JS.
> See PasteboardNotification.mm.

Done!
Comment 5 Wenson Hsieh 2017-06-12 22:49:16 PDT
Created attachment 312750 [details]
Patch for landing (holding for wk2r+)
Comment 6 Darin Adler 2017-06-13 08:52:25 PDT
Comment on attachment 312750 [details]
Patch for landing (holding for wk2r+)

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

> Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:263
> +    // FIXME: For backwards compatibility with clients that expect pre-iOS 11 behavior ('public.text' with an NSString as the value) when copying rich web content.

I don’t understand the comment. What is the FIXME here? What needs to be fixed? Are you saying that this should be removed some day?

> Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:353
> +        // FIXME: Writing an NSString directly to the pasteboard is only for backwards compability with apps that expect a copy from web content to vend
> +        // public.text, and not more specific subtypes.

Ditto.
Comment 7 Wenson Hsieh 2017-06-13 08:55:31 PDT
Comment on attachment 312750 [details]
Patch for landing (holding for wk2r+)

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

>> Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:263
>> +    // FIXME: For backwards compatibility with clients that expect pre-iOS 11 behavior ('public.text' with an NSString as the value) when copying rich web content.
> 
> I don’t understand the comment. What is the FIXME here? What needs to be fixed? Are you saying that this should be removed some day?

Yes -- I'm trying to say that this is only here to ensure backwards compatibility, and that we should remove it in the future. I'll tweak this comment to make it more clear.

>> Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:353
>> +        // public.text, and not more specific subtypes.
> 
> Ditto.

I'll edit this comment to clarify that this is something we should remove in the future.
Comment 8 Wenson Hsieh 2017-06-13 09:16:08 PDT
Created attachment 312777 [details]
Patch
Comment 9 Wenson Hsieh 2017-06-13 09:39:39 PDT
Comment on attachment 312777 [details]
Patch

Thanks Tim!
Comment 10 WebKit Commit Bot 2017-06-13 09:58:17 PDT
Comment on attachment 312777 [details]
Patch

Clearing flags on attachment: 312777

Committed r218180: <http://trac.webkit.org/changeset/218180>