1563649 - Computed value of 'bottom' for a position:fixed element is different in Firefox than in Chrome for Android

Status: VERIFIED FIXED

(Core :: Layout, defect, P3)

Description

[Ksenia Berezina [:ksenia]]

Attachment: 34281.html

As reported here:
https://github.com/webcompat/web-bugs/issues/34281

Steps to reproduce:

Go to https://www.cleanmeats.com.au/2019/07/01/out-of-body-experience-shiok-meats-and-their-lab-grown-seafood/ in Firefox for Android or in Nightly desktop RDM and observe the behavior.

Expected:
Functional site

Actual:
Site is covered by a social panel and unusable

Regression points to:
222:01.32 INFO: Last good revision: 4b74d76e55a819852c8fa925efd25c57fdf35c9d
222:01.32 INFO: First bad revision: 96d77e9c3f2985c2b702c0910e0da686bee54105
222:01.32 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=4b74d76e55a819852c8fa925efd25c57fdf35c9d&tochange=96d77e9c3f2985c2b702c0910e0da686bee54105

Site is relying on the computed bottom value of an element with class .g1-sharebar to do certain calculations and as a result applying top: 0px to it. That is causing element with .g1-sharebar to cover the whole screen.

I've attached a reduced test case. It has content on the page that is wider than the screen. The page looks the same in Firefox and Chrome, however when inspecting a computed value of bottom for an element with id=test, the values are different.

Comment 1

[Ksenia Berezina [:ksenia]]

Botond, the suggestion of using minimum-scale=1.0 from here https://bugzilla.mozilla.org/show_bug.cgi?id=1557707 fixes the issue, however I wonder if computed values should match the actual values, in this case bottom: 0px?

Comment 2

[Hiroyuki Ikezoe (:hiro)]

This seems to be a bug. We have to adjust the value somewhere in GetUsedAbsoluteOffset in the case where we expanded the layout viewport size.

Comment 3

[Botond Ballo [:botond]]

Thanks for the reduced testcase!

(In reply to Ksenia Berezina [:ksenia] from comment #1)

Botond, the suggestion of using minimum-scale=1.0 from here https://bugzilla.mozilla.org/show_bug.cgi?id=1557707 fixes the issue, however I wonder if computed values should match the actual values, in this case bottom: 0px?

I agree, we should be reporting bottom: 0px here.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: adjust-viewport-size-for-fixed-position

I have a patch for this, but I don't know who is the right person to ask review.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

The patch looks sensible to me fwiw (test and comment in nsComputedDOMStyle missing). I would've maybe made the method a method in ViewportFrame or such instead of PresShell since it seems easy to misuse otherwise. But it is the right fix.

Comment 6

[Liz Henry (:lizzard) (relman/hg->git project)]

Happy to take a patch for 70, but since this is already triaged and set to P3 priority I'm setting it as fix-optional.
That will remove the bug from weekly regression triage.

Sean, can you help find a reviewer for the patch? Thanks!

Comment 7

[Hiroyuki Ikezoe (:hiro)]

On IRC, Botond suggested me Markus for the reviewer. I will revise the patch as Emilio suggested in comment 5 (I did want to avoid calling doQueryFrame there but it's not a big deal) and send the review request to Markus.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1563649 - As with display list item and reflow for position:fixed elements, use the adjusted layout viewport size getComputedStyle() r?mstange

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bad311a6808c
As with display list item and reflow for position:fixed elements, use the adjusted layout viewport size getComputedStyle() r=mstange

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/18863 for changes under testing/web-platform/tests

Comment 11

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 12

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/bad311a6808c

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 14

[Chris Peterson [:cpeterson]]

Hiro, should we uplift your position:fixed fix from GeckoView 71 Nightly to 70 Beta? This bug is marked as a regression (in Fennec and GeckoView 67), but it is also a P3, suggesting it is not an urgent problem.

btw, since Fennec is now in maintenance mode in ESR 68, fixes in Gecko 71 will not automatically ride to Fennec. If we need to fix this regression in Fennec, we'll need to uplift the fix to ESR 68.

Comment 15

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 9085384 [details]
Bug 1563649 - As with display list item and reflow for position:fixed elements, use the adjusted layout viewport size getComputedStyle() r?mstange

Beta/Release Uplift Approval Request

• User impact if declined: It depends on what the site in question does, one of the sites reported on webcompat.com is rendered as blank
• 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): It just tweaks values by getComputedStyle for position:fixed elements, it shouldn't cause any serious issues, crashes etc.
• String changes made/needed: none

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is one of mobile web compat issues we've been tackling and worth fixing in Fennec
• User impact if declined: Same as above the description for beta uplift
• Fix Landed on Version: 71
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Same as above the description for beta uplift
• String or UUID changes made by this patch: none

Comment 16

[Hiroyuki Ikezoe (:hiro)]

Thanks you, Chris. Indeed we should uplift this to fennec.

Comment 17

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9085384 [details]
Bug 1563649 - As with display list item and reflow for position:fixed elements, use the adjusted layout viewport size getComputedStyle() r?mstange

Fairly bad webcompat issue, let's uplift for beta 6. And for ESR for the next Fennec beta build.

Comment 18

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e3a352243909

Comment 19

[Ryan VanderMeulen [:RyanVM]]

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

Comment 20

[Eliza Balazs]

Hi!
Verified as fixed on Nightly 71.0a1 (2019-09-15) and 70.0b7 with OnePlus 5T (Android 9), Huawei Honor 8 (Android 7.0), Prestigio Grace X5 (Android 4.4.2).
I will mark this as verified on Firefox 71 and Firefox 70.
Thanks!

Comment 21

[Laurentiu Apahidean]

I have tested the issue on Beta 68.2b3 using a Samsung Galaxy S9 (Android 8.0.0) and the issue no longer occurs, I will mark it accordingly.