We need a new private delegate method to allow clients to customize the sheet displayed on long press. rdar://problem/24823732
Created attachment 272803 [details] Patch
Attachment 272803 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/API/Cocoa/WKUIDelegatePrivate.h:42: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebKit2/UIProcess/ios/WKActionSheetAssistant.mm:284: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebKit2/UIProcess/ios/WKActionSheetAssistant.mm:400: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 3 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 272803 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272803&action=review > Source/WebKit2/UIProcess/API/Cocoa/WKUIDelegatePrivate.h:47 > +typedef NS_OPTIONS(NSUInteger, WKElementType) { > + WKElementTypeLink, > + WKElementTypeImage, > + WKElementTypeAttachment > +} WK_ENUM_AVAILABLE(NA, WK_IOS_TBA); > + I think you don't need this anymore? > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:1091 > + [uiDelegate _webView:_webView showCustomSheetForElement:[[_WKActivatedElementInfo alloc] _initWithType:_WKActivatedElementTypeAttachment URL:[NSURL URLWithString:_positionInformation.url] location:_positionInformation.point title:_positionInformation.title rect:_positionInformation.bounds image:nil]]; Shouldn't this use _web_URLWithWTFString? > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3566 > + [UIApp _cancelAllTouches]; Yikes. > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:2251 > + info.title = attachment.attachmentTitle(); We never want to set isSelectable?
(In reply to comment #3) > Comment on attachment 272803 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=272803&action=review > > > Source/WebKit2/UIProcess/API/Cocoa/WKUIDelegatePrivate.h:47 > > +typedef NS_OPTIONS(NSUInteger, WKElementType) { > > + WKElementTypeLink, > > + WKElementTypeImage, > > + WKElementTypeAttachment > > +} WK_ENUM_AVAILABLE(NA, WK_IOS_TBA); > > + > > I think you don't need this anymore? > My bad. I'll remove it. > > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:1091 > > + [uiDelegate _webView:_webView showCustomSheetForElement:[[_WKActivatedElementInfo alloc] _initWithType:_WKActivatedElementTypeAttachment URL:[NSURL URLWithString:_positionInformation.url] location:_positionInformation.point title:_positionInformation.title rect:_positionInformation.bounds image:nil]]; > > Shouldn't this use _web_URLWithWTFString? > Yes. > We never want to set isSelectable? Not when handling long press.
Committed revision 197596.
Comment on attachment 272803 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272803&action=review > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:2253 > + info.url = downcast<HTMLAttachmentElement>(*hitNode).file()->path(); This is confusing. Is it a URL or a path?