Closed Bug 1385468 Opened 8 years ago Closed 8 years ago

Notify browser.xml when APZ cancels an AutoscrollAnimation

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: botond, Assigned: botond)

References

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file)

APZ will cancel a running AutoscrollAnimation when notified by browser.xml via AsyncPanZoomController::StopAutoscroll(). However, it can also cancel a running AutoscrollAnimation for other reasons (basically, any time CancelAnimation() is called), so it should notify browser.xml in those cases. Otherwise, we can get into a state where APZ thinks autoscrolling is stopped (so it stops scrolling), but browser.xml doesn't, so the popup is still shown.
Blocks: 1385463
Comment on attachment 8892223 [details] Bug 1385468 - Notify browser.xml when APZ cancels an AutoscrollAnimation. https://reviewboard.mozilla.org/r/163182/#review169222 ::: toolkit/content/widgets/browser.xml:1180 (Diff revision 2) > <body> > <![CDATA[ > - if (aTopic != "browser:purge-session-history") > + if (aTopic == "browser:purge-session-history") { > - return; > - > - this.purgeSessionHistory(); > + this.purgeSessionHistory(); Looks like there's a tab character added here? ::: toolkit/content/widgets/browser.xml:1252 (Diff revision 2) > if (this.isRemoteBrowser && this._autoScrollScrollId != null) { > let { tabParent } = this.frameLoader; > if (tabParent) { > tabParent.stopApzAutoscroll(this._autoScrollScrollId, > this._autoScrollPresShellId); > + spurious newline
Attachment #8892223 - Flags: review?(bugmail) → review+
Pushed by bballo@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7e558ba2e9a6 Notify browser.xml when APZ cancels an AutoscrollAnimation. r=kats
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Is there a way QA can manually verify this issue? Thank you!
Flags: qe-verify?
(In reply to Petruta Rasa [QA] [:petruta] from comment #8) > Is there a way QA can manually verify this issue? Thank you! Good question! I spent some time doing code reading and playing around with various interactions with autoscrolling and other input methods, but I wasn't able to trigger a scenario where APZ cancels the autoscroll but the main-thread codepath didn't. It's still possible that such scenarios exist, and it's good to have the fallback mechanism added in this bug for such cases, but they're not easy to trigger. So, I would say there is no need to QA verification on this issue. Thanks!
Thank you for your time!
Flags: qe-verify? → qe-verify-
Depends on: 1502614
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: