1501062 - Even while dragging the scroll thumb, if the mouse pointer is far enough away from the scrollbar, page scroll position should back to its original position

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect, P2)

Description

[Alice0775 White]

Reproducible: always when WebRender is enabled

Steps To Reproduce:
1. Open long page
    --- remember original scroll position
2. Dragging scroll thumb
3. Keeping mouse down, move the mouse pointer away from the scroll bar

Actual results:
Page scroll position does tot back to its original position.

Expected Results:
Page scroll position should back to its original position. This is Windows default behavior.

Comment 1

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

Strange, this should be handled in the part of the APZ code that is shared between WR and non-WR. I can look into it if it's deemed to be a blocker.

Comment 2

[Asa Dotzler [:asa]]

same as bug 1501092?

Comment 4

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

Yup, thanks.

Comment 5

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=17a1159f7f4d&tochange=273f8af59697

Regressed by:	43963e011193	Kartikaya Gupta — Bug 1389000 - Turn on webrender layers-free mode by default. r=jrmuizel

Comment 6

[Botond Ballo [:botond]]

No one sets WebRenderLayerScrollData::mVisibleRegion?

Comment 7

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #6)
> No one sets WebRenderLayerScrollData::mVisibleRegion?

To elaborate on that: triggering the scrollbar sanp-back feature from APZ requires knowing the bounds of the scrollbar thumb node, which comes from ScrollNode::GetVisibleRegion() in APZCTreeManager::UpdateHitTestingTree().

When I fixed bug 1352863, I implemented GetVisibleRegion() for ScrollNode=WebRenderScrollDataWrapper; the value came from WebRenderLayerScrollData::mVisibleRegion which was set in WebRenderLayerScrollData::Initialize(). At the time WebRenderLayerScrollData objects were constructed from layers.

Since then, things were refactored so WebRenderLayerScrollData objects are constructed from display items, and the setting of mVisibleRegion seems to have gotten lost.

Comment 8

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

https://searchfox.org/mozilla-central/rev/a7f4d3ba4fbfe3efbde832869f1d672fce7122f6/layout/painting/nsDisplayList.cpp#7070

^ This would be the place to re-add the setter, if we can get an equivalent rect from the display item.

Comment 9

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

I have a fix, but I'll try to write a test to go with it.

Comment 10

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

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=9b56a085856cca9dcbf2ea965f33bd5e936d6378

Comment 11

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

Attachment: Bug 1501062 - Set the visible region on scrollthumb WebRenderLayerScrollData items. r?botond

This field is used by APZ to implement the feature where the scrollbar snaps
back to the starting position if the mouse gets too far away.

Comment 12

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

Attachment: Bug 1501062 - Set the visible region on scrollthumb WebRenderLayerScrollData items. r?botond

This field is used by APZ to implement the feature where the scrollbar snaps
back to the starting position if the mouse gets too far away.

Comment 13

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

Sorry, I tried evolving the changeset via prune -s but I guess phabricator didn't like that...

Comment 14

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

Carried r=botond from the first phabricator review:

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

Comment 15

[Raul Gurzau (:RaulG)]

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

Comment 17

[laszlo.bialis]

Verified on latest Nightly 65.0a1 with WebRender activated and the issue is fixed, the page scroll position goes back to its original position.