1313264 - meta viewport and initial-scale=0 should be clamped to 0.25

Status: RESOLVED DUPLICATE of bug 1510214

(Core :: Layout, defect, P3)

Description

[Karl Dubost💡 :karlcow]

When trying to understand Web Compatibility issues about meta viewport, we seem to not not have the proper algorithm for meta viewport initial-scale value.

<meta name="viewport" content="width=device-width, initial-scale=0">

Currently the draft spec says:
https://drafts.csswg.org/css-device-adapt/#min-scale-max-scale


> The properties are translated into 'zoom', 'min-zoom', 
> and 'max-zoom' respectively with the following translations of values.
> 
>  1. Non-negative number values are translated to <number> values, clamped to the range [0.1, 10]
>    2. Negative number values are dropped
>    3. yes is translated to 1
>    4. device-width and device-height are translated to 10
>    5. no and unknown values are translated to 0.1 
> 
> For a viewport <META> element that translates into an @viewport 
> rule with no max-zoom declaration and a non-auto min-zoom value 
> that is larger than the max-zoom value of the UA stylesheet, 
> the min-zoom declaration value is clamped to the UA stylesheet 
> max-zoom value.

If I interpret this correctly we should be clamping the value to 0.1 aka

<meta name="viewport" content="width=device-width, initial-scale=0.1">

(Maybe I missed it)

See Also: 
https://github.com/whatwg/compat/issues/62
https://webcompat.com/issues/2293

Comment 1

[Mike Kaply [:mkaply]]

Can we look at Chrome and see what they are doing?

Comment 2

[Mike Kaply [:mkaply]]

Anything I can do to help move this along?

Do we need to build some testcases for other browsers?

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Yeah, first thing that needs to happen is figuring out what other browsers (probably Chrome and Safari are most relevant) are doing here.

Comment 4

[Mike Taylor [:miketaylr]]

(In reply to David Baron :dbaron: ⌚️UTC-8 from comment #3)
> Yeah, first thing that needs to happen is figuring out what other browsers
> (probably Chrome and Safari are most relevant) are doing here.

I think Karl has done a lot of cross-browser research here.

Comment 5

[Karl Dubost💡 :karlcow]

1. Yes, there are clamping values.
2. It's difficult to understand the full code.
I spent last week a bit of time in source code. 
On WebKit if I understand the code right, there are clamping values


Let me add a couple of tests. 
http://www.la-grange.net/2016/12/01/viewport/index.html


Here pages with the following conditions.
1. The biggest element in the page is an element which is 200px wide
2. The width is set at 250
   <meta name="viewport" content="width=250, initial-scale=1.0">
3. we change the initial-scale
  * test 0010 - width=250, initial-scale=1.0
    http://www.la-grange.net/2016/12/01/viewport/viewport-test-0010.html
  * test 0011 - width=250, initial-scale=0.5
    http://www.la-grange.net/2016/12/01/viewport/viewport-test-0011.html
  * test 0012 - width=250, initial-scale=0.1
    http://www.la-grange.net/2016/12/01/viewport/viewport-test-0012.html
  * test 0013 - width=250, initial-scale=0.01
    http://www.la-grange.net/2016/12/01/viewport/viewport-test-0013.html


==============
Results: 
==============
# iOS 10.2.1, Safari 10 (602.1) [iPodTouch]
  window.devicePixelRatio = 2
  window.screen.width = 320px

* test 010 - initial-scale=1.0
  document.documentElement.clientWidth = 320px
* test 011 - initial-scale=0.5
  document.documentElement.clientWidth = 640px  (320/640 = 0.5)
* test 012 - initial-scale=0.1
  document.documentElement.clientWidth = 1280px (320/1280 = 0.25)
* test 013 - initial-scale=0.01
  document.documentElement.clientWidth = 1280px (320/1280 = 0.25)
 
The value is clamped at 0.25

Their test
https://github.com/WebKit/webkit/blob/c595dc9b3993d095e25311b0ec1797bd665447e8/LayoutTests/fast/viewport/viewport-35.html
https://github.com/WebKit/webkit/blob/c595dc9b3993d095e25311b0ec1797bd665447e8/LayoutTests/fast/viewport/viewport-35-expected.txt

And indeed in 
https://github.com/WebKit/webkit/blob/0d326ed82bcfa89da5e4078641049999d4446ac5/Source/WebCore/page/ViewportConfiguration.cpp#L220-L230

ViewportConfiguration::Parameters ViewportConfiguration::webpageParameters()
{
    Parameters parameters;
    parameters.width = 980;
    parameters.widthIsSet = true;
    parameters.allowsUserScaling = true;
    parameters.allowsShrinkToFit = true;
    parameters.minimumScale = 0.25;
    parameters.maximumScale = 5;
    return parameters;
}


to note that they have
ViewportConfiguration::Parameters ViewportConfiguration::textDocumentParameters()
with parameters.minimumScale = 0.25;

BUT (though it doesn't change anything for images inside documents.)
ViewportConfiguration::Parameters ViewportConfiguration::imageDocumentParameters()
with parameters.minimumScale = 0.01;


They also have this but I haven't figured out yet when it was coming into play.
https://github.com/WebKit/webkit/blob/0d326ed82bcfa89da5e4078641049999d4446ac5/Source/WebCore/page/ViewportConfiguration.cpp#L312-L313

void ViewportConfiguration::updateConfiguration()
{
    m_configuration = m_defaultConfiguration;

    const double minimumViewportArgumentsScaleFactor = 0.1;
    const double maximumViewportArgumentsScaleFactor = 10.0;

    bool viewportArgumentsOverridesInitialScale;
    bool viewportArgumentsOverridesWidth;
    bool viewportArgumentsOverridesHeight;
…



In the original code for introducing 0.25, we get the following comment
https://github.com/WebKit/webkit/blame/677689d1ffa397b77634ef32c37574d46961815c/Source/WebCore/page/ViewportConfiguration.cpp#L45-L46


    // Setup a reasonable default configuration to avoid computing infinite scale/sizes.
    // Those are the original iPhone configuration.



# Android 4.4.4 Chrome 56
  window.devicePixelRatio = 3
  window.screen.width = 360px

* test 010 - initial-scale=1.0
  document.documentElement.clientWidth = 360px
* test 011 - initial-scale=0.5
  document.documentElement.clientWidth = 720px  (360/720 = 0.5)
* test 012 - initial-scale=0.1
  document.documentElement.clientWidth = 1440px (360/1440 = 0.25)
* test 013 - initial-scale=0.01
  document.documentElement.clientWidth = 1280px (360/1280 = 0.25)


# Android 4.4.4 Firefox 54
  window.devicePixelRatio = 3
  window.screen.width = 360px

* test 010 - initial-scale=1.0
  document.documentElement.clientWidth = 360px
* test 011 - initial-scale=0.5
  document.documentElement.clientWidth = 720px  (360/720 = 0.5)
* test 012 - initial-scale=0.1
  document.documentElement.clientWidth = 3600px (360/3600 = 0.1)
* test 013 - initial-scale=0.01
  document.documentElement.clientWidth = 250px (360/250 = 1.44)



Let me modify my title for this bug. 
Also under 0.1, it seems Firefox looses control… and ignore initial-scale and comes back to the width specified in the meta viewport. Here 250px. o_O

Comment 6

[Karl Dubost💡 :karlcow]

If I'm reading the right thing:
https://dxr.mozilla.org/mozilla-central/rev/e150eaff1f83e4e4a97d1e30c57d233859efe9cb/dom/base/nsViewportInfo.h#15-18

static const mozilla::LayoutDeviceToScreenScale kViewportMinScale(0.1f);
static const mozilla::LayoutDeviceToScreenScale kViewportMaxScale(10.0f);
static const mozilla::CSSIntSize kViewportMinSize(200, 40);
static const mozilla::CSSIntSize kViewportMaxSize(10000, 10000);

Then
https://dxr.mozilla.org/mozilla-central/rev/e150eaff1f83e4e4a97d1e30c57d233859efe9cb/dom/base/nsViewportInfo.h#71-76

     /**
     * Constrain the viewport calculations from the
     * nsIDocument::GetViewportInfo() function in order to always return
     * sane minimum/maximum values.
     */
    void ConstrainViewportValues();

which goes here
https://dxr.mozilla.org/mozilla-central/source/dom/base/nsViewportInfo.cpp#14

I would love to understand what is happening for values under 0.1

Comment 7

[Karl Dubost💡 :karlcow]

Confirmation for Chrome. Clamped at 0.25.
Given by https://github.com/whatwg/compat/issues/62#issuecomment-283675333

Source code is:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/html/HTMLMetaElement.cpp?type=cs&q=HTMLMetaElement&l=397

Comment 8

[Mike Kaply [:mkaply]]

dbaron: So it looks like we have our data. 0.25 it is. What do we change here?

I tried changing

static const mozilla::LayoutDeviceToScreenScale kViewportMinScale(0.1f);
to
static const mozilla::LayoutDeviceToScreenScale kViewportMinScale(0.25f);

But that didn't work. Clearly I don't understand this code :)

Comment 9

[Karl Dubost💡 :karlcow]

This part of the code seems also to play in terms of zoom.
https://dxr.mozilla.org/mozilla-central/source/dom/base/nsDocument.cpp#7611

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So I don't know much about that code either, so redirecting needinfo to kats, who seems to have at least a bit of the hg blame for nsDocument::GetViewportInfo.

Comment 11

[Kartikaya Gupta (email:[e-mail])]

(In reply to Karl Dubost :karlcow from comment #6)
> which goes here
> https://dxr.mozilla.org/mozilla-central/source/dom/base/nsViewportInfo.cpp#14
> 
> I would love to understand what is happening for values under 0.1

So yes, that's (part of) the relevant code. What happens for values under 0.1 is that we set mDefaultZoom back to 0.1, but we also set mDefaultZoomValid = false. And when we do that, we later use a different algorithm [1] to pick the initial scale.

Reading the spec you quoted in comment 0, it looks like we should be clamping to 0.1 in this case, which could probably be achieved by just not setting mDefaultZoomValid = false in the first place. If that fixes this case but doesn't make anything worse I'd be fine with making that change. Do you have cycles to try it out? If not let me know and I can push a build to tryserver with this change.

[1] http://searchfox.org/mozilla-central/rev/d4adaabd6d2f88be0d0b151e022c06f6554909da/layout/base/MobileViewportManager.cpp#220

Comment 12

[Mike Kaply [:mkaply]]

I can try this out locally.

Comment 13

[Mike Kaply [:mkaply]]

Attachment: Patch that isn't working

This is the patch I tried, but wsj.com is still showing oversized instead of fitting in the viewport.

Am I missing something?

Comment 14

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8844948 [details] [diff] [review]
Patch that isn't working

This patch looks correct as far as I can tell. Can you uncomment the MVM_LOG at the top of layout/base/MobileViewportManager.cpp and load the page with that? The log might shed some more on what's going on.

Comment 15

[Mike Kaply [:mkaply]]

Attachment: MVM log

For this log, I loaded wsj.com, cleared the log and then did a reload.

It's definitely showing the mobile version of wsj, just very wide.

Comment 16

[Kartikaya Gupta (email:[e-mail])]

Wait, why are we looking at wsj.com? That seems to have an initial-scale of 1.0001, at least when I check on desktop with UA spoofing/RDM. I thought http://www.la-grange.net/2016/12/01/viewport/viewport-test-0013.html was the relevant test here, because that one has initial-scale=0.01 (which is less than the allowed minimum).

Comment 17

[Mike Kaply [:mkaply]]

The original bug that pointed me over here was:

https://bugzilla.mozilla.org/show_bug.cgi?id=1323062#c17

So I thought we were looking at the same issue.

So I guess we have two separate issues?

Comment 18

[Kartikaya Gupta (email:[e-mail])]

We have a number of different viewport issues. I don't think the wsj viewport issue is the same as this one. That one (if I'm reading it correctly, we treat 1.0001 very differently from 1.0?) seems pretty weird, and doesn't seem like intended behaviour.

Comment 19

[Karl Dubost💡 :karlcow]

(drifting)
The 1.0001 issue is something different that we need to treat separately. :)
I'm still investigating. From what I have gathered right now, it is due to a code change in iOS between 8 and 9 (an epsilon value for fuzzy comparison, it's all crazy ). And the Web developers wanted to previous behavior, but it creates Web compat issues down the road. 

(back on track)
Here we are talking about the min-value clamping and that any values inferior to the min clamping value should not trigger weird behavior (like we see currently in Firefox)
The spec says 0.1 but probably should say 0.25 as both Chrome and iOS clamps it at 0.25.
I intend to send a comment to the spec developer. 

Thanks kats for catching this.
Thanks mike for working on this patch. 

:)

Comment 20

[Mike Kaply [:mkaply]]

It looks like la-grange.net went away. Any other site to see this behavior?

Comment 21

[Karl Dubost💡 :karlcow]

Attachment: viewport-test-0010.html

Comment 22

[Karl Dubost💡 :karlcow]

Attachment: viewport-test-0011.html

Comment 23

[Karl Dubost💡 :karlcow]

Attachment: viewport-test-0012.html

Comment 24

[Karl Dubost💡 :karlcow]

Attachment: viewport-test-0013.html

Comment 25

[Karl Dubost💡 :karlcow]

Uploaded the test cases here. :)
(YES my server is down for a couple of days. Moving machines.  Sorry about that)

Comment 26

[Mike Kaply [:mkaply]]

Attachment: Screenshot after patch

So I've finally had the chance to do some testing with these. I do see different results with the patch, but they still don't match chrome.

See the attached.

So there's definitely more going on here...

Comment 27

[Karl Dubost💡 :karlcow]

(In reply to Karl Dubost :karlcow from comment #19)
> (drifting)
> The 1.0001 issue is something different that we need to treat separately. :)
> I'm still investigating. From what I have gathered right now, it is due to a
> code change in iOS between 8 and 9 (an epsilon value for fuzzy comparison,
> it's all crazy ). And the Web developers wanted to previous behavior, but it
> creates Web compat issues down the road. 

See Bug 1469747 for this.

Comment 28

[Hiroyuki Ikezoe (:hiro)]

The value has been changed in bug 1510214.