Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify what touch-action allows with rotated scrollable elements #115

Closed
staktrace opened this issue Jul 22, 2016 · 20 comments

Comments

@staktrace
Copy link

commented Jul 22, 2016

I think the spec is unclear on what the intended behaviour is for elements which have rotation transforms on them. For example on this test page Chrome allows scrolling while your finger is moving in the "x" direction, even though that means the element is scrolling vertically (the scrollTop value is changing). While that's a reasonable thing to do, our implementation in Firefox does the opposite (allows scrolling such that scrollLeft changes).

@patrickhlauke

This comment has been minimized.

Copy link
Member

commented Jul 22, 2016

The spec currently says

In the case of pan-left, pan-right, pan-up and pan-down, the direction is interpreted as the opposite of the physical movement in the screen co-ordinate space.

wondering if dropping the "In the case..." bit, and simply stating that "The direction..." relates to the (document-level?) screen co-ordinate space, would help clarify the situation here? (this, would then also explain Chrome's behavior)

@RByers

This comment has been minimized.

Copy link
Contributor

commented Jul 25, 2016

Thanks for reporting this! We discussed this a bit here but I was sloppy. There's two things being conveyed by this sentence:

  1. All directions should be interpreted relative to the "screen" co-ordinate space.
  2. Up/down/left/right represent the opposite direction of finger travel (i.e. typically the direction in which panning would occur ignoring rotated iframes).

IIRC we went through a few edits of this text trying to come up with something concise that was accurate. Clearly I missed the non-direction-specific case. More importantly IMHO, we failed to add a test case for this. Let's use this bug to track fixing both.

@staktrace

This comment has been minimized.

Copy link
Author

commented Jul 26, 2016

I guess we can discuss it F2F but I just tried this on Edge as well and it does the same thing that FF does (i.e. different from Chrome).

@RByers

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2016

We discussed this at the hackathon, and realized that the Edge/Gecko behavior really is necessary. Imagine a page with a carousel that must allow horizontal dragging (so pan-y) hosted in a rotated iframe. The carousel will no longer work. So screen co-ordinate space really is wrong.

What is the right co-ordinate space then, is it client? I.e. should a CSS rotated DIV also behave like a rotated iframe?

@RByers RByers changed the title Spec text should clarify what touch-action allows with rotated scrollable elements Clarify what touch-action allows with rotated scrollable elements Jul 29, 2016

@RByers

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2016

Note that fully supporting this behavior (eg. arbitrary angle rotation) will be hard in Chrome's architecture (I don't think we handle non-axis-aligned scrolling well at all). In particular, we filter our gestures for touch-action in the browser process before we know anything about the content under the finger. But I think we could probably do a quick hack for the 90/180/270 rotation cases (take rotation into account when computing the effective touch action) and leave the rest as a pretty low priority bug blocked on our compositor hit-test rework.

@patrickhlauke

This comment has been minimized.

Copy link
Member

commented Jul 29, 2016

What is the right co-ordinate space then, is it client?

document perhaps? (assuming that the iframe's content counts as its own document)

I.e. should a CSS rotated DIV also behave like a rotated iframe?

I'd say no, but I can see how that may be confusing. But if we said that it's document, it should be clear (maybe with extra clarification) that this is unaffected by any rotation etc.

@staktrace

This comment has been minimized.

Copy link
Author

commented Jul 29, 2016

I would rather be consistent between divs and iframes, so use "client" coordinate space. I'm not sure if there's any value to allowing off-kilter scrolling (i.e. what Chrome does now if you have a div rotated to 45 degrees). Also, the argument for iframes (that a carousel hosted in a rotated iframe should behave sanely) applies to divs as well, in that somebody might be using a library to build a carousel and then want to host it inside a rotated div.

@patrickhlauke

This comment has been minimized.

Copy link
Member

commented Apr 4, 2017

Would be good to get something roughed out for this (likely as part of the overall touch-action definition, rather than repeating it for each directional value)

@mustaqahmed

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2017

We also need to fix the computation of effective touch-action:

A touch behavior is supported if allowed by the touch-action properties of all elements between the hit tested element and it's nearest ancestor with the default touch behavior...
Perhaps defining the computation "recursively" is the simplest option, after mentioning the coordinate space above it.

@mustaqahmed mustaqahmed self-assigned this Apr 19, 2017

@mustaqahmed

This comment has been minimized.

Copy link
Contributor

commented Apr 25, 2017

Here is the Chrome bug.

@mustaqahmed

This comment has been minimized.

Copy link
Contributor

commented Apr 26, 2017

Let's leave this open:

  • The part on touch-action value changes during an ongoing action may need some attention.
  • We need to add tests, better to track it here.
@mustaqahmed

This comment has been minimized.

Copy link
Contributor

commented Apr 26, 2017

While rearranging the touch-action section (Sec 9) of the spec to bring all coordinate space related text close together (in my last pull request above), I also moved the Note on dynamic changes to touch-action values into a non-Note para (the third bullet in Sec 9.2). I will request a closer look just in case.

@patrickhlauke

This comment has been minimized.

Copy link
Member

commented Apr 26, 2017

moving the dynamic changes bit out into an actual bullet looks good to me.

@RByers

This comment has been minimized.

Copy link
Contributor

commented Apr 26, 2017

I would rather be consistent between divs and iframes, so use "client" coordinate space.

I think this is normally called "local" co-ordinate space, right? I.e. this is NOT what clientX and clientY events do (they are unaffected by rotated DIVs, just rotated iframes) IIRC. I think the non-standard MouseEvent.layerX/layerY properties may behave this way.

Can someone verify what Edge and FF do in the case of a rotated DIV? Does touch-action operate in client or the element-local co-ordinate space?

In particular if Edge and FF are both really using the client co-ordinate space today then I'd rather spec/test it that way. Yes in theory there could be a problem within a frame, but that problem exists for co-ordinates generally - the code rotating something needs to also apply the same rotation to any client co-ordinate handling (and can probably do so because it's all in the same frame). Only the iframe case is really unsolvable IMHO (because the code that knows about the transform in the parent frame doesn't necessarily have access to the events being delivered to the child frame). Implementing the local version is probably harder for chromium than implementing the client version. But if at least one other browser has really already implemented local as @staktrace described, then I'm OK saying Chrome just has a bug here for now.

@RByers

This comment has been minimized.

Copy link
Contributor

commented Apr 26, 2017

Oh also we should think about how we can best test this. The touch-action tests today aren't very scalable (one test per gesture - a lot of boilerplate to add a few new test cases like the ones discussed here). I wonder if now is the time to rewrite them with an assumption of having automation driving them? Or at least using some generic framework in a single (or few) test file which can either be automated or driven manually one gesture after another in the same test... Thoughts?

@mustaqahmed

This comment has been minimized.

Copy link
Contributor

commented Apr 27, 2017

Can someone verify what Edge and FF do in the case of a rotated DIV? Does touch-action operate in client or the element-local co-ordinate space?

For all of Edge, FF and Chrome, a rotated div filters touch-actions the same way as a rotated frame. Here is a repro that has "pan-y" in a rotated div.

Edge: Works perfectly: only horizontal (along screen x-axis) swipes can scroll the div along local y-axis direction.
FF: Semi perfect. Horizontal (along screen x) swipes scroll the div along local y-axis. But some of the vertical (along screen y) swipes also seem to scroll the div along local y-axis, perpendicular to the swipe! Attention @staktrace.
Chrome: Bad, ignores the rotation.

@mustaqahmed

This comment has been minimized.

Copy link
Contributor

commented Apr 27, 2017

Looks like @staktrace's repro link in the original post is also for a rotated div. Then so far we don't know what happens to a rotated iframe, right?

@mustaqahmed

This comment has been minimized.

Copy link
Contributor

commented Apr 27, 2017

Here is a rotated subframe repro. Looks like I was (blindly) right above: div and subframe behave similarly for each of the three browsers.

@RByers

This comment has been minimized.

Copy link
Contributor

commented Apr 27, 2017

Thanks Mustaq! Ok then, documenting what Edge is already doing (and Firefox is mostly doing) seems indeed to be the way to go. Thanks for double checking!

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Jun 2, 2017
Added a test for touch-action in rotated divs
Tests that touch-action directions in a div rotated by 90-degrees are interpreted in the local (rotated) coordinate space.

Fixes w3c/pointerevents#115.

Bug: 715148
Change-Id: I2c4b62d250ceb65900b81ffc58ad1ac3fd9c1065
Reviewed-on: https://chromium-review.googlesource.com/521931
Cr-Commit-Position: refs/heads/master@{#476699}
WPT-Export-Revision: c87cba496b5e1d8221a4ddc48a4da380d19f1079
chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Jun 5, 2017
Added a test for touch-action in rotated divs
Tests that touch-action directions in a div rotated by 90-degrees are interpreted in the local (rotated) coordinate space.

Fixes w3c/pointerevents#115.

Bug: 715148
Change-Id: I2c4b62d250ceb65900b81ffc58ad1ac3fd9c1065
Reviewed-on: https://chromium-review.googlesource.com/521931
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#476982}
WPT-Export-Revision: 021b0659fd77e6514fc4a74fb06c827536550900
chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Jun 5, 2017
Added a test for touch-action in rotated divs
Tests that touch-action directions in a div rotated by 90-degrees are interpreted in the local (rotated) coordinate space.

Fixes w3c/pointerevents#115.

Bug: 715148
Change-Id: I2c4b62d250ceb65900b81ffc58ad1ac3fd9c1065
Reviewed-on: https://chromium-review.googlesource.com/521931
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#476982}
WPT-Export-Revision: 021b0659fd77e6514fc4a74fb06c827536550900
MXEBot pushed a commit to mirror/chromium that referenced this issue Jun 6, 2017
Added a test for touch-action in rotated divs
Tests that touch-action directions in a div rotated by 90-degrees are interpreted in the local (rotated) coordinate space.

Fixes w3c/pointerevents#115.

Bug: 715148
Change-Id: I2c4b62d250ceb65900b81ffc58ad1ac3fd9c1065
Reviewed-on: https://chromium-review.googlesource.com/521931
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#477025}
@mustaqahmed

This comment has been minimized.

Copy link
Contributor

commented Jun 6, 2017

The test has got merged.

@mustaqahmed mustaqahmed closed this Jun 6, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.