<rdar://problem/48134710>
Created attachment 363009 [details] First pass
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
(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 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 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.
Created attachment 363387 [details] Patch for EWS
Created attachment 363396 [details] Fix WPE build
Comment on attachment 363396 [details] Fix WPE build Clearing flags on attachment: 363396 Committed r242317: <https://trac.webkit.org/changeset/242317>
All reviewed patches have been landed. Closing bug.