Bug 146557 - Allow the UIDelegate to customize an image preview
Summary: Allow the UIDelegate to customize an image preview
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-02 13:43 PDT by Beth Dakin
Modified: 2015-07-02 16:58 PDT (History)
7 users (show)

See Also:

Attachments
Patch (8.63 KB, patch)
2015-07-02 13:53 PDT, Beth Dakin 		thorton: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 2015-07-02 13:43:59 PDT 
Allow the UIDelegate to customize an image preview

rdar://problem/21657424
Comment 1 Beth Dakin 2015-07-02 13:53:28 PDT 
Created attachment 256030 [details]
Patch
Comment 2 Tim Horton 2015-07-02 14:20:58 PDT 
Comment on attachment 256030 [details]
Patch

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

> Source/WebKit2/UIProcess/API/Cocoa/WKUIDelegatePrivate.h:60
> +- (void)_webView:(WKWebView *)webView willPreviewImageWithURL:(NSURL *)imageURL;
> +- (void)_webView:(WKWebView *)webView commitPreviewedImageWithURL:(NSURL *)imageURL;

Please add WK_AVAILABLE macros.

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3216
> +    id<WKUIDelegatePrivate> uiDelegate = static_cast<id <WKUIDelegatePrivate>>([_webView UIDelegate]);

space after id

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3237
> +        if (absoluteImageURL.isEmpty() || !WebCore::protocolIsInHTTPFamily(absoluteImageURL))

At some point we should consider data: URL images.

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3241
> +            NSURL *targetURL = [NSURL _web_URLWithWTFString:_positionInformation.imageURL];

No real need for the temporary, just like below.

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3252
> +    id<WKUIDelegatePrivate> uiDelegate = static_cast<id <WKUIDelegatePrivate>>([_webView UIDelegate]);

space after id

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:2162
> +                info.imageURL = [(NSURL *)element->document().completeURL(downcast<HTMLImageElement>(*element).src()) absoluteString];

What happens if it's a srcset image?
Comment 3 Beth Dakin 2015-07-02 16:58:15 PDT 
(In reply to comment #2)
> Comment on attachment 256030 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=256030&action=review
> 
> > Source/WebKit2/UIProcess/API/Cocoa/WKUIDelegatePrivate.h:60
> > +- (void)_webView:(WKWebView *)webView willPreviewImageWithURL:(NSURL *)imageURL;
> > +- (void)_webView:(WKWebView *)webView commitPreviewedImageWithURL:(NSURL *)imageURL;
> 
> Please add WK_AVAILABLE macros.
> 

Added.

> > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3216
> > +    id<WKUIDelegatePrivate> uiDelegate = static_cast<id <WKUIDelegatePrivate>>([_webView UIDelegate]);
> 
> space after id
> 

Fixed.

> > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3237
> > +        if (absoluteImageURL.isEmpty() || !WebCore::protocolIsInHTTPFamily(absoluteImageURL))
> 
> At some point we should consider data: URL images.
> 

I added them! Still need to follow-up with Sam to make sure it's okay.

> > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3241
> > +            NSURL *targetURL = [NSURL _web_URLWithWTFString:_positionInformation.imageURL];
> 
> No real need for the temporary, just like below.
> 

Fixed.

> > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3252
> > +    id<WKUIDelegatePrivate> uiDelegate = static_cast<id <WKUIDelegatePrivate>>([_webView UIDelegate]);
> 
> space after id
> 

Fixed.

> > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:2162
> > +                info.imageURL = [(NSURL *)element->document().completeURL(downcast<HTMLImageElement>(*element).src()) absoluteString];
> 
> What happens if it's a srcset image?

Good eye! They were broken. I fixed them by getting the URL from the RenderImage's CachedImage instead. 

http://trac.webkit.org/changeset/186236