1550105 - Negative values in meta viewport result in different initial viewports on Firefox and Chrome

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Thomas Wisniewski [:twisniewski]]

For instance, using this:

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

Causes Firefox and Chrome to show different initial viewports, as seen on kpn.portal.fon.com.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

As per the device adaptation spec, negative values should be treated as if it's not specified, but we don't.

https://www.w3.org/TR/css-device-adapt-1/#min-scale-max-scale

Comment 2

[Hiroyuki Ikezoe (:hiro)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=04262e81b1d44f3825a4bd7331aed0fabaac2ec4

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1550105 - Implement the parser for scale values in viewport meta tag as what the spec defines. r?botond

The relevant definition in the spec;
https://drafts.csswg.org/css-device-adapt/#min-scale-max-scale

Before this change, if both of initial-scale and maximum-scale are negative,
both values are clamped to 0.25. Whereas with this change, negative scale
values are treated as if it's not specified so that initial-scale value is
automatically calculated based on the layout viewport size.
negative-initial-and-maximum-scale.html is a test case for the case.

Also with this change, initial-scale values are going to be clamped to the
range [0.1, 10] during parsing it as the spec says so that
initial-scale-100.html needs to be modified, now the test is scaled by 10x.
(Before this change, initial-scale=100 was treated as a invalid scale value
in nsViewportInfo::ConstrainViewportValues[1])

[1] https://searchfox.org/mozilla-central/rev/6c9f60f8cc064a1005cd8141ecd526578ae9da7a/dom/base/nsViewportInfo.cpp#15

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4cd6d838796a
Implement the parser for scale values in viewport meta tag as what the spec defines. r=botond

Comment 5

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/4cd6d838796a

Comment 7

[Botond Ballo [:botond]]

This fixed at least a couple of webcompat issues reported against Fennec. Could we ask for a 68 uplift so the fix reaches Fennec users? (Fennec is transitioning to 68 ESR, so a fix on the 69 branch would not reach Fennec users.)

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 9066616 [details]
Bug 1550105 - Implement the parser for scale values in viewport meta tag as what the spec defines. r?botond

Beta/Release Uplift Approval Request

• User impact if declined: Some kind of sites are not rendered properly on both on Mobile Firefox and on Responsive Design Mode in devtools
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): One caveat about this patch is that the patch changes initial-scale calculation (i.e. auto scaling to fit the content to the screen size) and the change is not compatible with prior versions of Mobile Firefox, so if there were sites that are optimized only for Mobile Firefox, the site will be broken to some extent, but I believe the change is the right thing to do because it's exactly what the spec defines, what Chome does actually.
• String changes made/needed: None

Comment 9

[Hiroyuki Ikezoe (:hiro)]

(In reply to Botond Ballo [:botond] from comment #7)

This fixed at least a couple of webcompat issues reported against Fennec. Could we ask for a 68 uplift so the fix reaches Fennec users? (Fennec is transitioning to 68 ESR, so a fix on the 69 branch would not reach Fennec users.)

Yeah, indeed. We should. I did forget about ESR. Thanks for suggestion!

Comment 10

[Julien Cristau [:jcristau]]

I'm tempted to punt right now but keep this on the radar for the 68.1 cycle. Leaving the uplift request as-is until we have esr68 flags in bugzilla.

Comment 11

[Julien Cristau [:jcristau]]

Comment on attachment 9066616 [details]
Bug 1550105 - Implement the parser for scale values in viewport meta tag as what the spec defines. r?botond

see comment 8

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9066616 [details]
Bug 1550105 - Implement the parser for scale values in viewport meta tag as what the spec defines. r?botond

Fixes a mobile web compat issue. Approved for 68.1esr and Fennec 68.1b1.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/fe659e4395fa

Comment 14

[Hiroyuki Ikezoe (:hiro)]

Thank you!