Skip to content
This repository has been archived by the owner on Mar 27, 2019. It is now read-only.

Android default browser fire two click event #361

Closed
ehretf opened this issue May 8, 2013 · 23 comments
Closed

Android default browser fire two click event #361

ehretf opened this issue May 8, 2013 · 23 comments

Comments

@ehretf
Copy link

ehretf commented May 8, 2013

Hello,

I have noticed that the Android's default browser fire two click events on every DOM element when using iscroll with jquery and jquery mobile (no issue when using iscroll with jquery only).
This seems to be only with the default browser every thing is fine with Chrome (didn't test on other browser).
I noticed this issue with my Nexus 4 ( Android 4.2.2) and my Galaxy S (Cyanogen 9).

I have first thnik phonegap was the root cause but when using it on a web page (served by a remote http server) I had the same issue.

To fix this, I have changed click to vclick in line 558 in iscroll.js
ev.initMouseEvent('vclick', true, true, e.view ...

This seems to make iscroll acting fine and fixed my issue. This make iscroll depending on jquery and may need additionnal testing to ensure this doesn't break something else

I would like to know what you think on this and also this may help other users.

Thanks a lot

Best Regards.

Florent

@cubiq
Copy link
Owner

cubiq commented May 9, 2013

thanks for taking the time to experiment with iscroll. this will be hopefully fixed in iScroll 5. Unfortunately jQuery dependency is a no-go.

@sebbie1o1
Copy link

Bug still exists in iScroll5

@cubiq
Copy link
Owner

cubiq commented Jun 12, 2013

use tap:true and listen to tap event instead.

@santifdez
Copy link

ev.initMouseEvent('vclick', true, true, e.view is good solution for Android mobile but doesn´t work for IOS

@pierluigi
Copy link

@santifdez it actually works on Jellybean 4.2 but doesnt on 4.1.2 / Samsung with TouchWiz (same webkit 4.0 Browser)

@davidpfahler
Copy link
Collaborator

This is (mostly) fixed in v5. v4 or earlier is not supported anymore. Please check out v5 if you have this issue.

If you still run into this problem on v5, please comment here. Please provide as many details as possible. I will reopen then.

@scottcarter
Copy link

I am seeing this problem with iScroll v5. I'm testing using the Android SDK:

AVD_for_Nexus_One_by_Google, Android 4.3, API Level 18, CPU Intel Atom (x86).

I'm using 4.3 instead of 4.4.2 (latest) because the latter doesn't support rotate to landscape properly.

My setup is driven by Rails and I'm including:
jquery, jquery.ui.draggable, jquery_ujs (rails adapter), iscroll, twitter/bootstrap, jPlayer (http://jplayer.org)

I debugged this a little and noticed that for every single tap I see:

IScroll.prototype, handleEvent, mousedown
IScroll.prototype, handleEvent, touchstart
Call to utils.tap

IScroll.prototype, handleEvent, mousedown
Call to utils.tap

Since my setup already detects whether I am on a mobile device, I am working around this issue by using disableMouse: true

@davidpfahler davidpfahler reopened this Feb 11, 2014
@chrisbod
Copy link

This problem occurs for me with iScroll5 as well

@Nedvedyy
Copy link

tap:true this works good,.

@m5jun
Copy link

m5jun commented Jul 30, 2014

i use iscroll5 ,set tap: true, but it still exists in android, there are two tap event fired.

@kisPocok
Copy link

kisPocok commented Sep 5, 2014

Still valid.

@joantune
Copy link

joantune commented Nov 5, 2015

Thank you for posting the issue and the solution. We had this issue with iScroll4. This was an incredibly time consuming, especially because we didn't knew the problem came from iScroll, and we actually thought there was a Ghost click that the native WebView was triggering.

The fact that my android does not allow my chrome inspector to connect to the App's webview, didn't quite help ('old' version of Android [circa 4.2.x]).
Also, there's quite a lot of mambo jumbo out there that kinda confuses and doesn't help much.

So, a big +1 for this thread.

We did not try it with iScroll5

@sculove
Copy link
Collaborator

sculove commented Mar 9, 2016

The event flow of android browser is different.
A little browser triggers mouse event before touch events.
Therefore, iScroll couldn't prevent next events.
so, click or tap event is fired twice over.

You can fix it if your browser is mobile.
remove mouse events
put disableMouse option to use in iScroll

new IScroll('#wrapper', {
     disableMouse : true,
     // ...
});

@zivester
Copy link

zivester commented Apr 19, 2016

@sculove @cubiq - This was a pretty big breaking change which should have warranted a major version bump to 6.0.0. Changing the defaults pretty much breaks anyone (including my team) who is relying on them. Thanks for the module, but hopefully next time this adheres to semver and only gets applied to the next major version bump.

@sculove
Copy link
Collaborator

sculove commented Apr 20, 2016

@zivester The default values of disableMouse, disableTouch, disablePointer are decided automatically according to a browser.

eg1. if the browser is able to use pointer and touch and mouse events, iScroll options are like this.

disablePointer: false,
disableTouch: true,
disableMouse: true

eg2. if the browser is able to use touch and mouse events, iScroll options are like this.

disablePointer: true,
disableTouch: false,
disableMouse: true

eg3. if the browser is able to use mouse event, iScroll options are like this.

disablePointer: true,
disableTouch: true,
disableMouse: false

I tested many devices.
Could you tell me what you are worrying about ?

@zivester
Copy link

zivester commented Apr 20, 2016

@sculove

Well the defaults changed which is 100% a breaking change.

We've tested this on numerous devices running chrome (Android 4, 5, iOS), of which we had declarations like the following:

new Scroll($scrollWrapper, {
    mouseWheel: true,
    scrollbars: true,
    bounce: true,
    eventPassthrough: 'horizontal',
});

Touch events stopped being produced on these devices (and scrolling broke entirely). Scrollbars were still visible, and mouse events worked (when testing on Chrome Desktop). The offending line being:

+       disableTouch : utils.hasPointer || !utils.hasTouch,

Locking our application into 5.1.3 resolves the issue. I'd advise reverting this change on the 5.1.x revision, and save the default upgrade to 6.x.x.

@kaansoral
Copy link

No offense, but, every update to iscroll requires extensive testing and re-development due to the nature of the library

I tested 5.2.0 for example, it seemed extremely fast and smoother with speed, however, none of the clicks was working for me, probably because I had countless modifications to the code at 5.1.3, one of which perfected the android clicks for 5.1.3

What should definitely be tested are inputs, if input clicks doesn't work, it might break sign ups etc., my .02

@zivester
Copy link

I'm not debating whether the updates are meaningful, but the detection and defaults you introduced in this commit do break existing installations.

Do what you want with it, but we have locked into an older version while we make sure that our defaults are aligned to the pre 5.2.0 defaults.

I will fork this repo while removing the defaults introduced in this commit. If you'll accept a PR for the removal of these defaults on the 5.x.x revisions I will be happy to provide one.

@joantune
Copy link

If you are updating a library version without testing (or if you didn't
lock the version in the first place) do not complain that the update broke
your product. Never ship a new version without testing.

That said, this library is worse than making html+js for old versions of
I.E., because there are many different versions of the default webview for
Android, and a bit less for Apple's counterpart.
What I'm saying is: any little change can break anything anywhere, and
AFAIK there is no robust and free way to test in every platform (let me
know if there is).
Plus, never forget that this is an open source library, made out of
contributions, so an attitude like that really makes no sense for the
amount of contributions that you gave back.

On 03:37, Wed, Apr 20, 2016 zivester notifications@github.com wrote:

I'm not debating whether the updates are meaningful, but the detection and
defaults you introduced in this commit do break existing installations.

Do what you want with it, but we have locked into an older version while
we make sure that our defaults are aligned to the pre 5.2.0 defaults.

I will fork this repo while removing the defaults introduced in this
commit. If you'll accept a PR for the removal of these defaults on the
5.x.x revisions I will be happy to provide one.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#361 (comment)

@sculove
Copy link
Collaborator

sculove commented Apr 20, 2016

@zivester I couldn't produce your situation.
I tested Chrome Desktop, Android 2,4,5, iOS8, 9.
Can you tell me your environment in detail?

  • browser version, full iScroll options, or custom events...

@zivester
Copy link

zivester commented Apr 20, 2016

@sculove:

navigator.userAgent
"Mozilla/5.0 (Linux; Android 5.1; XT1528 Build/LPI23.29-17.5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/39.0.2171.93 Mobile Safari/537.36"
utils.hasPointer
true
utils.hasTouch
true

Obviously in this case, touch is being disabled because hasPointer is true. I can reproduce this on all of my devices. Would anything else trigger hasPointer being set to true for mobile chrome?

EDIT: Seem to have found the culprit, another library we're using, https://github.com/jquery/PEP which polyfills window.PointerEvent which is throwing all of this off.

@sculove
Copy link
Collaborator

sculove commented Apr 21, 2016

@zivester OK.
I added PEP before iScroll initialization. I face a problem on Chrome Desktop (Emulator), Android, iOS.
but the Chrome Desktop is working.
That's as maybe this issue is PEP.
You'd better check PEP.

@hasbug
Copy link

hasbug commented Dec 15, 2016

i use iscroll-probe 5.2.0 ,set tap: true, but it still exists in android, there are two tap event fired.
PC/IOS is worked.
but, Penetrate to document (Android)。

Tried in:
Chrome mobile emulator (devtools Android)
Android Browser

options:
` var myScroll = new IScroll(ele, {
scrollbars: scrollbar,
fadeScrollbars: true,
disableMouse : true,
click: iScrollClick(),
tap: true,
probeType: 2
});

function iScrollClick(){
        // workaround click bug iscroll #674
        if (/iPhone|iPad|iPod|Macintosh/i.test(navigator.userAgent)) return false;
        if (/Chrome/i.test(navigator.userAgent)) return (/Android/i.test(navigator.userAgent));
        if (/Silk/i.test(navigator.userAgent)) return false;
        if (/Android/i.test(navigator.userAgent)) {
            var s=navigator.userAgent.substr(navigator.userAgent.indexOf('Android')+8,3);
            return parseFloat(s[0]+s[3]) < 44 ? false : true}
    }

`

aisouard pushed a commit to Mappy/iscroll that referenced this issue Feb 22, 2017
if browser doesn't have events, a default value of disablePointer/Touch/Mouse options is 'true'
- change default value about disablePointer,disableTouch, disableMouse options
- don't attach unnesassray events.
- fix two click/tap issue.

Close cubiq#361
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