1247854 - Scrolling with mousewheel is broken in some situations

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect, P1)

Description

[Julien Wajsberg [:julienw]]

[Tracking Requested - why for this release]:

STR:
1. Go to https://www.paris-web.fr/association/membres.php
2. put your mouse on a non empty space
3. Try to scroll using the mousewheel (or anything similar like the trackpad scroll -- on Linux at least)
=> It doesn't work at all.

This works when doing the same gesture on the blue background on the left or the right of the page.
This also works when the page is in "mobile" mode (you can press ctrl + + repeatidly to enter this mode, or enter responsive mode).

Reproducing in Aurora 46 and Nightly 47 (Build ID: 20160211030242).
NOT reproducing in Beta 45 and Stable 44.

Nothing looks unusual in this page's CSS. The obvious difference between the mobile and desktop versions of the site is the use of a "float" in the desktop version.

Comment 1

[Alice0775 White]

Regression window: 
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=2ec54b38a33da939a8255612999d9e867eb11664&tochange=bb5becd378f40a9be14e4e635d7034f2835fc7b5

Via local build
Last Good: db3a7c3208e7
First Bad: 7d23d87b02f8

Comment 2

[Alice0775 White]

Reproduced on Windows7 and Ubuntu14.04

Comment 3

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

Also happens on OS X aurora and nightly. Doesn't happen if APZ is disabled.

Comment 4

[Alice0775 White]

Attachment: reduced html

Comment 6

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

This is a P1 blocking APZ rollout since we're generally recommending Keith's pure-CSS parallax approach as being APZ-friendly and it's currently broken because of this bug.

Comment 7

[Markus Stange [:mstange]]

Attachment: reftest

Comment 8

[Markus Stange [:mstange]]

Attachment: MozReview Request: Bug 1247854 - Apply the correct scroll clips to the nsDisplayTransform and nsDisplayPerspective of a scrolled perspective item. r?mattwoodrow

I've decided to fix this in a very explicit way. The only "magic" part that's
left is how we decide that the AGR of the perspective item is outside the
scrolled frame (and I'm not sure myself how that works).

I didn't want to change what scroll clips we set on what items, because the
scroll clip really belongs on the perspective item, because that's the item
that needs to be clipped, and it should also be the item that should be
scrolled if it weren't for the fact that APZ wouldn't know that it should
apply the perspective transform before the APZ transform.

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

Comment 9

[Markus Stange [:mstange]]

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

Comment 10

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8724047 [details]
MozReview Request: Bug 1247854 - Apply the correct scroll clips to the nsDisplayTransform and nsDisplayPerspective of a scrolled perspective item. r?mattwoodrow

https://reviewboard.mozilla.org/r/36833/#review33773

Comment 11

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Tracking, regression. 
Markus, once this lands, want to request uplift?

Comment 12

[Markus Stange [:mstange]]

Yes, absolutely, this needs to be uplifted once it lands. It's currently waiting for the patches in bug 1238564 to be reviewed and landed.

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e0b2fffc1360

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e0b2fffc1360

Comment 15

[Markus Stange [:mstange]]

Comment on attachment 8724047 [details]
MozReview Request: Bug 1247854 - Apply the correct scroll clips to the nsDisplayTransform and nsDisplayPerspective of a scrolled perspective item. r?mattwoodrow

Approval Request Comment
[Feature/regressing bug #]: APZ / bug 1147673
[User impact if declined]: scrolling impossible on some pages, and a certain parallax technique won't work properly
[Describe test coverage new/current, TreeHerder]: this patch adds a test
[Risks and why]: low
[String/UUID change made/needed]: none

This patch requires the changes in bug 1238564.

Comment 16

[Chris Peterson [:cpeterson]]

kats says we'll want this fix for the APZ experiment on Beta 46.

Comment 17

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8724047 [details]
MozReview Request: Bug 1247854 - Apply the correct scroll clips to the nsDisplayTransform and nsDisplayPerspective of a scrolled perspective item. r?mattwoodrow

This sounds like a must fix, has been in Nightly for a week, taking it. Aurora47+, Beta46+

Comment 18

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Julien, could you please verify this issue is fixed as expected on the latest Nightly build (03-10-2016 build on wards has the fix)? Thanks!

Comment 19

[Julien Wajsberg [:julienw]]

Yes this is fixed on Nightly :)

Thanks for the hard work !

Comment 20

[Carsten Book [:Tomcat]]

has conflicts uplifting to aurora:

grafting 332726:e0b2fffc1360 "Bug 1247854 - Apply the correct scroll clips to the nsDisplayTransform and nsDisplayPerspective of a scrolled perspective item. r=mattwoodrow"
merging layout/base/FrameLayerBuilder.cpp
merging layout/reftests/async-scrolling/reftest.list
warning: conflicts while merging layout/base/FrameLayerBuilder.cpp! (edit, then use 'hg resolve --mark')
warning: conflicts while merging layout/reftests/async-scrolling/reftest.list! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use hg resolve and hg graft --continue)

Comment 21

[Markus Stange [:mstange]]

This patch requires the patches in bug 1238564. We can't uplift it until those patches are also uplifted.

Comment 22

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

Ritu, could you please also approve bug 1238564 for uplift?

Comment 23

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

wes, can you try to land this again after landing the uplifts in bug 1238564? Thanks!

Comment 24

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Liz and I chatted about the risks involved in uplifting bug 1238564. The fix was deemed must-have for 46 and 47. She A+'d in the other bug.

Comment 25

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/c690a0802e00

Comment 26

[Carsten Book [:Tomcat]]

has problems uplifting to beta:

merging layout/base/FrameLayerBuilder.cpp
merging layout/base/FrameLayerBuilder.h
merging layout/reftests/async-scrolling/reftest.list
warning: conflicts while merging layout/base/FrameLayerBuilder.cpp! (edit, then use 'hg resolve --mark')
warning: conflicts while merging layout/reftests/async-scrolling/reftest.list! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use hg resolve and hg graft --continue)

Comment 27

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

https://hg.mozilla.org/releases/mozilla-beta/rev/24b6aaac8973

Comment 28

[Petruta Horea [:phorea]]

Verified as fixed using Firefox 47 beta 1 (with e10s enabled) and latest Aurora 48.0a2.