Bug 155074

Summary: Support preview on attachment elements
Product: WebKit Reporter: Enrica Casucci <enrica>
Component: WebKit2Assignee: Enrica Casucci <enrica>
Status: RESOLVED FIXED    
Severity: Normal CC: sam, thorton
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch2 sam: review+

Enrica Casucci
Reported 2016-03-05 16:23:33 PST
We want to be able to preview attachment elements, like we do for links and images. rdar://problem/24806079
Attachments
Patch (5.67 KB, patch)
2016-03-05 16:28 PST, Enrica Casucci 		no flags
DetailsFormatted DiffDiff
Patch2 (5.80 KB, patch)
2016-03-05 18:53 PST, Enrica Casucci 		sam: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Enrica Casucci
Comment 1 2016-03-05 16:28:35 PST
Created attachment 273108 [details] Patch
Enrica Casucci
Comment 2 2016-03-05 18:53:38 PST
Created attachment 273117 [details] Patch2
Sam Weinig
Comment 3 2016-03-05 19:01:41 PST
Comment on attachment 273117 [details] Patch2 View in context: https://bugs.webkit.org/attachment.cgi?id=273117&action=review > Source/WebKit2/UIProcess/API/Cocoa/WKUIDelegatePrivate.h:76 > +- (NSUInteger)_webView:(WKWebView *)webView attachmentIndexElement:(_WKActivatedElementInfo *)element WK_AVAILABLE(NA, WK_IOS_TBA); This name is a bit awkward. I think something like, _webView:indexIntoAttachmentListForElement: or something like that. > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3712 > + enum { UIPreviewItemTypeAttachment = 5 }; Even though I suggested this, it now makes me nervous. I think you should just use the 5 below with a comment, or pick a name that won't conflict with UIKit.
Enrica Casucci
Comment 4 2016-03-06 19:30:01 PST
(In reply to comment #3) > Comment on attachment 273117 [details] > Patch2 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=273117&action=review > > > Source/WebKit2/UIProcess/API/Cocoa/WKUIDelegatePrivate.h:76 > > +- (NSUInteger)_webView:(WKWebView *)webView attachmentIndexElement:(_WKActivatedElementInfo *)element WK_AVAILABLE(NA, WK_IOS_TBA); > > This name is a bit awkward. I think something like, > _webView:indexIntoAttachmentListForElement: or something like that. > I'll change it. > > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3712 > > + enum { UIPreviewItemTypeAttachment = 5 }; > > Even though I suggested this, it now makes me nervous. I think you should > just use the 5 below with a comment, or pick a name that won't conflict with > UIKit. I agree with you, I'll use a different name for the enum.
Enrica Casucci
Comment 5 2016-03-06 19:37:46 PST
Committed revision 197656.
mitz
Comment 6 2016-03-06 19:44:47 PST
Comment on attachment 273117 [details] Patch2 View in context: https://bugs.webkit.org/attachment.cgi?id=273117&action=review > Source/WebKit2/UIProcess/API/Cocoa/WKUIDelegatePrivate.h:75 > +- (NSArray *)_attachmentListForWebView:(WKWebView *)webView WK_AVAILABLE(NA, WK_IOS_TBA); Can this be WK_ARRAY<> of a more specific type?
Note You need to log in before you can comment on or make changes to this bug.