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

Touch action processing model doesn't match IE/Edge behavior around zooming #19

Closed
RByers opened this Issue Sep 8, 2015 · 6 comments

Comments

Projects
None yet
2 participants
@RByers
Contributor

RByers commented Sep 8, 2015

Hey,
The touch-action processing model says:

To determine the effect of a touch, find the nearest ancestor (starting from the element itself) that has a default touch behavior. Then examine the touch-action property of each element between the hit tested element and the element with the default touch behavior (including both the hit tested element and the element with the default touch behavior). If the touch-action property of any of those elements disallows the default touch behavior, do nothing.

This makes sense for scrolling, but what about zooming?

In particular, we've been saying "use touch-action: manipulation to disable the click delay" but what if your page has scrollable divs? Do you need to re-apply touch-action manipulation to each scroller to ensure the click delay is disabled everywhere? The above spec text implies that you do. But this sucks for this click-delay use case (means touch-action isn't a complete substitute for fastclick.js afterall).

Happily, this isn't actually what IE/Edge implement. So I think we should correct the spec text and blink implementation to match the IE / Edge behavior. I seem to recall some earlier iteration of the spec that referred to the ancestor that implements the specific default touch behavior being considered (as opposed to any "default touch behavior"). I know we agonized over trying to keep this definition accurate yet simple. Maybe we need to go back through the history here?

Anyway, if there's agreement that the spec should be corrected to match the Edge behavior, then I can take a crack at coming up with some text for this. Perhaps we should just be more verbose - describing an algorithm in stages, etc.

@jacobrossi can you describe precisely what the Edge behavior here is to make sure I'm not missing something subtle?

@RByers

This comment has been minimized.

Show comment
Hide comment
@RByers

RByers Sep 11, 2015

Contributor

@teddink said (on the list):

I spoke with Jacob (and others) here is what I was able to glean from those conversations:

This is covered by this section:

"When a user touches an element, the effect of that touch is determined by the value of the touch-action property and the default touch behaviors on the element and its ancestors. To determine the effect of a touch, find the nearest ancestor (starting from the element itself) that has a default touch behavior. Then examine the touch-action property of each element between the hit tested element and the element with the default touch behavior (including both the hit tested element and the element with the default touch behavior). If the touch-action property of any of those elements disallows the default touch behavior, do nothing. Otherwise allow the element to start considering the touch for the purposes of executing a default touch behavior."

That still correctly describes our behavior, in particular the statement around " find the nearest ancestor (starting from the element itself) that has a default touch behavior"

Does that help?

Contributor

RByers commented Sep 11, 2015

@teddink said (on the list):

I spoke with Jacob (and others) here is what I was able to glean from those conversations:

This is covered by this section:

"When a user touches an element, the effect of that touch is determined by the value of the touch-action property and the default touch behaviors on the element and its ancestors. To determine the effect of a touch, find the nearest ancestor (starting from the element itself) that has a default touch behavior. Then examine the touch-action property of each element between the hit tested element and the element with the default touch behavior (including both the hit tested element and the element with the default touch behavior). If the touch-action property of any of those elements disallows the default touch behavior, do nothing. Otherwise allow the element to start considering the touch for the purposes of executing a default touch behavior."

That still correctly describes our behavior, in particular the statement around " find the nearest ancestor (starting from the element itself) that has a default touch behavior"

Does that help?

@RByers

This comment has been minimized.

Show comment
Hide comment
@RByers

RByers Sep 11, 2015

Contributor

Does that help?

No, sorry - it doesn't (sorry, I should have provided a concrete repro from the start). For this concrete test case, double tap inside the scroller. The scroller satisfies the "nearest ancestor that has a default touch behavior" clause (because it can scroll), but Edge appears to not actually stop there. Instead it takes the touch-action of it's ancestor into account (and so disables double-tap zoom). Compare to the behavior in Chrome (which I believe does match the spec but the Edge behavior is better for developers).

So the "a default touch behavior" isn't sufficient here to describe Edge's behavior AFAICT.

Contributor

RByers commented Sep 11, 2015

Does that help?

No, sorry - it doesn't (sorry, I should have provided a concrete repro from the start). For this concrete test case, double tap inside the scroller. The scroller satisfies the "nearest ancestor that has a default touch behavior" clause (because it can scroll), but Edge appears to not actually stop there. Instead it takes the touch-action of it's ancestor into account (and so disables double-tap zoom). Compare to the behavior in Chrome (which I believe does match the spec but the Edge behavior is better for developers).

So the "a default touch behavior" isn't sufficient here to describe Edge's behavior AFAICT.

@RByers

This comment has been minimized.

Show comment
Hide comment
@RByers

RByers Oct 14, 2015

Contributor

Ping @teddink @jacobrossi. Note that WebKit is implementing 'touch-action: manipulation', I'd really like to clarify the expected behavior here and ensure chromium matches.

Contributor

RByers commented Oct 14, 2015

Ping @teddink @jacobrossi. Note that WebKit is implementing 'touch-action: manipulation', I'd really like to clarify the expected behavior here and ensure chromium matches.

@jacobrossi

This comment has been minimized.

Show comment
Hide comment
@jacobrossi

jacobrossi Oct 16, 2015

Member

I think the missing detail is that it's the nearest ancestor that has a default touch behavior for the gesture being performed, actually. So in the case of double-tap the scroller isn't configured to handle that gesture so Edge keeps looking up the chain for more rules. I'm not sure how we define that without getting into scope jail. But that's I think the missing nuance.

Member

jacobrossi commented Oct 16, 2015

I think the missing detail is that it's the nearest ancestor that has a default touch behavior for the gesture being performed, actually. So in the case of double-tap the scroller isn't configured to handle that gesture so Edge keeps looking up the chain for more rules. I'm not sure how we define that without getting into scope jail. But that's I think the missing nuance.

@RByers RByers self-assigned this Oct 17, 2015

@RByers

This comment has been minimized.

Show comment
Hide comment
@RByers

RByers Oct 17, 2015

Contributor

Thanks, that's exactly what I expected. I'll try to propose some text, but we should fix Chrome ASAP regardless /cc @dtapuska.

Contributor

RByers commented Oct 17, 2015

Thanks, that's exactly what I expected. I'll try to propose some text, but we should fix Chrome ASAP regardless /cc @dtapuska.

RByers added a commit to RByers/w3c-pointerevents that referenced this issue Oct 22, 2015

Fix the touch-action processing model for zoom scenarios
It's incorrect to consider the touch-action values only up to the nearest
ancestor that supports ANY default touch behavior.  Eg. in Edge you can
disable double-tap to zoom across an entire page (including any iframes
or other scrolling elements) by applying 'touch-action: manipulation' to
the html element.  See http://jsbin.com/tipure/ and http://jsbin.com/jucuna.

This is issue w3c#19.

RByers added a commit to RByers/w3c-pointerevents that referenced this issue Oct 22, 2015

Fix the touch-action processing model for zoom scenarios
It's incorrect to consider the touch-action values only up to the nearest
ancestor that supports ANY default touch behavior.  Eg. in Edge you can
disable double-tap to zoom across an entire page (including any iframes
or other scrolling elements) by applying 'touch-action: manipulation' to
the html element.  See http://jsbin.com/tipure/ and http://jsbin.com/jucuna.
This is issue w3c#19.

This change also moves the core processing model details up to
above the specific value definitions to make it easier to follow
without getting lost in all the value-specific notes.

@RByers RByers added the v2-blocking label Nov 3, 2015

@RByers

This comment has been minimized.

Show comment
Hide comment
@RByers

RByers Dec 4, 2015

Contributor

Good enough for now...

Contributor

RByers commented Dec 4, 2015

Good enough for now...

@RByers RByers closed this Dec 4, 2015

dstockwell pushed a commit to dstockwell/chromium that referenced this issue Dec 4, 2015

Don't reset touch-action zoom state at scrollers
This ensures that when a page disables double-tap zoom (eg. via
touch-action: manipulation) and hence the click delay, it stays
disabled even when tapping inside a scroller or iframe.

See w3c/pointerevents#19 for details.

Also removes 'toCoreFrame' declaration which has no implementation
anywhere.  I needed this from my test but discovered the common
pattern was to just invoke the toImplBase method directly.

BUG=529295

Review URL: https://codereview.chromium.org/1498923004

Cr-Commit-Position: refs/heads/master@{#363310}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment