Closed
Bug 1247074
Opened 9 years ago
Closed 8 years ago
Smooth scroll gets interrupted by frame reconstruction
Categories
(Core :: Panning and Zooming, defect, P2)
Tracking
()
People
(Reporter: kats, Assigned: kats)
References
Details
(Keywords: correctness, regression, Whiteboard: gfx-noted)
Attachments
(2 files, 1 obsolete file)
See attached test - loading the page should result in the inner frame scrolling to 500px. That's what happens in Chrome and Safari. In Firefox it doesn't. I think the reason is the same as what's happening in bug 1235899 - the smooth-scroll gets interrupted because the nsGfxScrollFrame is destroyed and re-created. That being said, I would have expected my patch in bug 1235899 to fix this (with APZ enabled) but it doesn't seem to. It needs some more investigation.
Assignee | ||
Updated•9 years ago
|
Component: Layout → Panning and Zooming
Updated•9 years ago
|
Whiteboard: gfx-noted,perf
Assignee | ||
Comment 1•9 years ago
|
||
This is not really perf, more correctness. Also I guess technically it's a regression from when the scroll-behavior stuff was turned on.
Keywords: regression
Whiteboard: gfx-noted,perf → gfx-noted,correctness
Assignee | ||
Updated•9 years ago
|
Keywords: correctness
Whiteboard: gfx-noted,correctness → gfx-noted
Assignee | ||
Updated•8 years ago
|
Assignee | ||
Updated•8 years ago
|
Updated•8 years ago
|
Version: Trunk → 48 Branch
Assignee | ||
Comment 2•8 years ago
|
||
Bumping out of 48
Assignee | ||
Updated•8 years ago
|
Priority: -- → P2
Updated•8 years ago
|
Assignee | ||
Comment 3•8 years ago
|
||
This one turned out to be pretty straightforward: the frame reconstruction basically loses the destination of the smooth-scroll animation. The restored scroll position is whatever the scroll position was at the time the frame was destroyed. In the non-APZ case the AsyncScroll or AsyncSmoothMSDScroll scroll is destroyed during frame reconstruction, halting the animation. In the APZ case a new scroll position update is sent to the APZ which aborts the animation in the compositor and resets the position back to what the main thread thinks it was. As per discussion with tnikkel last week, it's better to save/restore the destination of the animation - that way we'll end up scrolled to the right position, even if we don't do a smooth animation to it. The attached patch seems to work, and includes the test case converted to a mochitest. Try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea5795a031d1
Assignee: nobody → bugmail
Assignee | ||
Comment 4•8 years ago
|
||
The try push showed an intermittent failure in the test I wrote. I have to put this on the back burner for now to go fix other intermittent failures but I'll get back to it later.
Assignee | ||
Comment 5•8 years ago
|
||
Cleaner try push with modified test at https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c3c2719b3dc&group_state=expanded
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8784914 -
Attachment is obsolete: true
Comment 7•8 years ago
|
||
mozreview-review |
Comment on attachment 8786149 [details] Bug 1247074 - When a compositor-based smooth scroll animation is in progress and the scrollframe is reconstructed, restore to the animation destination. https://reviewboard.mozilla.org/r/75116/#review73870
Attachment #8786149 -
Flags: review?(tnikkel) → review+
Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f478daa490df When a compositor-based smooth scroll animation is in progress and the scrollframe is reconstructed, restore to the animation destination. r=tnikkel
Comment 9•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f478daa490df
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Assignee | ||
Comment 10•8 years ago
|
||
Since this didn't appear to fix bug 1297419 (the reporter can still repro that one) it's probably not worth uplifting.
You need to log in
before you can comment on or make changes to this bug.
Description
•