Closed Bug 1705280 Opened 22 days ago Closed 10 days ago

Overscroll handoff doesn't work on where pan gestures happen on a child scroll container and the scroll position of the parent scroll container is at the edge

Categories

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

defect

Tracking

()

VERIFIED FIXED
90 Branch
Fission Milestone M7a
Tracking Status
firefox89 --- verified
firefox90 --- verified

People

(Reporter: hiro, Assigned: hiro)

References

(Blocks 2 open bugs, Regressed 1 open bug)

Details

(Whiteboard: [proton-uplift])

Attachments

(3 files)

Attached file an example

This is the issue I've been seeing on 9to5mac.com (bug 1702988), and I believe overflow:hidden document in iframe is commonly used (for ads mainly?), so this should be fixed until we release Fission.

Anyway, the issue was a bit complicated than I thought initially. Attaching case is an example to see the issue with Fission. A STR is just

  1. Overscroll upwards by panning on the iframe when the root scroll position is (0, 0)

If the root scroll position is not at (0, 0), the issue doesn't happen, i.e.

  1. Scroll down a bit
  2. Overscroll upwards by panning on the iframe

Then, the issue doesn't happen.

So a problem is here in OverscrollHandoffChain::FindFirstScrollable

  for (size_t i = 0; i < Length(); i++) {                                       
    if (mChain[i]->CanScroll(aInput)) {                                         
      return mChain[i];                                                         
    }                                                                           
                                                                                
    *aOutAllowedScrollDirections &= mChain[i]->GetAllowedHandoffDirections();   
    if (aOutAllowedScrollDirections->isEmpty()) {                               
      return nullptr;                                                           
    }                                                                           
  }

With the upwards pan gestures at (0, 0) position in the root scroller, the mChain[i]->CanScroll(aInput) will never return true, so that we will never handoff to the root one.

I wonder whether the root content APZC could be the last resort for overscroll handoff.

How does it work with non-Fission?

With non-Fission, the iframe's overflow:hidden document doesn't have any APZC (If I read properly a log output by MOZ_LOG=apz:manager).

See Also: → 1705462
Duplicate of this bug: 1705462

Now I am convinced the root content APZC should be the last one for pan gestures with checking overscroll-behavior is either "auto" or "contains".

Assignee: nobody → hikezoe.birchill
Status: NEW → ASSIGNED
Fission Milestone: --- → M7a

Looks like I did unintentionally fix non fission cases, I just realized it when I wrote a gtest.

In this case, there is a child scroll container which is vertically scrollable, then if the root scroll position is at top, and if the child scroll container's position is also at top, then scroll handoff doesn't work, I mean panning on the child scroll container doesn't cause overscrolling on the root scroll container.

Severity: -- → S3
Priority: -- → P2

To be precise the root APZC should actually be scrollable in the opposed
directions of the given input, it's checked in
AsyncPanZoomController::GetOverscrollableDirections.

Note that I gave up writing a gtest which will be working with WebRender hit test code since it requires more work than I thought.

Pushed by hikezoe.birchill@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/86a1362f15e6
Allow overscroll handoff to the root content APZC on pan gestures even if the root APZC is not scrollable to the given input directions. r=botond

Backed out changeset 86a1362f15e6 (Bug 1705280) for causing Android Gtest failures in APZCOverscrollTesterForLayersOnly.

Push with failures

Failure log

Backout link

Flags: needinfo?(hikezoe.birchill)

Forgot to enclose the gtest with "#ifndef MOZ_WIDGET_ANDROID".

Flags: needinfo?(hikezoe.birchill)
Pushed by hikezoe.birchill@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3b9fbea8a60e
Allow overscroll handoff to the root content APZC on pan gestures even if the root APZC is not scrollable to the given input directions. r=botond
Status: ASSIGNED → RESOLVED
Closed: 10 days ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch
Summary: Overscroll handoff doesn't work on overflow:hidden OOP iframe documents → Overscroll handoff doesn't work on where pan gestures happen on a child scroll container and the scroll position of the parent scroll container is at the edge

Comment on attachment 9217602 [details]
Bug 1705280 - Allow overscroll handoff to the root content APZC on pan gestures even if the root APZC is not scrollable to the given input directions. r?botond

Beta/Release Uplift Approval Request

  • User impact if declined: [Required for MR1 / Proton] overscroll effect doesn't happen at all
  • 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): The change is effective only for overscroll state with pan gesture.
  • String changes made/needed: none
Attachment #9217602 - Flags: approval-mozilla-beta?
Whiteboard: [proton-uplift]
Duplicate of this bug: 1702988

Comment on attachment 9217602 [details]
Bug 1705280 - Allow overscroll handoff to the root content APZC on pan gestures even if the root APZC is not scrollable to the given input directions. r?botond

Approved for 89 beta 6, thanks.

Attachment #9217602 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Regressions: 1709723

Verified fixed on the latest Firefox Beta 89.0b8 (20210504185920) and Nightly 90.0a1 (20210505215208).

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.