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

Chrome 56 breaks touch events #8968

Closed
mstijak opened this issue Feb 9, 2017 · 17 comments
Closed

Chrome 56 breaks touch events #8968

mstijak opened this issue Feb 9, 2017 · 17 comments

Comments

@mstijak
Copy link

mstijak commented Feb 9, 2017

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
React attaches events to the document node which causes the latest version of Chrome to issue the following warning.

Unable to preventDefault inside passive event listener due to target being treated as passive. See https://www.chromestatus.com/features/5093566007214080

It appears that Chrome now treats all document level touch events as passive.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/reactjs/69z2wepo/).

class App extends Component {
  render() {
    return (
      <div
          className="box"
          onTouchStart={e=>{e.preventDefault()}}
          onTouchMove={e=>{e.preventDefault()}}
      >
          Drag Me
      </div>
    );
  }
}

What is the expected behavior?
preventDefault should be allowed, which means React should pass { passive: false } when adding event listeners.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
React 15.4.1, Chrome 56.0.2924.87, Windows 10

@mstijak mstijak changed the title Unable to call preventDefault in React event handlers in Chrome 56 preventDefault doesn't work for touch event handlers in Chrome 56 Feb 9, 2017
@mstijak mstijak changed the title preventDefault doesn't work for touch event handlers in Chrome 56 Chrome 56 breaks touch event handlers Feb 9, 2017
@mstijak mstijak changed the title Chrome 56 breaks touch event handlers Chrome 56 breaks touch events Feb 9, 2017
@oliviertassinari
Copy link
Contributor

oliviertassinari commented Feb 11, 2017

I would link that issue with #6436.

@mstijak If you are looking for a workaround. You can manually use the addEventListener and removeEventListener DOM API with the right option.

@mstijak
Copy link
Author

mstijak commented Feb 11, 2017

@oliviertassinari Manually attaching listeners seems wrong. I think this should be fixed in React.

@mstijak
Copy link
Author

mstijak commented Feb 11, 2017

Here is the fiddle. To see the warning open the console and switch to the device mode (touch events).

@sebmarkbage
Copy link
Collaborator

Ok. This will be interesting to deal with. If Chrome makes a breaking change, does that mean libraries should respond by unbreaking it? Effectively undoing the purpose.

Now this whole thing will be complicated further by the fact that React Fiber is a whole shift to be able to write 60fps apps with active event listeners. In that world defaulting to active makes more sense.

On the flip side we're not quite there yet in the ecosystem and there is some action from apps to be able to make that palatable.

This will need some careful consideration to make the right choices for defaults.

@RByers
Copy link

RByers commented Feb 12, 2017

Rather than set passive:false, it's much better to apply the appropriate touch-action to disable scrolling where desired. See here.

@mstijak
Copy link
Author

mstijak commented Feb 12, 2017

@RByers What would be the best thing to do if a component uses both mouse and touch events?

@sebmarkbage
Copy link
Collaborator

sebmarkbage commented Feb 12, 2017

@RByers does that let you disable scrolling the outer window if you're at the bottom of an inner scrollable surface such as the right sidebar on Facebook? (Without disabling scrolling of the sidebar itself.)

@RByers
Copy link

RByers commented Feb 12, 2017

@mstijak mouse and touch are orthogonal here. You can/should still call preventDefault, just also set touch-action to indicate declaratively what touch gestures you want disabled.

@sebmarkbage it can, in at least the same scenarios you can disable scrolling via TouchEvent.preventDefault in Safari. I.e. once scrolling has started you can't change it.
You may want to set pan-up when at the bottom.

@gaearon
Copy link
Collaborator

gaearon commented Feb 15, 2017

Relevant Chromium issue for context:
https://bugs.chromium.org/p/chromium/issues/detail?id=639227

@nhunzaker
Copy link
Contributor

Was this ever resolved?

@gaearon
Copy link
Collaborator

gaearon commented Jul 12, 2017

No, it wasn't.

@jh3141
Copy link

jh3141 commented Aug 29, 2017

@RByers - "it's much better to apply the appropriate touch-action to disable scrolling where desired" ... yes, but this doesn't provide the flexibility that you can achieve using preventDefault. I'm working on an app, for instance, that allows scrolling around a list with a touch + move gesture, but switches to dragging a single item with touch + hold for 0.5s + move. Waiting to see whether a move arrives before the 0.5s and adding the touch-action style doesn't seem to work reliably (I think it ignores the touch action for the entire gesture if the first movement of the gesture is in a direction that can cause scrolling, but accepts it if the first movement is in a direction that doesn't cause scrolling).

@RByers
Copy link

RByers commented Aug 30, 2017

@jh3141: Yes, that's absolutely true. I have a proposal for (what I think is) that exact use case here, I'd love your feedback there! If we get a few developers saying they need this and can agree on a design that would work, then I'm sure it's something we could ship sooner rather than later.

scottyantipa added a commit to strateos/react-map-interaction that referenced this issue Apr 21, 2019
scottyantipa added a commit to strateos/react-map-interaction that referenced this issue Apr 22, 2019
scottyantipa added a commit to strateos/react-map-interaction that referenced this issue Jun 13, 2019
scottyantipa added a commit to strateos/react-map-interaction that referenced this issue Jun 13, 2019
scottyantipa added a commit to strateos/react-map-interaction that referenced this issue Jun 13, 2019
scottyantipa added a commit to strateos/react-map-interaction that referenced this issue Jun 13, 2019
@Dmitra
Copy link

Dmitra commented Oct 25, 2019

this is a blocker for enabling touch specific event handlers, as there is no way to prevent default mouse events from firing according to https://developer.mozilla.org/en-US/docs/Web/API/Touch_events/Supporting_both_TouchEvent_and_MouseEvent

@GeoMarkou
Copy link

Will this be fixed in React 17.0? I noticed in the changes that events will be bound to "root element" instead of "document".

https://reactjs.org/blog/2020/08/10/react-v17-rc.html#changes-to-event-delegation

@gaearon
Copy link
Collaborator

gaearon commented Sep 10, 2020

Yes, we've accidentally "fixed" this with that change in one of the RCs, but in the 17 we're explicily making onWheel, onTouchStart, and onTouchMove passive to align with the browser behavior. Since this change has improved the scrolling performance on the web overall, we don't think it would be productive to "undo" the fix — as it's still likely going to lead to performance problems. For now, you can keep using refs + addEventListener as a workaround if you need active event listeners.

@gaearon
Copy link
Collaborator

gaearon commented Mar 24, 2021

Seems like we just went with what Chrome did. And if you want to work around, you can use refs and addEventListener yourself.

@gaearon gaearon closed this as completed Mar 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants