Closed Bug 1268009 Opened 8 years ago Closed 8 years ago

Top and bottom bar issues when disabling APZ per page

Categories

(Core :: Panning and Zooming, defect)

48 Branch
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla49
Tracking Status
firefox46 --- unaffected
firefox47 --- unaffected
firefox48 --- verified
firefox49 --- verified

People

(Reporter: phorea, Assigned: botond)

References

Details

(Keywords: correctness, regression, Whiteboard: [gfx-noted])

Attachments

(1 file)

[Note]:
- This only happens when setting the pref apz.disable_for_scroll_linked_effects to True

[Affected versions]:
- Aurora 48.0a2 2016-04-26
- Nightly 49.0a1 2016-04-26

[Affected platforms]:
- Win 7 64-bit
- Mac OS X 10.9.5

[Steps to reproduce]:
1. Set pref apz.disable_for_scroll_linked_effects to True and restart the browser
2. Open https://docs.python.org/2/library/logging.html#handler-objects
3. Scroll the page down to reach the bottom bar then scroll up until the top bar is shown

[Expected result]:
- Top and bottom bars are visible
- Side menu is not moved

[Actual result]:
- Sometimes top and bottom bars are not visible or only partially visible: http://i.imgur.com/97oEp5a.png 
- Side menu is moved up/down when reaching the top/bottom of the page

[Regression range]:
- Introduced by bug 1246290
Blocks: 1246290
Bottom part of https://www.amazon.com/clouddrive/home/ is also affected.
Keywords: regression
I tried reproducing this on today's nightly on Linux and wasn't able to.

What method are you using to scroll? How often do you see the problem after scrolling to the top/bottom?
I can reproduce the problem with STR of comment#0 on Nightly49.0a1 Windows7.

> What method are you using to scroll?

  Turn mouse wheel / Hold mousedown on up,down button/ Keypress up,down,Page UP,Page Down / Autoscroll.

  (If you cannot reproduce, you may need slowly scroll, and should not move mouse position)

>  How often do you see the problem after scrolling to the top/bottom?

  Very easy to reproduce.
  Top Bar: 100%
  Bottom Bar: 100%
I can reproduce the bug with the top bar on Windows. Clicking on the page makes the top bar fully appear, suggesting that we're missing a paint with the final scroll position of the page.

(That also explains why I couldn't reproduce the problem on Linux - on Linux, the scroll bar button becomes disabled when you reach the beginning / end of the page, triggering a repaint.)
Keywords: correctness
Whiteboard: [gfx-noted]
Maybe the paint skipping code needs to be conditioned on APZ not being force-disabled?
(In reply to Botond Ballo [:botond] from comment #4)
> (That also explains why I couldn't reproduce the problem on Linux - on
> Linux, the scroll bar button becomes disabled when you reach the beginning /
> end of the page, triggering a repaint.)

Indeed - I can reproduce on Linux by disabling scrollbar buttons.

(In reply to Botond Ballo [:botond] from comment #5)
> Maybe the paint skipping code needs to be conditioned on APZ not being
> force-disabled?

Hmm, it looks like we already disable paint skipping if we have scroll-linked effects, and we're also only force-disabling APZ if we have scroll-linked effects, so that should be fine already.
(In reply to Botond Ballo [:botond] from comment #6)
> Hmm, it looks like we already disable paint skipping if we have
> scroll-linked effects

Oh, but not for apz-originated scrolls!
Assignee: nobody → botond
Comment on attachment 8746197 [details]
MozReview Request: Bug 1268009 - If APZ is force-disabled, disable paint skipping even for apz-originated scrolls. r=kats

https://reviewboard.mozilla.org/r/49301/#review46151

Good find, thanks!
Attachment #8746197 - Flags: review?(bugmail.mozilla) → review+
This will need to be rebased across bug 1256727.
(In reply to Botond Ballo [:botond] from comment #11)
> This will need to be rebased across bug 1256727.

Er, the backout of it.
Comment on attachment 8746197 [details]
MozReview Request: Bug 1268009 - If APZ is force-disabled, disable paint skipping even for apz-originated scrolls. r=kats

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49301/diff/1-2/
Comment on attachment 8746197 [details]
MozReview Request: Bug 1268009 - If APZ is force-disabled, disable paint skipping even for apz-originated scrolls. r=kats

Asking for re-review, to make sure I've got the conditions correct.
Attachment #8746197 - Flags: review+ → review?(bugmail.mozilla)
Comment on attachment 8746197 [details]
MozReview Request: Bug 1268009 - If APZ is force-disabled, disable paint skipping even for apz-originated scrolls. r=kats

Yup, looks correct.
Attachment #8746197 - Flags: review?(bugmail.mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/247e67077292
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Please request uplift to 48
Flags: needinfo?(botond)
Comment on attachment 8746197 [details]
MozReview Request: Bug 1268009 - If APZ is force-disabled, disable paint skipping even for apz-originated scrolls. r=kats

Approval Request Comment
[Feature/regressing bug #]:
  Bug 1246290, which implemented a mode where APZ is disabled
  on pages with scroll-linked effects. This mode is disabled
  by default, but we want to leave open the option of enabling
  it in Firefox 48 if necessary.

[User impact if declined]:
  With this mode enabled, paints are sometimes skipped during
  scrolling, which can leave the page in an inconsistent state
  (e.g. with a header bar coming only partially into view)
  until something else (e.g. a click) forces a repaint.

[Describe test coverage new/current, TreeHerder]:
  On m-c since 2016-04-29.

[Risks and why]: 
  Low - this is a small, localized changes, and it only affects
  the APZ force-disabled mode.

[String/UUID change made/needed]:
  None.
Flags: needinfo?(botond)
Attachment #8746197 - Flags: approval-mozilla-aurora?
Comment on attachment 8746197 [details]
MozReview Request: Bug 1268009 - If APZ is force-disabled, disable paint skipping even for apz-originated scrolls. r=kats

This will help give more options for APZ in 48, please uplift.
Attachment #8746197 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed under Win 7 64-bit and Mac OS X 10.9.5 using 48.0a2 and 49.0a2 2016-06-06/07.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.