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)
Core
Panning and Zooming
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.
Updated•8 years ago
|
Priority: -- → P3
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•8 years ago
|
||
Comment hidden (mozreview-request) |
Comment 4•8 years ago
|
||
mozreview-review |
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+
Comment hidden (mozreview-request) |
Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7e558ba2e9a6
Notify browser.xml when APZ cancels an AutoscrollAnimation. r=kats
Comment 7•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 8•7 years ago
|
||
Is there a way QA can manually verify this issue? Thank you!
Flags: qe-verify?
Assignee | ||
Comment 9•7 years ago
|
||
(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!
You need to log in
before you can comment on or make changes to this bug.
Description
•