Bug 110304 – Add setting to add viewport tag to image documents

• Status: UNCONFIRMED
• Component: New Bugs

[Tien-Ren Chen]

Add setting to add viewport tag to image documents

[Tien-Ren Chen]

Created attachment 189242 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Tien-Ren Chen]

Note: to experiment this patch you need to manually change the default value of Settings::overrideViewportForImage to true.

[Alexandre Elias]

Comment on attachment 189242 [details]
Patch

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

What happens when the image is very large?  I expect it will look cut off.  It would be better to have an exact fit in that case, especially given that deviceScaleFactor means we can usually have a pageScaleFactor of 0.5 to get the true resolution of the image.

> Source/WebCore/html/ImageDocument.cpp:210
> +        metaElement->setAttribute(contentAttr, "width=device-width, height=device-height");

Please remove "height=device-height", that is not normally present in viewport tags.

[Adam Barth]

Comment on attachment 189242 [details]
Patch

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

> Source/WebCore/html/ImageDocument.cpp:204
> +    if (frame()->page()->settings()->overrideViewportForImage()) {

Are frame and page always non-zero here?

[Tien-Ren Chen]

(In reply to comment #4)
> (From update of attachment 189242 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=189242&action=review
> 
> What happens when the image is very large?  I expect it will look cut off.  It would be better to have an exact fit in that case, especially given that deviceScaleFactor means we can usually have a pageScaleFactor of 0.5 to get the true resolution of the image.

Yea I'm not sure what behavior gives best user experience. Let's have some discussion with UX folks when we have time.

(In reply to comment #5)
> (From update of attachment 189242 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=189242&action=review
> 
> > Source/WebCore/html/ImageDocument.cpp:204
> > +    if (frame()->page()->settings()->overrideViewportForImage()) {
> 
> Are frame and page always non-zero here?

I think so. The same function calls shouldShrinkToFit() below, which accesses frame()->page()->settings() too.

[John Mellor]

(In reply to comment #6)
> (In reply to comment #4)
> > (From update of attachment 189242 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=189242&action=review
> > 
> > What happens when the image is very large?  I expect it will look cut off.  It would be better to have an exact fit in that case, especially given that deviceScaleFactor means we can usually have a pageScaleFactor of 0.5 to get the true resolution of the image.
> 
> Yea I'm not sure what behavior gives best user experience. Let's have some discussion with UX folks when we have time.

I agree with Alex, that it would be better to fit the image to the screen when it is too big, as happens on desktop. A simple way to do that would be to add "max-width: 100%;" to the <img> element. You would still be able to pinch-zoom in etc (though we should probably double-check the ensuing zoom limits, and that double-tap does something sensible).

Even better would be to fit both the width and height of the image within the window; perhaps simply enabling the shouldShrinkToFit functionality would enable this (though we might want to remove that click listener on touchscreens)?

[Andreas Kling]

Comment on attachment 189242 [details]
Patch

Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.