Currently in Fennec, if the user starts a scroll from within a scrollable element that is at the end of its scroll, the scroll is handed off until an ancestor that is able to scroll is found. If the user changes direction during the scroll however, the scrollable element will start to scroll. For Fennec, once an APZ target has been selected for a scroll, it should not change for the duration of the scroll.
Do we want to change this behaviour for Fennec ("no immedate handoff" mode) only, or in general? I understood bug 1247964 comment 7 as suggesting we change it in general, but I may have misread it.
I think in general makes more sense.
Created attachment 8731400 [details] [diff] [review] 0001-Bug-1257264-When-panning-with-handoff-disabled-APZ-handoff-should-not-occur-when-panning-changes-direction-16031613-24e124d.patch
Attachment #8731400 - Flags: feedback?(botond)
Comment on attachment 8731400 [details] [diff] [review] 0001-Bug-1257264-When-panning-with-handoff-disabled-APZ-handoff-should-not-occur-when-panning-changes-direction-16031613-24e124d.patch Review of attachment 8731400 [details] [diff] [review]: ----------------------------------------------------------------- With this change, we should be able to restore the original assertion in InputBlockState::SetScrolledApzc(). r+ with comments ::: gfx/layers/apz/src/AsyncPanZoomController.cpp @@ +2330,5 @@ > // to *decrease* as the page follows your finger. > ParentLayerPoint displacement = aStartPoint - aEndPoint; > > ParentLayerPoint overscroll; // will be used outside monitor block > + bool allowScroll = gfxPrefs::APZAllowImmediateHandoff() || I'd call this variable "scrollThisApzc". I also think this deserves a comment, along the lines of: // If the direction of panning is reversed within the same input block, // a later event in the block could potentially scroll an APZC earlier // in the handoff chain, than an earlier event in the block (because // the earlier APZC was scrolled to its extent in the original direction). // If immediate handoff is disallowed, we want to disallow this (to // preserve the property that a single input block only scrolls one APZC), // so we skip the earlier the APZC.
Attachment #8731400 - Flags: feedback?(botond) → feedback+
Comment on attachment 8731400 [details] [diff] [review] 0001-Bug-1257264-When-panning-with-handoff-disabled-APZ-handoff-should-not-occur-when-panning-changes-direction-16031613-24e124d.patch Meant this as an r+ (with comments).
Attachment #8731400 - Flags: feedback+ → review+
(In reply to Kartikaya Gupta (email:email@example.com) from comment #2) > I think in general makes more sense. Randall's patch implements it for Fennec. We can extend this to the general case with a bit of extra work: - Track InputBlockState::mScrolledApzc regardless of the value of gfxPrefs::APZAllowImmediateHandoff(). - Have the restriction on handoff added in Randall's patch apply regardless of the value of gfxPrefs::APZAllowImmediateHandoff(). - Keep other restrictions on handoff (those added in bug 1230552) conditioned on !gfxPrefs::APZAllowImmediateHandoff(). We can do this in a follow-up patch (in this bug, or in another bug, I'm fine either way).
Created attachment 8731468 [details] [diff] [review] 0001-Bug-1257264-When-panning-with-handoff-disabled-APZ-handoff-should-not-occur-when-panning-changes-direction-r-botond-16031615-b26480d.patch Address comments. Carry forward r+ from :botond.
Attachment #8731400 - Attachment is obsolete: true
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox48: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.