Top and bottom bar issues when disabling APZ per page

VERIFIED FIXED in Firefox 48

Status

()

defect
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: petruta.rasa, Assigned: botond)

Tracking

({correctness, regression})

48 Branch
mozilla49
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox46 unaffected, firefox47 unaffected, firefox48 verified, firefox49 verified)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 attachment)

[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
Assignee

Comment 2

3 years ago
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?

Comment 3

3 years ago
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%
Assignee

Comment 4

3 years ago
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.)
Assignee

Updated

3 years ago
Keywords: correctness
Whiteboard: [gfx-noted]
Assignee

Comment 5

3 years ago
Maybe the paint skipping code needs to be conditioned on APZ not being force-disabled?
Assignee

Comment 6

3 years ago
(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.
Assignee

Comment 7

3 years ago
diagnosis
(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

Updated

3 years ago
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+
Assignee

Comment 11

3 years ago
This will need to be rebased across bug 1256727.
Assignee

Comment 12

3 years ago
(In reply to Botond Ballo [:botond] from comment #11)
> This will need to be rebased across bug 1256727.

Er, the backout of it.
Assignee

Comment 13

3 years ago
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/
Assignee

Comment 14

3 years ago
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+

Comment 17

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/247e67077292
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Please request uplift to 48
Flags: needinfo?(botond)
Assignee

Comment 19

3 years ago
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.