Bug 195053 - [iOS] Programmatic paste access should be granted when copying and pasting within the same origin
Summary: [iOS] Programmatic paste access should be granted when copying and pasting wi...
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: 2019-02-26 10:20 PST by Wenson Hsieh
Modified: 2019-03-02 13:50 PST (History)
16 users (show)

See Also:


Attachments
First pass (66.71 KB, patch)
2019-02-26 13:03 PST, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Patch for EWS (66.93 KB, patch)
2019-03-01 16:07 PST, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Fix WPE build (66.96 KB, patch)
2019-03-01 18:39 PST, 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 2019-02-26 10:20:15 PST
<rdar://problem/48134710>
Comment 1 Wenson Hsieh 2019-02-26 13:03:09 PST
Created attachment 363009 [details]
First pass
Comment 2 EWS Watchlist 2019-02-26 13:05:06 PST
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 3 Wenson Hsieh 2019-02-26 13:10:04 PST
(In reply to Build Bot from comment #2)
> Thanks for the patch. If this patch contains new public API please make sure
> it follows the guidelines for new WebKit2 GTK+ API. See
> http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

Doesn't seem relevant here, since the method is private.
Comment 4 Ryosuke Niwa 2019-03-01 14:00:54 PST
Comment on attachment 363009 [details]
First pass

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

r=me. Please be sure to fix WPE build failure before landing.

> Source/WebCore/dom/DOMPasteAccess.h:28
> +#import <wtf/Forward.h>

We should be using #include instead. That's why WPE isn't happy.

> Source/WebCore/page/Frame.cpp:686
> +        return response != DOMPasteAccessResponse::DeniedForGesture;

I think we should use a switch and explicitly list all values.
Otherwise, we could miss updating this location when we add a new kind of denial.

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:5032
> +    NSIndexSet *indices = [NSIndexSet indexSetWithIndexesInRange:NSMakeRange(0, [pasteboard numberOfItems])];

Use auto?

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:5033
> +    NSArray<NSData *> *allCustomData = [pasteboard dataForPasteboardType:@(WebCore::PasteboardCustomData::cocoaType()) inItemSet:indices];

Ditto.

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:5037
> +    BOOL foundAtLeastOneMatchingIdentifier;

Huh, does objective-C always initialize BOOL to NO?
Comment 5 Wenson Hsieh 2019-03-01 15:24:16 PST
Comment on attachment 363009 [details]
First pass

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

>> Source/WebCore/dom/DOMPasteAccess.h:28
>> +#import <wtf/Forward.h>
> 
> We should be using #include instead. That's why WPE isn't happy.

Indeed — fixed.

>> Source/WebCore/page/Frame.cpp:686
>> +        return response != DOMPasteAccessResponse::DeniedForGesture;
> 
> I think we should use a switch and explicitly list all values.
> Otherwise, we could miss updating this location when we add a new kind of denial.

Good idea — changed to use switch case.

>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:5032
>> +    NSIndexSet *indices = [NSIndexSet indexSetWithIndexesInRange:NSMakeRange(0, [pasteboard numberOfItems])];
> 
> Use auto?

👍

>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:5033
>> +    NSArray<NSData *> *allCustomData = [pasteboard dataForPasteboardType:@(WebCore::PasteboardCustomData::cocoaType()) inItemSet:indices];
> 
> Ditto.

👍

>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:5037
>> +    BOOL foundAtLeastOneMatchingIdentifier;
> 
> Huh, does objective-C always initialize BOOL to NO?

Good catch — BOOL is only initialized to NO for instance variables! Changed this to be NO.
Comment 6 Wenson Hsieh 2019-03-01 16:07:16 PST
Created attachment 363387 [details]
Patch for EWS
Comment 7 Wenson Hsieh 2019-03-01 18:39:43 PST
Created attachment 363396 [details]
Fix WPE build
Comment 8 WebKit Commit Bot 2019-03-02 13:50:50 PST
Comment on attachment 363396 [details]
Fix WPE build

Clearing flags on attachment: 363396

Committed r242317: <https://trac.webkit.org/changeset/242317>
Comment 9 WebKit Commit Bot 2019-03-02 13:50:51 PST
All reviewed patches have been landed.  Closing bug.