Skip to content
This repository has been archived by the owner on Feb 11, 2021. It is now read-only.

Scrolling broken when touch-action and touch events are supported #116

Closed
RByers opened this issue Dec 30, 2013 · 2 comments
Closed

Scrolling broken when touch-action and touch events are supported #116

RByers opened this issue Dec 30, 2013 · 2 comments

Comments

@RByers
Copy link

RByers commented Dec 30, 2013

To repro:
Use a browser that supports both touch-events and touch-action (for example chromium with https://codereview.chromium.org/108673003/ applied and --enable-experimental-web-platform-features set, soon also Firefox). Then try a test page that uses PointerEvents like samples/scroller/index.html. Notice that you can't scroll anything anymore.

I believe this is because touch.js calls scrollType.set in the !HAS_TOUCH_ACTION code paths, but not in the HAS_TOUCH_ACTION codepaths. With HAS_TOUCH_ACTION shouldScroll appears to always return false because scrollType is unpopulated, and so all touchmove events are consumed. It looks like you can simulate this same state by forcing HAS_TOUCH_ACTION to be true in touch.js.

It's not immediately obvious to me what the right fix is, but I think the reasoning behind this code is flawed. It looks like when touch-action is supported it's using a simpler path of always registering touch-event handlers, otherwise it adds the handlers only when required by the touch-action. I believe this is to avoid blocking composited scrolling in the common case where scrolling is permitted. However browsers that support JUST touch-action and touch-events will still have this problem - for compatibility if there's a touch handler then scrolling must be blocked on the handler. Firefox intends to ship this way, so PointerEvents should not be disabling the touch handler optimization just because touch-action is supported.

In Chrome we plan to address the scroll-blocking issue through an additional feature on top of touch-action: 'touch-action-delay'. When 'touch-action-delay: none' is in effect then the presence of a touch handler will not impact scroll performance. So I think the correct fix here is to change HAS_TOUCH_ACTION to HAS_TOUCH_ACTION_DELAY (and fix the issue where scrollType isn't set).

touch-action support in chrome is imminent (http://crbug.com/241964), but touch-action-delay support is still a ways out (http://crbug.com/329559). So it's fine if we want to make the trivial change first (make behavior conditional on touch-action-delay instead of touch-action) and worry about the larger issue (setting the scrollType correctly) later.

RByers pushed a commit to RByers/PointerEvents that referenced this issue Dec 31, 2013
Change HAS_TOUCH_ACTION to HAS_TOUCH_ACTION_DELAY.  Even with touch-action we should still avoid unnecessary touch handlers.  It's only with touch-action-delay: none that we can be sure our handlers won't delay threaded scrolling.

This works around jquery-archive#116 for now without actually solving it completely.
@RByers
Copy link
Author

RByers commented Dec 31, 2013

Damn, apparently there's a bug and Chrome 33 and the CSSOM objects exist for touchActionDelay even though all parsing of it will fail. I've fixed it in https://codereview.chromium.org/108673003/, but in the meantime it means the approach in this pull request won't work (it'll break scrolling for all chrome users!).

Instead I suggest we disable this code path entirely for now until there's a browser closer to being able to use it.

@RByers
Copy link
Author

RByers commented Jun 12, 2014

I guess we should make it clear this bug is fixed by the work-around I landed back in December - closing.

@RByers RByers closed this as completed Jun 12, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant