Summary: | Add a mechanism to customize the long press action | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Enrica Casucci <enrica> | ||||
Component: | WebKit2 | Assignee: | Enrica Casucci <enrica> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | andersca, cdumez, commit-queue, esprehn+autocc, gyuyoung.kim, sam, thorton | ||||
Priority: | P2 | ||||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Enrica Casucci
2016-03-03 16:55:12 PST
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? |