1431601 - [viewport] maximum-scale = 0 in meta viewport makes the viewport awfully wide

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect, P3)

Description

[Karl Dubost💡 :karlcow]

This is a spin-off of https://webcompat.com/issues/14888

STR:

1. Go to https://www.looopings.nl/

Actual:
Enjoy a very very wide website

Expected:
The site fits the screen like in Chrome.


Why this is happening:

The site contains the line

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


maximum-scale = 0 is putting the layout completely off.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

I think this bug will be solved by implementing "Resolve min-zoom and max-zoom values" [1] in the spec.

[1] https://drafts.csswg.org/css-device-adapt/#constraining-min-max-zoom

Comment 2

[Karl Dubost💡 :karlcow]

Interesting just happens to have a new issue with this.

Comment 3

[Karl Dubost💡 :karlcow]

(In reply to Karl Dubost :karlcow from comment #2)
> Interesting just happens to have a new issue with this.

So something interesting also with this issue 
https://webcompat.com/issues/19179

On responsive design mode (RDM) with Firefox Android UA and 360x640 window, we get a wide layout too, even if **meta viewport** is not supposed to be interpreted by RDM. but if we just go out of the RDM, and not reloading (so the mobile layout), we get a layout working even when resizing the window.

Comment 4

[Botond Ballo [:botond]]

(In reply to Karl Dubost :karlcow from comment #3)
> On responsive design mode (RDM) with Firefox Android UA and 360x640 window,
> we get a wide layout too, even if **meta viewport** is not supposed to be
> interpreted by RDM. but if we just go out of the RDM, and not reloading (so
> the mobile layout), we get a layout working even when resizing the window.

Note that as of a few days ago (bug 1290420), RDM noew respects meta viewport.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=18dbb943fa7e1b847265fefb908308e4472af58f

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Attachment: Comparison with the binary in the try in comment 5

Here are two screenshots of the site with adding below meta tag in comment 0 (because the site has already dropped the meta tag).

 <meta name="viewport" content="width = device-width, initial-scale = 1, minimum-scale = 1, maximum-scale = 0">
 
Left side is the result of current m-c, right one is the result of the binary in the try.  The too wide content is actually being mitigated by the "Resolve min-zoom and max-zoom values" to some extent, but sill wider than Chrome.  I think it is because of bug 1423709 and other bugs.  Anyway I am going to fix one by one.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1431601 - Align boolean member in nsViewportInfo. r=botond

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1431601 - Fix the comment for nsIDOMWindowUtils.getViewportInfo. r=botond

nsContentUtils::GetViewportInfo was removed and the comment for the function
was moved to nsIDocument::GetViewportInfo in
https://hg.mozilla.org/mozilla-central/rev/37d64688bf9d

Depends on D8316

Comment 9

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1431601 - Move getViewportInfo function into viewport_helpers.js. r=botond

Depends on D8317

Comment 10

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1431601 - Rewrite test_meta_viewportX.html with add_task. r=botond

Depends on D8318

Comment 11

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1431601 - Run meta viewport tests on Desktop. r=botond

Depends on D8319

Comment 12

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1431601 - Implement "Resolve min-zoom and max-zoom values" in the spec. r=botond

https://drafts.csswg.org/css-device-adapt/#constraining-min-max-zoom

The test case in this commit fails without this change, width will be 8000 and
height will be 4800 respectively when it fails.

Depends on D8320

Comment 13

[Hiroyuki Ikezoe (:hiro)]

For the reference (and requested by Botond in review), I should have noted what happened when 'maximum-scale=0' is specified along with initial-scale=1.


1. https://hg.mozilla.org/mozilla-central/file/87b544fc71d5/dom/base/nsDocument.cpp#l7327
 
    mScaleMaxFloat = mozilla::clamped(                                          
        mScaleMaxFloat, kViewportMinScale, kViewportMaxScale);

  Here, the maximum-scale=0 is clamped to kViewportMinScale which is 0.1.

2. https://hg.mozilla.org/mozilla-central/file/87b544fc71d5/dom/base/nsDocument.cpp#l7452

    if (mValidScaleFloat && scaleFloat >= scaleMinFloat && scaleFloat <= scaleMaxFloat) {
      CSSSize displaySize = ScreenSize(aDisplaySize) / scaleFloat;              
      size.width = std::max(size.width, displaySize.width);                     
      size.height = std::max(size.height, displaySize.height);                  
    } else if (effectiveValidMaxScale) {                                        
      CSSSize displaySize = ScreenSize(aDisplaySize) / scaleMaxFloat;           
      size.width = std::max(size.width, displaySize.width);                     
      size.height = std::max(size.height, displaySize.height);                  
    }

  Here, |scaleMaxFloat| is 0.1, thus we fails the first if check then effectiveValidMaxScale is true since we have a valid maximum-scale even if it's clamed, so we divide the screen size by |scaleMaxFloat|, which means the viewport size is 10 times bigger than the screen size.  And of course, unfortunately there is 'initial-scale=1'.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ecc77355ba3e
Align boolean member in nsViewportInfo. r=botond
https://hg.mozilla.org/integration/autoland/rev/930561bbe55d
Fix the comment for nsIDOMWindowUtils.getViewportInfo. r=botond
https://hg.mozilla.org/integration/autoland/rev/4a79571c40a0
Move getViewportInfo function into viewport_helpers.js. r=botond
https://hg.mozilla.org/integration/autoland/rev/d455d8d93e2c
Rewrite test_meta_viewportX.html with add_task. r=botond
https://hg.mozilla.org/integration/autoland/rev/02de107cdac4
Run meta viewport tests on Desktop. r=botond
https://hg.mozilla.org/integration/autoland/rev/2e7851b166ce
Implement "Resolve min-zoom and max-zoom values" in the spec. r=botond

Comment 15

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/ecc77355ba3e
https://hg.mozilla.org/mozilla-central/rev/930561bbe55d
https://hg.mozilla.org/mozilla-central/rev/4a79571c40a0
https://hg.mozilla.org/mozilla-central/rev/d455d8d93e2c
https://hg.mozilla.org/mozilla-central/rev/02de107cdac4
https://hg.mozilla.org/mozilla-central/rev/2e7851b166ce