Closed Bug 1782339 Opened 2 years ago Closed 2 years ago

incorrectly hit overscroll bounce makes it impossible to scroll page fully

Categories

(Core :: Panning and Zooming, defect)

Product:
Core
Component:
Panning and Zooming
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
106 Branch
Tracking Flags:
Tracking Status
firefox-esr91 --- unaffected
firefox-esr102 --- wontfix
firefox103 --- wontfix
firefox104 --- wontfix
firefox105 --- verified
firefox106 --- verified

People

(Reporter: tnikkel, Assigned: rzvncj)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Log file with MOZ_LOG="apz.controller:5,apz.axis:5"
362.37 KB, text/plain
Details
Bug 1782339 - Incorrectly hit overscroll bounce makes it impossible to scroll page fully. r?botond
48 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-beta+
Details | Review

Load https://archive.ph/uE3pB
Pinch zoom in a bit. Pan down on the page in short small increments.
If it doesn't happen then pan up in short small increments and then pan down again in short small increments. If it doesn't happen change your pinch zoom slightly.

It doesn't take too long for me to hit a situation where I hit an overscroll bounce in the middle of the page and am unable to keep scrolling the page. Even if I pause for a long time and try scrolling again in the same direction. Even if I scroll left/right and then try scrolling down again after that.

I've reproduced on both a retina (2x) and regular (1x) screen.

Similar to bug 1720240 but worse.

Bisected to bug 1659124.

Regressed by: 1659124

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

status-firefox103: --- → affected
status-firefox104: --- → affected
status-firefox105: --- → affected
status-firefox-esr102: --- → affected
status-firefox-esr91: --- → unaffected

:rzvncj, since you are the author of the regressor, bug 1659124, could you take a look?
For more information, please visit auto_nag documentation.

Flags: needinfo?(rzvncj)
Flags: needinfo?(rzvncj)

That was basically a patch I wrote pretty much on a paint-by-numbers process guided by [:botond] - it looked like a trivial change. I'm not terribly familiar yet with the deep design of the code, so other than reverting the patch until we figure it out I don't know what to suggest at this point.

I've also CCed [:dlrobertson], while [:botond] is away he may have other suggestions.

FWIW I've debugged this a bit, and none of the functions modified in the patch for bug 1659124 are being hit in my manual test when doing the small scroll gestures. virtual bool mozilla::layers::GenericFlingAnimation<mozilla::layers::DesktopFlingPhysics>::DoSample(mozilla::layers::FrameMetrics &, const mozilla::TimeDuration &) [FlingPhysics = mozilla::layers::DesktopFlingPhysics] is occasionally being hit for more ample gestures, but that's about it.

Got a stray mozilla::ParentLayerPoint mozilla::layers::APZCTreeManager::DispatchFling(mozilla::layers::AsyncPanZoomController *, const mozilla::layers::FlingHandoffState &) as well, but again only for more ample gestures that appear to work as expected.

Nevermind, I see IsZero() being called from paths that didn't need to be modified explicitly in the aforementioned patch.

OK, so what appears to happen when small scroll gestures don't actually scroll for me (Linux / Thinkpad laptop touchpad) is that IsZero() is true here, because adjustedDisplacement is, for example, {0, 0.0235291}. This would have caused the previous version of IsZero() to return false, since 0.0235291 > COORDINATE_EPSILON, and COORDINATE_EPSILON = 0.02f; (see this code).

However, we've now added AsyncPanZoomController::IsZero() here, and this new function divides its argument by zoom before calling layers::IsZero() on the result of the division. This is the IsZero() that's called all over AsyncPanZoomController.cpp.

So, if the zoom is, say, 1.2, then the function will compare 0.0235291 / 1.2 (which is 0.0196075) to 0.2, and IsZero() will return true.

Sorry if I've missed anything, but that at least is one way in which the code doesn't behave like it did before we've addressed bug 1659124.

I'm attaching a log obtained after exporting MOZ_LOG="apz.controller:5,apz.axis:5" and a couple of added logs of my own in the code. Please check out, searching in reverse, from the end of the file, the first occurences of IsZero(adjustedDisplacement): 1, e.g.:

[Parent 37768: Main Thread]: D/apz.controller 7ff531e6fe00 displacement: {0.000000, 46.444458}
[Parent 37768: Main Thread]: D/apz.axis 7ff531e6fe00|Y has overscrolled, clearing velocity
[Parent 37768: Main Thread]: D/apz.controller 7ff531e6fe00 IsZero(adjustedDisplacement): 1, Metrics().GetZoom() != CSSToParentLayerScale(0):
 1
[Parent 37768: Main Thread]: D/apz.controller 7ff531e6fe00 adjustedDisplacement: {0.000000, 0.034302}
[Parent 37768: Main Thread]: D/apz.controller 7ff531e6fe00 taking overscroll during panning
[Parent 37768: Main Thread]: D/apz.axis 7ff531e6fe00|Y changed overscroll amount to 43.362114
[Parent 37768: Main Thread]: D/apz.axis 7ff531e6fe00|Y got position 1144.626343
[Parent 37768: Main Thread]: D/apz.axis 7ff531e6fe00|Y velocity from tracker is 0.152414
[Parent 37768: WRSceneBuilder#1]: V/apz.controller 7ff53f2bb300 NotifyLayersUpdated short-circuit

The fact that we hit this line makes me wonder if there is a bug in Axis::AdjustDisplacement or if our frame metrics calculation is off:

[Parent 37768: Main Thread]: D/apz.axis 7ff531e6fe00|Y has overscrolled, clearing velocity

In my testing a displacement of 46 shouldn't cause the composition bounds to extend beyond the page length. Could GetExpandedScrollableRect be more or less returning the composition bounds, then causing GetPageEnd here think we're hitting the bottom of the page?

(In reply to Dan Robertson (:dlrobertson) from comment #10)

The fact that we hit this line makes me wonder if there is a bug in Axis::AdjustDisplacement or if our frame metrics calculation is off:

I haven't quite followed this problem so I might be misunderstanding it, anyway, I'd say it's a kinda bug in Axis::AdjustDisplacement, or maybe we can call it a problem in the call site in AttemptScroll. I mean, we'd also need to use IsZero() for overscroll value to tell whether it would be visible overscroll effects or not.

The code I meant is here;

aStartPoint = aEndPoint + overscroll

(In reply to Hiroyuki Ikezoe (:hiro) from comment #12)

The code I meant is here;

aStartPoint = aEndPoint + overscroll

Isn't this check sufficient?

EDIT: Or I guess you mean overscroll should be checked before assigning the output parameter aStartPoint?

(In reply to Razvan Cojocaru from comment #13)

(In reply to Hiroyuki Ikezoe (:hiro) from comment #12)

The code I meant is here;

aStartPoint = aEndPoint + overscroll

Isn't this check sufficient?

EDIT: Or I guess you mean overscroll should be checked before assigning the output parameter aStartPoint?

Hmm yeah. I am not sure. But given that we observe overscrolling, it looks like the IsZero(overscroll) returns false in the case?

(In reply to Hiroyuki Ikezoe (:hiro) from comment #14)

(In reply to Razvan Cojocaru from comment #13)

(In reply to Hiroyuki Ikezoe (:hiro) from comment #12)

The code I meant is here;

aStartPoint = aEndPoint + overscroll

Isn't this check sufficient?

EDIT: Or I guess you mean overscroll should be checked before assigning the output parameter aStartPoint?

Hmm yeah. I am not sure. But given that we observe overscrolling, it looks like the IsZero(overscroll) returns false in the case?

That's right, IsZero(overscroll) is indeed false:

[Parent 22549: Main Thread]: D/apz.controller 7f0d002de100(subframe scrollId=5): got a pan-pan in state PANNING_LOCKED_Y
[Parent 22549: Main Thread]: D/apz.controller 7f0d002de100 displacement: {0.000000, 76.229553}
[Parent 22549: Main Thread]: D/apz.axis 7f0d002de100|Y has overscrolled, clearing velocity
[Parent 22549: Main Thread]: D/apz.controller 7f0d002de100 IsZero(adjustedDisplacement): 1, Metrics().GetZoom() != CSSToParentLayerScale(0):
 1
[Parent 22549: Main Thread]: D/apz.controller 7f0d002de100 adjustedDisplacement: {0.000000, 0.026428}
[Parent 22549: Main Thread]: D/apz.controller 7f0d002de100 IsZero(overscroll): 0
[Parent 22549: Main Thread]: D/apz.controller 7f0d002de100 taking overscroll during panning
[Parent 22549: Main Thread]: D/apz.axis 7f0d002de100|Y changed overscroll amount to 28.757048
[Parent 22549: Main Thread]: D/apz.axis 7f0d002de100|Y got position 908.035156
[Parent 22549: Main Thread]: D/apz.axis 7f0d002de100|Y velocity from tracker is 0.237851
[Parent 22549: Compositor]: V/apz.controller Calculated displayport as (x=-53.7326, y=-66.4277, w=537.326, h=185.997) from velocity (0,0) zo
oming 0 paint time 50.000000 metrics:{ [cb=(x=0, y=0, w=728, h=90)] [sr=(x=0, y=0, w=728, h=90)] [s=(0,0)] [dp=(x=0, y=0, w=728, h=90)] [rcs
=(429.861 x 53.1421)] [v=(x=0, y=0, w=728, h=90)] [z=(ld=1.000 r=1.000 cr=1 z=1 t=1.69357 )] [u=(0 5)] scrollId=6 }

(In reply to Dan Robertson (:dlrobertson) from comment #10)

The fact that we hit this line makes me wonder if there is a bug in Axis::AdjustDisplacement or if our frame metrics calculation is off:

[Parent 37768: Main Thread]: D/apz.axis 7ff531e6fe00|Y has overscrolled, clearing velocity

In my testing a displacement of 46 shouldn't cause the composition bounds to extend beyond the page length. Could GetExpandedScrollableRect be more or less returning the composition bounds, then causing GetPageEnd here think we're hitting the bottom of the page?

Here's a piece of my new log looking at variables in GetExpandedScrollableRect() (the full log is available if you want it):

[Parent 34129: Main Thread]: D/apz.controller 7fe2249e3e00(subframe scrollId=4): got a pan-pan in state PANNING_LOCKED_Y
[Parent 34129: Main Thread]: D/apz.controller 7fe2249e3e00 displacement: {0.000000, 1242.279175}
[Parent 34129: Main Thread]: D/apz.frame GetExpandedScrollableRect: scrollableRect: {1024.000000, 12852.900391}
[Parent 34129: Main Thread]: D/apz.frame GetExpandedScrollableRect: compSize: {1024.000000, 11949.833008}
[Parent 34129: Main Thread]: D/apz.frame GetExpandedScrollableRect: scrollableRect: {1024.000000, 12852.900391}
[Parent 34129: Main Thread]: D/apz.frame GetExpandedScrollableRect: compSize: {1024.000000, 11949.833008}
[Parent 34129: Main Thread]: D/apz.frame GetExpandedScrollableRect: scrollableRect: {1024.000000, 12852.900391}
[Parent 34129: Main Thread]: D/apz.frame GetExpandedScrollableRect: compSize: {1024.000000, 11949.833008}
[Parent 34129: Main Thread]: D/apz.frame GetExpandedScrollableRect: scrollableRect: {1024.000000, 12852.900391}
[Parent 34129: Main Thread]: D/apz.frame GetExpandedScrollableRect: compSize: {1024.000000, 11949.833008}
[Parent 34129: Main Thread]: D/apz.frame GetExpandedScrollableRect: scrollableRect: {1024.000000, 12852.900391}
[Parent 34129: Main Thread]: D/apz.frame GetExpandedScrollableRect: compSize: {1024.000000, 11949.833008}
[Parent 34129: Main Thread]: D/apz.axis 7fe2249e3e00|Y has overscrolled, clearing velocity
[Parent 34129: Main Thread]: D/apz.controller 7fe2249e3e00 IsZero(adjustedDisplacement): 1, Metrics().GetZoom() != CSSToParentLayerScale(0): 1
[Parent 34129: Main Thread]: D/apz.controller 7fe2249e3e00 adjustedDisplacement: {0.000000, 0.029175}
[Parent 34129: Main Thread]: D/apz.controller 7fe2249e3e00 IsZero(overscroll): 0
[Parent 34129: Main Thread]: D/apz.controller 7fe2249e3e00 taking overscroll during panning
Severity: -- → S3

I can reproduce this on my Linux laptop.

An important piece of context here that wasn't clear to me until I tried the STR myself, is that the page contains two scrollable elements: the root, and a subframe which contains the actual article content (and which, strangely, has a super tall viewport -- almost the entire length of the article -- and then just a few hundred pixels or so of scroll range).

Looking at a fragment from the log in comment 9:

[Parent 37768: Main Thread]: D/apz.controller 7ff531e6fe00(subframe scrollId=4): got a pan-pan in state PANNING_LOCKED_Y
...
[Parent 37768: Main Thread]: D/apz.controller 7ff531e6fe00 displacement: {0.000000, 46.444458}
[Parent 37768: Main Thread]: D/apz.axis 7ff531e6fe00|Y has overscrolled, clearing velocity
[Parent 37768: Main Thread]: D/apz.controller 7ff531e6fe00 IsZero(adjustedDisplacement): 1, Metrics().GetZoom() != CSSToParentLayerScale(0): 1
[Parent 37768: Main Thread]: D/apz.controller 7ff531e6fe00 adjustedDisplacement: {0.000000, 0.034302}
[Parent 37768: Main Thread]: D/apz.controller 7ff531e6fe00 taking overscroll during panning

On the first line, 7ff531e6fe00(subframe scrollId=4) tells us it is the subframe's APZC whose AttemptScroll() is being called and printing the lines below.

displacement: {0.000000, 46.444458} and adjustedDisplacement: {0.000000, 0.034302} tell us that the subframe is close to the end of its scroll range (there's only 0.03 pixels of room left to scroll, but the pan gesture event has a delta of 46.4 pixels).

taking overscroll during panning tells us that the subframe is going into overscroll.

However, before overscrolling the subframe, we should first be trying to hand off the remaining delta to the parent (root) APZC and give that a chance to consume it instead. That should happen in this block. I think the next step to investigate is why this handoff isn't happening.

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

However, before overscrolling the subframe, we should first be trying to hand off the remaining delta to the parent (root) APZC and give that a chance to consume it instead. That should happen in this block. I think the next step to investigate is why this handoff isn't happening.

Well, for this code:

2312 bool AsyncPanZoomController::AllowScrollHandoffInCurrentBlock() const {
2313   bool result = mInputQueue->AllowScrollHandoff();
2314 
2315   APZC_LOG("%p StaticPrefs::apz_allow_immediate_handoff(): %d", this,
2316       StaticPrefs::apz_allow_immediate_handoff());
2317 
2318   if (!StaticPrefs::apz_allow_immediate_handoff()) {
2319     APZC_LOG("%p GetCurrentInputBlock(): %p", this, GetCurrentInputBlock());
2320 
2321     if (InputBlockState* currentBlock = GetCurrentInputBlock()) {
2322       // Do not allow handoff beyond the first APZC to scroll.
2323       APZC_LOG("%p currentBlock->GetScrolledApzc(): %p", this, currentBlock->GetScrolledApzc());
2324       if (currentBlock->GetScrolledApzc() == this) {
2325         result = false;
2326         APZC_LOG("%p dropping handoff; AllowImmediateHandoff=false\n", this);
2327       }
2328     }
2329   }
2330   return result;
2331 }

I'm now getting this output (that suggests that mInputQueue->AllowScrollHandoff() is false):

[Parent 21714: Main Thread]: D/apz.controller 7f4cef7be500 displacement: {0.000000, 16.679626}
[Parent 21714: Main Thread]: D/apz.frame GetExpandedScrollableRect: scrollableRect: {1024.000000, 12853.183594}
[Parent 21714: Main Thread]: D/apz.frame GetExpandedScrollableRect: compSize: {1024.000000, 11949.833008}
[Parent 21714: Main Thread]: D/apz.frame GetExpandedScrollableRect: scrollableRect: {1024.000000, 12853.183594}
[Parent 21714: Main Thread]: D/apz.frame GetExpandedScrollableRect: compSize: {1024.000000, 11949.833008}
[Parent 21714: Main Thread]: D/apz.frame GetExpandedScrollableRect: scrollableRect: {1024.000000, 12853.183594}
[Parent 21714: Main Thread]: D/apz.frame GetExpandedScrollableRect: compSize: {1024.000000, 11949.833008}
[Parent 21714: Main Thread]: D/apz.frame GetExpandedScrollableRect: scrollableRect: {1024.000000, 12853.183594}
[Parent 21714: Main Thread]: D/apz.frame GetExpandedScrollableRect: compSize: {1024.000000, 11949.833008}
[Parent 21714: Main Thread]: D/apz.frame GetExpandedScrollableRect: scrollableRect: {1024.000000, 12853.183594}
[Parent 21714: Main Thread]: D/apz.frame GetExpandedScrollableRect: compSize: {1024.000000, 11949.833008}
[Parent 21714: Main Thread]: D/apz.axis 7f4cef7be500|Y has overscrolled, clearing velocity
[Parent 21714: Main Thread]: D/apz.controller 7f4cef7be500 IsZero(adjustedDisplacement): 1, Metrics().GetZoom() != CSSToParentLayerScale(0):
 1
[Parent 21714: Main Thread]: D/apz.controller 7f4cef7be500 adjustedDisplacement: {0.000000, 0.025330}
[Parent 21714: Main Thread]: D/apz.controller 7f4cef7be500 IsZero(overscroll): 0
[Parent 21714: Main Thread]: D/apz.inputqueue GetCurrentPanGestureBlock()
[Parent 21714: Main Thread]: D/apz.controller 7f4cef7be500 StaticPrefs::apz_allow_immediate_handoff(): 0
[Parent 21714: Main Thread]: D/apz.controller 7f4cef7be500 GetCurrentInputBlock(): 7f4ce48253a0
[Parent 21714: Main Thread]: D/apz.controller 7f4cef7be500 currentBlock->GetScrolledApzc(): 0
[Parent 21714: Main Thread]: D/apz.controller 7f4cef7be500 AllowScrollHandoffInCurrentBlock() is false
[Parent 21714: Main Thread]: D/apz.controller 7f4cef7be500 taking overscroll during panning

The interesting part is [Parent 21714: Main Thread]: D/apz.inputqueue GetCurrentPanGestureBlock(), which corresponds to this path:

 656 bool InputQueue::AllowScrollHandoff() const {
 657   if (GetCurrentWheelBlock()) {
 658     INPQ_LOG("GetCurrentWheelBlock()");
 659     return GetCurrentWheelBlock()->AllowScrollHandoff();
 660   }
 661   if (GetCurrentPanGestureBlock()) {
 662     INPQ_LOG("GetCurrentPanGestureBlock()");
 663     return GetCurrentPanGestureBlock()->AllowScrollHandoff();
 664   }
 665   if (GetCurrentKeyboardBlock()) {
 666     INPQ_LOG("GetCurrentKeyboardBlock()");
 667     return GetCurrentKeyboardBlock()->AllowScrollHandoff();
 668   }
 669   return true;
 670 }

Following this further, we get to:

bool PanGestureBlockState::AllowScrollHandoff() const { return false; }

Ok, it looks like pan gestures are an input type for which we don't do scroll handoff within a single block.

Rather, we choose the first APZC with room to scroll (which may not be the innermost one) at the beginning of the input block in the FindFirstScrollable() call here.

The fact that the bug involves getting stuck in a place where you can't scroll further suggests that FindFirstScrollable() repeatedly returns the subframe's APZC, but AttemptScroll() repeatedly fails to actually scroll it.

Does this mean that this is, in fact, expected behaviour and not a bug? Or does the current behaviour need chaging?

It's definitely a bug. The bug is that FindFirstScrollable() and AttemptScroll() do not agree on whether the subframe has enough room to scroll to be considered "scrollable".

AttemptScroll() makes that decision using the IsZero(adjustedDisplacement) check discussed previously, which bug 1659124 caused to change from false to true in affected situations (where the remaining room to scroll is > 0.02 in ParentLayer pixels but < 0.02 in CSS pixels).

My guess is that FindFirstScrollable() does a similar check but rather than diving by the zoom, it compares a quantity in ParentLayer pixels to COORDINATE_EPSILON (0.02) directly, leading to a different answer.

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

My guess is that FindFirstScrollable() does a similar check but rather than diving by the zoom, it compares a quantity in ParentLayer pixels to COORDINATE_EPSILON (0.02) directly, leading to a different answer.

Axis::CanScroll() is what FindFirstScrollable() ultimately uses, and indeed it compares a ParentLayer coordinate.

==> We should be able to fix the bug by converting that coordinate to CSS pixels before the comparison (by dividing by the zoom, as in IsZero()).

Assignee: nobody → rzvncj
Status: NEW → ASSIGNED
Pushed by bballo@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/81e4e05408f2 Incorrectly hit overscroll bounce makes it impossible to scroll page fully. r=botond,dlrobertson
Pushed by bballo@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f47c138919bb Incorrectly hit overscroll bounce makes it impossible to scroll page fully. r=botond,dlrobertson,hiro
Flags: needinfo?(botond)
Blocks: 1787099

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

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
status-firefox106: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 106 Branch

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

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

For more information, please visit auto_nag documentation.

Flags: needinfo?(rzvncj)

I think it is, but [:botond] should know better, so I'll defer to his judgement.

Flags: needinfo?(rzvncj) → needinfo?(botond)

On the one hand, this is an annoying regression and it would be nice to have the fix one release sooner (105 rather than 106).

On the other hand, this regression has been shipping since 102, has no duplicate reports, and based on comment 25, I'm slightly worried about the regression risk of the fix itself.

I'll be less worried once bug 1787099 is fixed as well, since then everything should be consistently compared in CSS units.

So my suggestion here is to wait until bug 1787099 also lands (and has had a few days of baking on nightly), and then if we're still in time to consider a 105 uplift, consider uplifting this + bug 1787099 together.

Leaving needinfo on me for now.

Now that bug 1787099 has landed, I would suggest letting it bake on nightly until Wednesday, and if we haven't seen any problems until then, then request uplift of this bug + bug 1787099 together.

Comment on attachment 9290591 [details]
Bug 1782339 - Incorrectly hit overscroll bounce makes it impossible to scroll page fully. r?botond

Beta/Release Uplift Approval Request

  • User impact if declined: On some pages, scrolling via touchpad can sometimes get stuck in a position where you can't scroll further even though there is room.

This has so far only been observed on certain pages where you're pinch-zoomed in, but could in principle affect other pages we're not aware of.

  • 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: Bug 1787099 (with this one sequenced first)
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Especially if uplifted together with bug 1787099, this patch moves the relevant code areas in a direction of greater consistency (in the units we use to make comparisons like "are we close to the edge"), and regressions such as this one have been caused by lack of consistency.
  • String changes made/needed:
  • Is Android affected?: No
Flags: needinfo?(botond)
Attachment #9290591 - Flags: approval-mozilla-beta?

Comment on attachment 9290591 [details]
Bug 1782339 - Incorrectly hit overscroll bounce makes it impossible to scroll page fully. r?botond

Approved for 105.0b6.

Attachment #9290591 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

Reproduced the issue with Firefox 105.0a1 (2022-07-09) on macOS 11 with a touchpad. After following the steps from comment 0 the page cannot be scrolled anymore in the same direction.
I can no longer reproduce the initial issue with Firefox 105.0b7 (20220904185841) and 106.0a1 (20220904213226) on Windows 10x64 and macOS 11 with touchpad. However, I can still see that sometimes the scroll is suddenly stopping while panning down on the page in short small increments. On macOS, the scroll bounces up a little. On Windows, the page does not scroll sometimes on the first attempt only after panning again. Attached a screen recording made on macOS (link). Should we file a new issue for this and close this one as verified or reopen this one? Thank you!

Flags: needinfo?(rzvncj)

Hello, and thanks for testing! I'd say that with small enough increments and a large enough zoom factor what you are seeing is normal, judging from what I've learned from the code, however I am merely a volunteer contributor. IMHO this should probably be a separate issue, but [:botond] (info requested) should really be the judge of that.

Flags: needinfo?(rzvncj) → needinfo?(botond)

Alexandru, would you mind trying to see whether it's a regression or not, and trying to find out the regression ranges? The symptoms look pretty same but the underlying problems may be different.

Flags: needinfo?(alexandru.trif)

The page has two nested scroll frames, that leads to some not-so-intuitive scrolling behaviours which might be what you are seeing.

Thank you everyone for your input.

(In reply to Hiroyuki Ikezoe (:hiro) from comment #38)

Alexandru, would you mind trying to see whether it's a regression or not, and trying to find out the regression ranges? The symptoms look pretty same but the underlying problems may be different.

Sure thing. I looked for a regression range on macOS 11 and it seems that I can reproduce the issue with Firefox 71.0a1 (2019-09-02) if apz.allow_zooming is set on true. Older builds like the one from (2019-08-02) cannot load the site anymore so I think it's safe to assume that this is not a regression.

I will set the flags as verified based on comment 36 given the fact that this happens on older builds and has nothing to do with this issue. If a new issue is required for this behavior please let me know.

status-firefox105: fixed → verified
status-firefox106: fixed → verified
Flags: needinfo?(alexandru.trif)

Yeah, filing a new bug would be super-helpful for us. I mean, I think it's a bug. :) Thanks!

Flags: needinfo?(alexandru.trif)

I think the behaviour described in comment 36 and shown in the linked screen capture is intended behaviour (if a bit unfortunate) given the odd page structure, where the article content is contained in a subframe with a very tall viewport and short scroll range.

When you start scrolling down, the subframe will scroll down first (by the small amount in its scroll range), and then overscroll. The next gesture will scroll the root and allow you to keep scrolling down.

Every time you switch direction, this process repeats. For example, if the subframe is scrolled to the bottom (which will basically happen any time your last scroll was downward), the first upward scroll will scroll the subframe up and overscroll it, and subsequent upward scrolls will scroll the root up.

Flags: needinfo?(botond)

I have filled bug 1789610 for tracking purposes. Thank you!

Flags: needinfo?(alexandru.trif)
Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: