Closed Bug 1631518 Opened 4 years ago Closed 4 years ago

Page shouldn't scroll while dragging (WR-specific)

Categories

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

Unspecified
All
defect

Tracking

()

VERIFIED FIXED
mozilla78
Tracking Status
firefox76 --- wontfix
firefox77 --- wontfix
firefox78 --- verified
firefox79 --- verified

People

(Reporter: agi, Assigned: kats)

References

(Blocks 3 open bugs)

Details

Attachments

(5 files)

Example: https://www.cnn.com/2020/04/15/world/viking-mountain-pass-norway-scn/index.html

drag the image slider left and right, on Chrome you can't scroll while dragging, on Firefox you can, and it makes dragging really hard.

I'll cook up a minimal testcase soon.

Agi to move to APZ component once minimal test case found.

Flags: needinfo?(agi)

I've tried for a couple of hours yesterday but could not figure out why that page can scroll while dragging on Firefox but not Chrome.

They seem to use an outdated library called owl carousel, version 2.1 and jQuery 1.11 but even with these libraries the carousel scrolls in both Chrome and Firefox in a test case that I built (later versions of the carousel correctly don't scroll while dragging on both browsers).

Flags: needinfo?(agi)

James has an idea about Access lock handoff.

Flags: needinfo?(snorp)

Botond, does this have to do with how we're doing handoff on Android? Could we add some axis lock magic to prevent vertical scrolling of the parent if you're scrolling a child horizontally?

Flags: needinfo?(snorp) → needinfo?(botond)

I can reproduce this on Instagram as well (scroll all the way down in your feed page, there's a slideshow that on Firefox can pan and scroll, while on Chrome doesn't scroll while panning) I can attach a video if it's helpful.

Interestingly this works fine on Fennec. So I don't think it's a handoff problem, as that code hasn't changed since Fennec (at least as far as I remember).

Also works fine on a windows laptop using touchscreen.

Hm, seems to be specific to WebRender. If you force-disable WebRender this works fine in Fenix. On my old Z3C which has WR disabled by default it also works fine.

Blocks: wr-android
Severity: -- → S3
Component: General → Panning and Zooming
Flags: needinfo?(botond)
Priority: -- → P3
Product: GeckoView → Core
Summary: Page shouldn't scroll while dragging → Page shouldn't scroll while dragging (WR-specific)

Just guessing, but probably the carousel is setting touch-action properties that we're not properly respecting with WR.

Blocks: apz-hit-test

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #8)

Also works fine on a windows laptop using touchscreen.

Windows webrender touchscreen has the same issue for me, so it might be easier to debug there.
(custom settings: apz.allow_zooming: true, apz.windows.use_direct_manipulation: true)

I think this is higher priority, it makes high profile websites like Instagram really hard to use (I see this all the time I can bring more examples if needed).

Ok, I can try to take a look this week.

Assignee: nobody → kats
Severity: S3 → S2

Thank you kats!

I started investigating using the CNN test page, since I have concrete STR for that. There is indeed a touch-action: pan-y on one of the enclosing divs of the carousel (the one with class="js-owl-carousel owl-carousel carousel--full body owl-loaded owl-drag"). With non-WR the way this works is APZ knows it can do y-direction panning, so if the user does a horizontal pan it goes back from state TOUCHING to state NOTHING here and that prevents the touch block from triggering vertical panning later if the user changes direction.

Now I need to look at the WR path to see what's going on there.

With the WR path, APZ gets a hit info of 0xc1 which means pan-x AND pan-y. This causes APZ to obviously allow panning in both directions. The reason we get pan-x and pan-y is because we hit the codepath here because there's a div inside the pan-y one that is overflow:hidden and treat it like a scrollframe.

The non-WR equivalent codepath uses GetNearestScrollableFrame to determine what's a "scrollframe" and what's not, and that function ignores scrollframes where the overflow styles are both hidden, here (unless the appropriate flag is provided, which it isn't in this case).

So we need to fix the WR codepath to also ignore overflow:hidden scrollframes for touch-action purposes, and hopefully that will resolve this issue.

That does seem to fix the problem. Working on a test, should have patches up soonish.

Attached video repro video on GVE

I downloaded GVE from the try build and I can still reproduce on instagram (maybe a different bug?)

Comment on attachment 9148199 [details]
doesn't repro on Blink

This is on Blink 61 (sorry that was the only version available on the emulator, it's the same on my up-to-date Chrome on my phone)

Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/67e9a99139c1
Print hit info as hex, not decimal. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/3184b5dc57b9
Don't treat overflow:hidden divs as scrollframes for touch-action purposes with WR enabled. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/a86d55f19f9d
Add a test. r=tnikkel

If you still see it on Instagram please file another bug with STR. Thanks!

I have tested the issue on Firefox Preview Nightly 200610 using a OnePlus 6T (Android 9) and Huawei MediaPad M3 (Android 7.0) and I am unable to reproduce the issue. When scrolling through a vertical pictures carousel at https://www.cnn.com/2020/04/15/world/viking-mountain-pass-norway-scn/index.html horizontal gestures are ignored.

Status: RESOLVED → VERIFIED
Blocks: 1764174
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: