Closed Bug 1565512 Opened 1 year ago Closed 1 year ago

Hit-testing is off on GeckoView when zooming on the reftest analyzer.

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox-esr60 --- wontfix
firefox-esr68 --- wontfix
firefox68 --- wontfix
firefox69 --- fixed
firefox70 --- fixed

People

(Reporter: emilio, Assigned: botond)

References

(Regression, )

Details

(Keywords: regression)

Attachments

(2 files)

STR:

  • Open a reftest-analyzer url like this one on Fenix.

  • Zoom and move to the right, to try to hit the "reference" checkbox.

  • Note how nothing happens.

  • Note that you're hitting / selecting the text to the left outside of the viewport, rather than the place where you're tapping.

Also reproducible in GVE.

This is not a new regression. I can reproduce this in GVE builds going as far back as 2018-08-20. (Prior to that, the page was laid out differently when zoomed in due to the lack of bug 1465616, and the STR aren't really applicable.)

Also reproducible on Fennec.

Suggested next steps:

  • See if the bug is reproducible on a page structure where the targeted element is not inside a position:fixed element.
  • If so, get a Fennec regression range with that modified page. (Because it's not position:fixed, the lack of bug 1465616 will not interfere with the STR, so a proper regression range can hopefully be found.) I say Fennec not GVE because the bug affects both, and GVE builds only go as far back as ~2018-07.

(I'm at a standards meeting next week, I plan to do the above when I get back if no one beats me to it.)

As far as I can tell the bug is not reproducible if the element is not inside a position:fixed element. So this is a regression from bug 1465616, in that presumably some APZ hit-testing code needs to be correspondingly updated.

Regressed by: 1465616
Attached file Reduced testcase

Here is a reduced testcase. The presence of an SVG image (even if empty) seems to be important, as well as the fact that the fixed element is also overflow: auto.

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

The presence of an SVG image (even if empty) seems to be important, as well as the fact that the fixed element is also overflow: auto.

What makes a difference here is whether the radio button is inside a scrollable subframe that gets layerized. The overflow: auto creates the subframe, the SVG image just creates enough content to make it actually scrollable (it doesn't need to be an SVG, any content that's sufficiently large will do).

If the subframe does get layerized, APZ hit testing will choose the subframe's guid as the target guid (which is correct). Then, in ApplyCallbackTransform, we map the target guid to a frame, and walk up the frame tree, accumulating callback transforms as we go.

If our starting point is the subframe created by the fixed content, the frame tree walk never encounters the RCD-RSF, and we never apply its callback transform. In cases where you're zoomed in and the visual viewport is scrolled away from the layout viewport origin, applying the RCD-RSF callback transform is important because it's what contains the offset between the two viewports.

(By contrast, if the subframe isn't layerized, hit testing falls back to the RCD-RSF's guid, so we start the frame tree walk from the RCD-RSF and include its callback transform.)

The reason bug 1465616 makes a difference is that it makes fixed content subject to the offset between the visual and layout viewports, which it wasn't before.

Assignee: nobody → botond
Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ea97d944e1a7
Ensure the RCD-RSF callback transform is applied for events targeting fixed content too. r=tnikkel
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Duplicate of this bug: 1542832
Duplicate of this bug: 1546222

As this fixes two bugs affecting Reddit as well (bug 1542832 and bug 1546222), we should get this uplifted.

Comment on attachment 9082809 [details]
Bug 1565512 - Ensure the RCD-RSF callback transform is applied for events targeting fixed content too. r=tnikkel

Beta/Release Uplift Approval Request

  • User impact if declined: When pages with position:fixed elements are zoomed in on mobile, interacting with the fixed element can fail in various ways, including taps registering at the wrong location (the original STR of this bug), text selection activating at the wrong location (bug 1542832), and clipped rendering (bug 1546222). The latter two issues affect Reddit when viewed in desktop mode.
  • Is this code covered by automated tests?: Unknown
  • 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): This is a small change that only affects a fairly specific scenario on mobile (position:fixed content, we're zoomed in, and the visual viewport is scrolled away from the layout viewport origin).
  • String changes made/needed:
Attachment #9082809 - Flags: approval-mozilla-beta?

Setting status-firefox69=affected because we'd like to uplift to GV Beta (69).
Setting status-firefox-esr68=wontfix because I assume these layout changes are too risky to uplift to ESR 68 for Fennec. ESR 68 is the last major version of Fennec. There will be no Fennec >= 69.

Comment on attachment 9082809 [details]
Bug 1565512 - Ensure the RCD-RSF callback transform is applied for events targeting fixed content too. r=tnikkel

Fixes a rendering issue on mobile affecting popular sites. Approved for 69.0b14.

Attachment #9082809 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Timothy, how would you feel about a potential 68esr uplift for this, in terms of risk?

Flags: needinfo?(tnikkel)

It's been a bug since firefox 63 and we only noticed it last month? That doesn't seem like it would qualify as a bad enough bug for uplift to esr?

Flags: needinfo?(tnikkel)

(In reply to Timothy Nikkel (:tnikkel) from comment #19)

It's been a bug since firefox 63 and we only noticed it last month?

The duplicate bugs were filed in April, and affect Reddit (albeit in desktop mode only).

I'm okay with the risk if it's severe enough to fix.

Comment on attachment 9082809 [details]
Bug 1565512 - Ensure the RCD-RSF callback transform is applied for events targeting fixed content too. r=tnikkel

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Low-risk fix for STR that affect a popular site (Reddit)
  • User impact if declined: Reddit threads, when viewed in desktop mode on a mobile device and pinch-zoomed in, exhibit rendering problems (rendering can get clipped) and text selection problems (text is selected at a location different from the point of touch). Other websites that use position:fixed for their main content the way Reddit does, can potentially experience similar problems.
  • Fix Landed on Version: 70, uplifted to 69
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Small change that only affects a fairly specific scenario (mobile, position:fixed content, zoomed in, visual viewport scrolled away from layout viewport origin).
  • String or UUID changes made by this patch: none
Attachment #9082809 - Flags: approval-mozilla-esr68?

Comment on attachment 9082809 [details]
Bug 1565512 - Ensure the RCD-RSF callback transform is applied for events targeting fixed content too. r=tnikkel

I'm not convinced this is worth the risk of possibly breaking something else, sorry...

Attachment #9082809 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68-
You need to log in before you can comment on or make changes to this bug.