1163920 - Can't scroll iframe with APZC + scrollgrab

Status: RESOLVED INCOMPLETE

(Core :: Panning and Zooming, defect)

Description

[Paul Rouget [:paul]]

B2G on osx, in a certified app, with APZC enabled, this code makes the iframe unscrollable with the mouse wheel.


<div style="overflow:scroll" >
 <iframe src="http://wikipedia.org"></iframe>
</div>

div.scrollgrab = true;

Comment 1

[Paul Rouget [:paul]]

Let me know if there's anything I can do to help debug this issue.

Would dumping the APZC hit test tree help?

Comment 2

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

Yeah, please get the APZ hit testing tree dump. Also please set apz.printtree to true and get the APZC tree that gets printed out from that, it should indicate which APZC has the scrollgrab set on it.

Comment 3

[Paul Rouget [:paul]]

Attachment: dump.txt

Comment 4

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

Unfortunately there doesn't seem to be enough in the log to really diagnose the issue. Can you enable the logging at the top of APZCTreeManager and try scrolling? It might provide more useful info.

Comment 5

[Paul Rouget [:paul]]

Attachment: dump.txt

Hope that's better.

Comment 6

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

Hm. I don't understand why there's so much apzctree spew in this log, that's weird. Did you make local changes to that code? The actual hit testing part seems to be working fine. I suspect that this is the wheel transaction code that doesn't allow scroll handoff while in a transaction. The scrollgrab frame is not scrollable but the handoff might not be happening either. Try commenting out the code at http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/InputBlockState.cpp?rev=0cca6dda7467#390 and see if that helps.

Comment 7

[Paul Rouget [:paul]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #6)
> Hm. I don't understand why there's so much apzctree spew in this log, that's
> weird. Did you make local changes to that code?

No. Is it bad? Should I file a bug?

There are so many logs that I'm not sure exactly when it's happening.
Might be when I focus the window, or might be when I start scrolling.

> The actual hit testing part
> seems to be working fine. I suspect that this is the wheel transaction code
> that doesn't allow scroll handoff while in a transaction. The scrollgrab
> frame is not scrollable but the handoff might not be happening either. Try
> commenting out the code at
> http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/
> InputBlockState.cpp?rev=0cca6dda7467#390 and see if that helps.

Returning true here appears to make scrolling work.

Comment 8

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

(In reply to Paul Rouget [:paul] from comment #7)
> (In reply to Kartikaya Gupta (email:[e-mail]) from comment #6)
> > Hm. I don't understand why there's so much apzctree spew in this log, that's
> > weird. Did you make local changes to that code?
> 
> No. Is it bad? Should I file a bug?

It is bad, and I can reproduce it locally. It's a bug in the logging code. I'll file a bug with a patch.

> > The actual hit testing part
> > seems to be working fine. I suspect that this is the wheel transaction code
> > that doesn't allow scroll handoff while in a transaction. The scrollgrab
> > frame is not scrollable but the handoff might not be happening either. Try
> > commenting out the code at
> > http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/
> > InputBlockState.cpp?rev=0cca6dda7467#390 and see if that helps.
> 
> Returning true here appears to make scrolling work.

Ok, so I think that confirms my theory. Are you able to reproduce the same behaviour on a regular desktop build with e10s and APZ enabled? It should in theory behave the same as a B2G desktop build but when I tried that using the testcase you gave in comment 0 I was able to scroll the page fine. If you can't reproduce on a desktop build then it's something specific to B2G desktop. If that's the case I can try making a B2G desktop build and tracking it down.

Comment 9

[Botond Ballo [:botond]]

Scrollgrab relies on scroll being handed off "constantly" from the APZC that's the event target (which is always the innermost APZC that was hit, regardless of scrollgrab), to the first APZC in the handoff chain (which would be the scroll-grabbing APZC if there is one). It's possible that the restrictions wheel transactions impose on scroll handoff are interfering with that.

Comment 10

[Botond Ballo [:botond]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #8)
> when I tried that using
> the testcase you gave in comment 0 I was able to scroll the page fine.

Were you seeing scroll-grabbing behaviour though? I think the condition at [1] means setting scrollgrab is only allowed in chrome-privileged code. (Certainly a testcase I put together where content tries to set scrollgrab [2] doesn't exhibit scrollgrabbing behaviour, although given my (lack of) HTML skills, I may very well have just messed up the testcase.)

[1] http://mxr.mozilla.org/mozilla-central/source/dom/html/nsGenericHTMLElement.cpp?rev=673edf1e9272#1717
[2] http://people.mozilla.org/~bballo/scrollgrab.html

Comment 11

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

(In reply to Botond Ballo [:botond] from comment #9)
> Scrollgrab relies on scroll being handed off "constantly" from the APZC
> that's the event target (which is always the innermost APZC that was hit,
> regardless of scrollgrab), to the first APZC in the handoff chain (which
> would be the scroll-grabbing APZC if there is one). It's possible that the
> restrictions wheel transactions impose on scroll handoff are interfering
> with that.

Quite possibly, yeah.

(In reply to Botond Ballo [:botond] from comment #10)
> Were you seeing scroll-grabbing behaviour though? I think the condition at
> [1] means setting scrollgrab is only allowed in chrome-privileged code.

Ah, my mistake - you're right that the version I put together didn't actually use scrollgrabbing because of this.

Comment 12

[Paul Rouget [:paul]]

With Firefox Desktop, I can reproduce with:
- forcing nsGenericHTMLElement::IsScrollGrabAllowed() to return true
- enabling layers.async-pan-zoom.enabled
- with this page: http://output.jsbin.com/muquyijoxi

Comment 13

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

Attachment: Test case

Comment 14

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

Attachment: Test case

Comment 15

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

Comment on attachment 8607611 [details]
Test case

Ugh, missed the JS.

Comment 16

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

So I can repro with the URL I attached above if I follow the STR in comment 12 (in particular the thing to enable scrollgrab). However since this requires scrollgrab to be working (which doesn't affect general web content) it's out of scope for the apz-on-desktop work we currently have in progress, and therefore lower priority.

Comment 17

[Paul Rouget [:paul]]

How hard is it to fix? If it's just a matter of piping things together, maybe I can give it a try if someone can explain to me what is wrong with the wheel code.

Comment 18

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

I'm not really sure, it would take some investigation to figure out how to fix this without breaking other wheel transaction use cases. Comment 9 would be a good starting point.

Comment 19

[Paul Rouget [:paul]]

David, maybe you have an idea of how to fix that.

Comment 20

[David Anderson [:dvander] - inactive, e-mail if emergency]

Wheel transactions don't do handoff, they just look at the first valid apzc that can scroll in the direction of the wheel movement. See here: https://dxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/OverscrollHandoffState.cpp#177

So it sounds like that would need to be changed to account for scrollgrab. Does scrollgrab work without apz?

Comment 21

[Botond Ballo [:botond]]

(In reply to David Anderson [:dvander] from comment #20)
> Does scrollgrab work without apz?

Nope.

(In reply to David Anderson [:dvander] from comment #20)
> Wheel transactions don't do handoff, they just look at the first valid apzc
> that can scroll in the direction of the wheel movement. See here:
> https://dxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/
> OverscrollHandoffState.cpp#177
> 
> So it sounds like that would need to be changed to account for scrollgrab.

The handoff chain should already be ordered such that its first element is the scroll-grabbing APZC [1], so I don't think that's the problem.

[1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/APZCTreeManager.cpp?rev=c780203cf304#1354

Comment 22

[Johnny Stenback (:jst)]

Botond, Kats, are you guys still looking into this?

Comment 23

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

Not at the moment. Is b2g-desktop a priority platform for some reason?

Comment 24

[Paul Rouget [:paul]]

Not sure if that makes sense or not, but commenting out this line:

http://hg.mozilla.org/mozilla-central/file/f5e3bacfb60e/gfx/layers/apz/src/AsyncPanZoomController.cpp#l2060

makes scrolling work.

Comment 25

[Paul Rouget [:paul]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #23)
> Not at the moment. Is b2g-desktop a priority platform for some reason?

We want to release the first version of browser.html this month.

Comment 26

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

Are you releasing from mozilla-central or from your project branch? If you're releasing from the project branch and the fix from comment 24 doesn't break anything else feel free to use that for now. We can investigate and fix it properly before you merge to central and/or once we have some time. I'm putting it on the backlog list so we prioritize it above other stuff.

Comment 27

[Paul Rouget [:paul]]

We have our own branch. But I just don't know what is the side effect of comment 24.

Comment 28

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

Right now, if you go to a page like http://people.mozilla.org/~kgupta/bug/1163920-2.html, put your mouse over the div halfway down the page, and fling really hard, you will see the div scroll to the end, but the scroll will not get handed off to the outside page. I *think* that with your patch you will see the scroll get handed off to the outside page, which is not a correct behavior for wheel event scrolling.

Comment 29

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

Is this still something you guys care about? B2G desktop isn't a thing any more AFAIK.

Comment 30

[Paul Rouget [:paul]]

> Is this still something you guys care about? B2G desktop isn't a thing any more AFAIK.

That would be nice to have, but not something we absolutely need. We are slowly transitioning to Servo.

Comment 31

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

Don't care about B2G any more.