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

MobileSafariClickEventPlugin requires touch events to be initialized #1169

Closed
Simek opened this issue Feb 24, 2014 · 39 comments · Fixed by #1536 or #3246
Closed

MobileSafariClickEventPlugin requires touch events to be initialized #1169

Simek opened this issue Feb 24, 2014 · 39 comments · Fixed by #1536 or #3246

Comments

@Simek
Copy link
Contributor

Simek commented Feb 24, 2014

Event do not trigger when node do not have "cursor: pointer" style on it.

Here you have an example:
http://jsfiddle.net/kb3gN/1345/

@syranide
Copy link
Contributor

There is no reference to pointer (not even cursor) in the React codebase, so I'm assuming this is some weird iOS bug, React might be able to work around it though.

@MattKunze
Copy link

My guess is that is an issue that has been around for quite a while (interesting timing that I ran into the same thing for a different context this morning):

http://www.quirksmode.org/blog/archives/2010/10/click_event_del_1.html
http://stackoverflow.com/questions/7358781/tapping-on-label-in-mobile-safari

You'll see a bunch of workarounds that add empty handlers when the page loads to address this ($('label').click(function() {});)

Doesn't seem like React's problem to fix - either add 'cursor: pointer' or empty event handlers where it causes an issue.

@syranide
Copy link
Contributor

@MattKunze We do try to fix things that can be fixed and it wouldn't surprise me if you can temporarily add a click-handler on touch and solve it like that.

@sophiebits
Copy link
Collaborator

Yes, we have https://github.com/facebook/react/blob/master/src/browser/eventPlugins/MobileSafariClickEventPlugin.js to fix this exact problem but it sounds like perhaps it's not working.

@MattKunze
Copy link

Is that plug-in registered automatically, or do you have to do something to enable it?

I can confirm that the original jsfiddle posted above isn't working for me on iOS

@Simek
Copy link
Contributor Author

Simek commented Feb 24, 2014

Digging through repository I found event plugin which seems to be related to similar bug:
https://github.com/facebook/react/blob/master/src/browser/eventPlugins/MobileSafariClickEventPlugin.js

But problem seems to be little wider because Chrome on iOS is also affected so this is an mobile webkit issue.

@MattKunze
Copy link

Adding

    componentDidMount: function() {
      this.getDOMNode().onclick = function() {}
    }

to the component is a workaround. It seems like the plugin is not working at the moment

@mietek
Copy link

mietek commented Mar 23, 2014

This is still not working in 0.10.0, and is actually a duplicate of #134.

Adding cursor: pointer to elements which are intended to be clickable is still the least-offensive workaround.

@sophiebits sophiebits changed the title Click event doesn't work on iOS on certain situation MobileSafariClickEventPlugin requires touch events to be initialized Mar 26, 2014
@sophiebits
Copy link
Collaborator

Ever since on-demand event listening was added, MobileSafariClickEventPlugin has been broken because touchstart isn't always listened to. cursor: pointer; is an okay workaround but we'll have to fix this for real somehow, probably by having onclick handlers turn into onclick="" markup.

nicolashery added a commit to tidepool-org/clamshell that referenced this issue Apr 29, 2014
Need to use either `<a>` elements, or add `{cursor: pointer;}` CSS rule
to element.

See: facebook/react#1169
@sophiebits sophiebits added the bug label May 15, 2014
sophiebits added a commit to sophiebits/react that referenced this issue May 27, 2014
Fixes facebook#1169.

This is a more robust way of fixing what MobileSafariClickPlugin previously tried to. Now even if you don't want anything to do with touch events, click events still work properly.

Test Plan: Added a click handler to an `<img />` element and triggered it in the iOS simulator -- it didn't execute before.
yungsters added a commit that referenced this issue May 27, 2014
Attach empty onclick listener to each node
@sophiebits
Copy link
Collaborator

This was reverted in 431155d.

@sophiebits sophiebits reopened this Jul 8, 2014
@MoOx
Copy link

MoOx commented Jul 23, 2014

Having the issue has well. And using a cursor pointer can't work in several case for me since I've set cursor to help|zoom-in in some place... :/
That would be nice that react include a awesome fix :)

@VinSpee
Copy link

VinSpee commented Sep 26, 2014

After going around with this quite a bit, I'm failing to see how this is an issue. iOS seems to be following semantics here. Any element that triggers an action should be a <button> or a <a>, right? Can't we just do that and solve this?

@slorber
Copy link
Contributor

slorber commented Jul 2, 2015

this is so weird :(

Found this interesting link: http://www.nczonline.net/blog/2012/07/05/ios-has-a-hover-problem/
It does not talk about cursor: pointer however.

What I understand is that if an element has no cursor pointer and a click listenerr, the first click will trigger the :hover state and only the second will fire the click.

This is what happens in our app, and adding cursor: pointer managed to solve the problem: the click listener is called directly and not the :hover

@slorber
Copy link
Contributor

slorber commented Jul 2, 2015

@spicyj sorry i'm a bit lost in your revert reverts of reverts.
Is this finally available? Which version?

What's the overhead of adding so much click listeners everywhere? Can't we do this only on iOS ?

I don't understand the solution. Where do you add that click listener?

for example I have

<div onClick={doSomething}>
   <intermediateComp>
       <children/>
   </intermediateComp>
</div>

When clicking on the children, the :hover is triggered (it seems to :hover the whole div actually!), but the click is only fired when reclicking the children.

Where does your solution put the click listeners exactly and why does it solve the problem?

@sophiebits
Copy link
Collaborator

Yes, it landed in master and will be in 0.14.

This diff adds an empty onclick listener so that Mobile Safari knows your div is clickable. This mirrors what would happen if you wrote onclick="..." directly in your HTML or called addEventListener without using React. I'm not sure how this interacts with hover.

We could probably do this only on iOS but I don't know if other browsers use similar heuristics. Hopefully this has minimal performance impact but we're planning to do some benchmarking soon and if this is significant we'll figure out a way to speed it up.

@slorber
Copy link
Contributor

slorber commented Jul 5, 2015

ok thanks will start using the beta then :)

Still not sure to understand on which node the onclick listener is setup. In my example the div already has one so I guess this means that all others will install the click listener?

@slorber
Copy link
Contributor

slorber commented Jul 6, 2015

Yes that seems to work.

I've updated the initial JSFiddle to test on iOS: http://jsfiddle.net/mgenhgpd/

@mietek
Copy link

mietek commented Jul 26, 2015

This is still an issue in 0.13.3.

@sophiebits
Copy link
Collaborator

It's fixed in 0.14 beta and will be in the final 0.14 as well.

@sophiebits
Copy link
Collaborator

As my earlier comment in this thread already says.

kjell added a commit to artsmia/art that referenced this issue Aug 7, 2015
dreyescat added a commit to dreyescat/react-rating that referenced this issue Aug 24, 2015
@ustun
Copy link

ustun commented Jan 24, 2016

We could probably do this only on iOS but I don't know if other browsers use similar heuristics. Hopefully this has minimal performance impact but we're planning to do some benchmarking soon and if this is significant we'll figure out a way to speed it up.

@spicyj Were you able to perform any benchmarks after this was merged?

@sophiebits
Copy link
Collaborator

@ustun Not on this specifically, but we have no evidence that it's a hotspot.

@hansthinhle
Copy link

It's fixed in 0.14 beta and will be in the final 0.14 as well.

@spicyj onClick is still not working on iOS in 0.14.7 but fixed in 15.0. And we sill get stuck in 0.14.7 I think.

@sophiebits
Copy link
Collaborator

I don't know of any changes in 15.0 that would have affected this.

@brunogarcia
Copy link

@spicyj I've tried with React 15.1.0 but still doesn't work in Safari 9.0.
By the way on Chrome 51.0.2 works perfectly.
Both tested into an iPad 9.1

@sophiebits
Copy link
Collaborator

@brunogarcia Can you post a jsfiddle or jsbin with this?

@brunogarcia
Copy link

@spicyj Hi, we've made more testing with Safari (iOS 9.3.2).
We've found that Safari on private browsing doesn't save data into LocalStore.
So, it's not a React bug.
Sorry for my mistake.

@easga
Copy link

easga commented Aug 12, 2016

This has taken my day, i had a form which was handling onSubmit first. Form was refreshing on iphone 4 even though it was executing event.preventDefault().(every other device i tested works with no problem since the beginning) Then i tried everything which i ended replacing form with a div and making the button handling onClick. I am running (react": "^15.0.1"), including cursor pointer but click event never happens so i can not execute the function. I really do not know what is left for me to try. I would wholeheartedly appreciate if i could find a solution. thanks.

@tiaaaa123
Copy link

I am having the same issue in react 15.0.1

@sophiebits
Copy link
Collaborator

@easga @tiaaaa123 If one of you can find a simple repro case (jsbin or jsfiddle), please post it and we can dig in.

@easga
Copy link

easga commented Sep 22, 2016

Thanks. My customer was seeing this problem on iphone 4, but once i realized i could simulate that device and check the logs, i realized that my production js was having problems due the reason mentioned in this article (http://lukecod.es/2016/03/14/react-invariant-violation-minified-exception-ios8-webpack-babel/). Then i made it work. Sorry that i forgot to update it here, everything works great with React itself!

@amiut
Copy link

amiut commented Sep 13, 2021

If the problem still exists , maybe overlaying a position absolute a tag that wraps your element and adding the click event to this a tag will help.

@facebook facebook locked and limited conversation to collaborators Sep 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet