1854010 - Unable to close cookie popup on portal.librus.pl due to position:sticky element not displayed

Status: VERIFIED FIXED

(Core :: Web Painting, defect)

Description

[Ksenia Berezina [:ksenia]]

Attachment: 127190.html

We've received a report in https://github.com/webcompat/web-bugs/issues/127190 where a user is unable to close a cookie popup. The issue is reproducible only on the actual device and not on RDM.

STR:
Visit https://portal.librus.pl/ in Firefox release or Nightly on mobile (on an actual phone). Wait for the cookie popup to appear and scroll down inside the popup.

Expected:
Buttons to accept/deny cookies appear

Actual:
Buttons are not visible

I've attached a slightly reduced test case. It seems that transform property that's applied on .modal-dialog is causing this (if I add transform:none, the sticky footer with buttons appears).

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Attachment: Further reduced test-case.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

I tried to run mozregression on this, but the oldest release I found for gve (111) still shows the issue.

Some questions (sorry for the ni? spam, but this seems like something that could affect various sites and seems like a pretty bad bug):

• Jamie, do you happen to know if we could bisect this further than the above? It seems like that'd be useful.
• Hiro, Botond, have you seen something like this? Any idea how to maybe reproduce this on desktop or any pref that could have an effect here? At first I suspected the dynamic toolbar stuff, but actually this reproduces with fixed toolbar and also with the toolbar at the top so...

In any case layout seems correct, this seems like a web painting / webrender issue (the fact that the transform causes it is a strong hint), so moving off layout.

Comment 3

[Jamie Nicol [:jnicol]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)

I tried to run mozregression on this, but the oldest release I found for gve (111) still shows the issue.

• Jamie, do you happen to know if we could bisect this further than the above? It seems like that'd be useful.

I see the same. This seems like a bug to me. Usually mozregression is able to find builds up to a year old. I've long since been frustrated it cannot go even back further for mozilla-central builds like on desktop, but certainly up to a year should work.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

It looks like the difference comes from nsDisplayOwnLayer::ShouldFixedAndStickyContentGetAnimationIds. If I do use Frame->PresContext()->IsRootContentDocumentCrossProcess() for desktop I can see the same result on Android. I haven't dug into details.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

For posterity and for references, how I could reach to the function is I did rg ANDROID layout/ and I initially checked nsGfxScrollFrame.cpp, there's nothing suspicious and then I checked nsDisplayList.cpp. So that was just fortunate.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1753019

:dlrobertson, since you are the author of the regressor, bug 1753019, could you take a look?

For more information, please visit BugBot documentation.

Comment 7

[Botond Ballo [:botond]]

(In reply to Jamie Nicol [:jnicol] from comment #3)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)

I tried to run mozregression on this, but the oldest release I found for gve (111) still shows the issue.

• Jamie, do you happen to know if we could bisect this further than the above? It seems like that'd be useful.

I see the same. This seems like a bug to me. Usually mozregression is able to find builds up to a year old. I've long since been frustrated it cannot go even back further for mozilla-central builds like on desktop, but certainly up to a year should work.

For reference, this is bug 1849082, and the root cause seems to be bug 1841685 ("files matching geckoview_example.apk were deleted [presumably accidentally] during a clean up of files in S3"). The cutoff date seems to be 2023-02-13.

A fallback option for regressions older than that is to use mozregression's new support for Fenix nightlies (bug 1556042) to get a nightly-granularity regression window (though this support is very recent and I haven't been able to get it to work yet, see bug 1556042 comment 19).

Comment 8

[Dan Robertson (:dlrobertson)]

I'll start digging into this as this is probably a higher priority than a good chunk of the scrollend bugs.

Comment 9

[Donal Meehan [:dmeehan]]

:dlrobertson do you intend to have anything for Fx120, likely this is too late for Fx119?

Comment 10

[Dan Robertson (:dlrobertson)]

Certainly too late for Fx119... Just now digging in. Made some progress, but I'm not super familiar with Display List things

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1753019

Comment 12

[Dan Robertson (:dlrobertson)]

Attachment: Bug 1854010 - Restrict fixed/sticky hit test info to items fixed to the root. r=hiro

Fixed and Sticky position side bits should only be necessary for fixed
or sticky position items that are fixed or stuck to the root content. As
a result, there is no need to add the fixed or sticky position hit test
info to items that are not.

Comment 13

[Dan Robertson (:dlrobertson)]

Attachment: Bug 1854010 - Add test for sticky content in a fixed position container. r=hiro

Add a reftest for sticky position content in a fixed position container.

Depends on D192272

Comment 14

[Ksenia Berezina [:ksenia]]

Attachment: 125199.html

I came across an issue that probably has the same root cause in https://github.com/webcompat/web-bugs/issues/125199.
In this case it's a filter button, and it is visible, however clicking on it has no effect (clicking at the bottom of the page makes it work, as if the button was there).
Attaching a reduced testcase in case it might be helpful.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/767972bb0fc3
Restrict fixed/sticky hit test info to items fixed to the root. r=hiro
https://hg.mozilla.org/integration/autoland/rev/262f7894aa0d
Add test for sticky content in a fixed position container. r=hiro

Comment 16

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

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

Comment 17

[Atila Butkovits]

Backed out for causing crashes at mochitest.toml.

Backout link: https://hg.mozilla.org/integration/autoland/rev/3201239bec85ca63b9cd1ea79e3a5acc21ad0f91

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=262f7894aa0df98ca9745fd2c190c2c16ef0b907&selectedTaskRun=FZFGN2NTQ5SptpMwjJYlOA.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=436127168&repo=autoland&lineNumber=2674

Comment 18

[Botond Ballo [:botond]]

Backed out for causing crashes at mochitest.toml.

This is kind of a side point, but in these log lines:

PROCESS-CRASH | MOZ_ASSERT(sideBits == SideBits::eNone) (Hit test results have side bits only for pos:fixed) [@ mozilla::layers::WRHitTester::GetAPZCAtPoint] | gfx/layers/apz/test/mochitest/mochitest.toml
PROCESS-CRASH | MOZ_ASSERT(false) (MOZ_ASSERT_UNREACHABLE: Did not assign a type to WidgetMouseEvent in MouseInput) [@ mozilla::MouseInput::ToWidgetEvent] | gfx/layers/apz/test/mochitest/mochitest.toml

Reporting "gfx/layers/apz/test/mochitest/mochitest.toml" as the location of either of these assertion failures looks like a bug (regression?) in the test harness's logging mechanism.

Comment 19

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

Upstream PR was closed without merging

Comment 20

[Dan Robertson (:dlrobertson)]

Hmm the test failure is interesting... this shouldn't cause side bits to be added for position sticky elements, but I guess I missed something.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dd7c79692c64
Restrict fixed/sticky hit test info to items fixed to the root. r=hiro
https://hg.mozilla.org/integration/autoland/rev/8e3a29705587
Add test for sticky content in a fixed position container. r=hiro

Comment 22

[Sandor Molnar[:smolnar]]

Backed out for causing mochitest plain / reftest failures

Backout link: https://hg.mozilla.org/integration/autoland/rev/ea42a7479dd7a48cb58ccf6b3b209ff9136d4953

Push with failures

Failure log -> reftest

Failure log -> mochitest #1

Failure log -> mochitest #2

Comment 23

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

Upstream PR was closed without merging

Comment 24

[Dan Robertson (:dlrobertson)]

Updated the patch to contain a fix for the failing tests.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dd6cba997d6a
Restrict fixed/sticky hit test info to items fixed to the root. r=hiro,botond
https://hg.mozilla.org/integration/autoland/rev/00fe06571a66
Add test for sticky content in a fixed position container. r=hiro

Comment 26

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/dd6cba997d6a
https://hg.mozilla.org/mozilla-central/rev/00fe06571a66

Comment 27

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

Upstream PR merged by moz-wptsync-bot

Comment 28

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:dlrobertson, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox122 to wontfix.

For more information, please visit BugBot documentation.

Comment 29

[Dan Robertson (:dlrobertson)]

Attachment: Bug 1854010 - Restrict fixed/sticky hit test info to items fixed to the root.

Fixed and Sticky position side bits should only be necessary for fixed
or sticky position items that are fixed or stuck to the root content. As
a result, there is no need to add the fixed or sticky position hit test
info to items that are not.

Original Revision: https://phabricator.services.mozilla.com/D192272

Comment 30

[Dan Robertson (:dlrobertson)]

Attachment: Bug 1854010 - Add test for sticky content in a fixed position container.

Add a reftest for sticky position content in a fixed position container.

Original Revision: https://phabricator.services.mozilla.com/D192281
Depends on D192272

Depends on D197510

Comment 31

[Phabricator Automation]

Uplift Approval Request

• Explanation of risk level: Only impacts android and only removes an additional reference frame for fixed/sticky items.
• Fix verified in Nightly: yes
• Risk associated with taking this patch: Low
• Code covered by automated testing: yes
• Steps to reproduce for manual QE testing: Navigate to https://bug1854010.bmoattachments.org/attachment.cgi?id=9356341
• User impact if declined: Users may be unable to close or see a display item on android.
• Needs manual QE test: yes
• String changes made/needed: No
• Is Android affected?: yes

Comment 32

[Phabricator Automation]

Uplift Approval Request

• Steps to reproduce for manual QE testing: See D197510.
• Code covered by automated testing: yes
• Risk associated with taking this patch: Low
• Fix verified in Nightly: yes
• Explanation of risk level: Tests
• Is Android affected?: yes
• Needs manual QE test: no
• String changes made/needed: No
• User impact if declined: Tests for D197510

Comment 33

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/da18c397d840
https://hg.mozilla.org/releases/mozilla-beta/rev/cabd1794168d

Comment 34

[Mira Lobontiu (Android QA)]

Verified as fixed on the latest Fenix Nightly 123.0a1 from 1/3 with Google Pixel 6 (Android 14).

STR followed were:

1. Visit https://portal.librus.pl/ in Firefox release or Nightly on mobile (on an actual phone).
2. Wait for the cookie popup to appear and scroll down inside the popup.

Expected:
Buttons to accept/deny cookies are displayed.

The issue is not yet fixed in Beta 122.0b4, therefor, I'll leave the qe-verify+ flag until beta 5, when we can properly verify this.

Comment 35

[Mira Lobontiu (Android QA)]

Verified as fixed also on Beta 122.0b5 with Sony Xperia Z5 Premium (Android 7.1.1), and Google Pixel 6 (Android 14).