1268009 - Top and bottom bar issues when disabling APZ per page

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect)

Description

[Petruta Horea [:phorea], Desktop Test Engineering]

[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

Comment 1

[Petruta Horea [:phorea], Desktop Test Engineering]

Bottom part of https://www.amazon.com/clouddrive/home/ is also affected.

Comment 2

[Botond Ballo [:botond]]

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

[Alice0775 White]

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%

Comment 4

[Botond Ballo [:botond]]

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.)

Comment 5

[Botond Ballo [:botond]]

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

Comment 6

[Botond Ballo [:botond]]

(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.

Comment 7

[Botond Ballo [:botond]]

(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!

Comment 8

[Botond Ballo [:botond]]

Attachment: MozReview Request: Bug 1268009 - If APZ is force-disabled, disable paint skipping even for apz-originated scrolls. r=kats

Review commit: https://reviewboard.mozilla.org/r/49301/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/49301/

Comment 9

[Kartikaya Gupta (email:[e-mail])]

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!

Comment 10

[Botond Ballo [:botond]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=40af378c5363

Comment 11

[Botond Ballo [:botond]]

This will need to be rebased across bug 1256727.

Comment 12

[Botond Ballo [:botond]]

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

Er, the backout of it.

Comment 13

[Botond Ballo [: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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49301/diff/1-2/

Comment 14

[Botond Ballo [: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

Asking for re-review, to make sure I've got the conditions correct.

Comment 15

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/247e67077292

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/247e67077292

Comment 18

[Kartikaya Gupta (email:[e-mail])]

Please request uplift to 48

Comment 19

[Botond Ballo [: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.

Comment 20

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 21

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8cb0342020dc

Comment 22

[Petruta Horea [:phorea], Desktop Test Engineering]

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.