Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Firefox 26.0 onTouchStart vs ontouchstart #958

Closed
ghost opened this Issue · 42 comments
@ghost

Flexslider 2.2.0
After update of FF to Version 26.0 my FlexSlider doesn't work correctly. I get error "ReferenceError: onTouchStart is not defined jquery.flexslider.js:397" with settings
animation: "slide", touch: true

When starting with a half sized browser window and 2 images the first image is scaled down. Everything seems to be OK. But when I enlarge the window to full size I see more than 2 images side by side and they don't react responsive. The clones aren't hidden.

Setting touch:false: works.

With touch:true I changed line 397 of jquery.flexslider.js from

el.addEventListener('touchstart', onTouchStart, false);

to (see lower case):

el.addEventListener('touchstart', ontouchstart, false);

The above mentioned error is gone and after some quick tests it looks like this fix works on FIREFOX (no further testing with other browsers!!).

@Daynos

Same problem with FlexSlider 2.2.2 + FireFox 26.0

No problem with Chrome 31 and IE 11

EDIT : problem solved by moving

el.addEventListener('touchstart', onTouchStart, false);

AFTER

function onTouchStart(e) { ... }
@Daynos Daynos referenced this issue from a commit in Daynos/FlexSlider
@Daynos Daynos Fix Issue #958
Fix issue woothemes#958
8a2300a
@robme

This is quite a major bug. Are there any plans to put the fix into a new release?

@mattyza
Owner
@miketaylr

The reason this breaks is it's relying on the function declaration inside the conditional to be hoisted--but that's not really defined behavior pre-ES6. kangax said it best, "Another important trait of function declarations is that declaring them conditionally is non-standardized and varies across different environments. You should never rely on functions being declared conditionally and use function expressions instead."

I can cook up a patch in a bit.

@miketaylr miketaylr referenced this issue from a commit in miketaylr/FlexSlider
@miketaylr miketaylr Fixes #958. Declare onTouchStart as a function expression.
This way it gets hoisted properly and we don't rely on undefined
behavior (which doesn't work in some browsers).
8af7e0d
@robme

I tested your fix and it works fine for me.

@amwelles

Fix works, but slides aren't fading like they should. See #977.

@miketaylr

Yeah, seems like perhaps this fix uncovered that other bug @amwelles. ping @mattyza :)

@banago

Same issue still persists. I'm on V.2.2.2 - Firefix 2.6.

UPDATE: lowercase 'ontouchstart' solves it for me with slider version 2.2.0.

@hrod

Fix works for me, but now I can't swipe on my iPad/iPhone.

@miketaylr miketaylr referenced this issue from a commit in miketaylr/FlexSlider
@miketaylr miketaylr Fixes #958. Declare onTouchStart as a function expression.
This way it gets hoisted properly and we don't rely on undefined
behavior (which doesn't work in some browsers).
042ceb9
@miketaylr miketaylr referenced this issue from a commit in miketaylr/FlexSlider
@miketaylr miketaylr Fixes #958. Declare onTouchStart, onTouchEnd, onTouchMove as function…
… expressions.

This way they get hoisted properly and we don't rely on undefined
behavior (which doesn't work in some browsers).
0b54de9
@miketaylr

I realize I only fixed ontouchstart, but not ontouchmove and ontouchend. Pushed updated patch.

@robme

I tried your updated patch but touch still doesn't work on an iPad.

If I use vanilla FlexSlider 2.2.0 then it works fine on an iPad, including swiping, but it won't work on Firefox because of the error in this issue.

If I use your fix that redeclares onTouchStart, onTouchEnd and onTouchMove then it works fine in Firefox without errors, and it works on iPads, but swiping does not work.

@hrod

Ditto what robme said.

@miketaylr

@robme @hrod any error messages, etc? Can you remote debug and check the console in the Safari devtools?

@robme

I don't have an iPad with me today, but emulating it with Chrome Dev tools, it seems that the onTouchStart event (and the other touch events) never actually fire. There are no errors.

@tony8891

I have the same problems but i could not prove to my colleagues
Uninstalled firefox completely ( erase all data also ) and it works again
Something is not right.........

@stevenaanen

I suffered the same issue, however, I think it might relate to the following:

cause

Firefox has the Responsive Design View as part of Developer Tools. It features touch gesture simulation using the mouse. When this has been used, Firefox enables touch events, but for some reason it doesn't always turn these back off after disabling it.

solution

Go to about:config, search for setting dom.w3c_touch_events.enabled and put it back to 0 if not already there. For me this was sufficient.

@mufaddalmw

https://github.com/miketaylr/FlexSlider/blob/8af7e0d02d332da4ca8e9dc094f6dd4aa3933d6d/jquery.flexslider.js
After replacing js file from above link, it works fine, Thanks for the fix!

@mufaddalmw

@miketaylr @mattyza Above solution make Flexslider work in firefox but stop swiping in touch screen devices, swiping is the main feature of slider or carousel that's why above solution is not appropriate.

Any other solutions ?

@miketaylr

Yeah, sorry--there's actually a small bug in my patch. Will try to update the PR today or tomorrow sorry!

@gudinne

I am also having the same issue as @mufaddalmw.

Thanks for your work on this @miketaylr!!

@miketaylr miketaylr referenced this issue from a commit in miketaylr/FlexSlider
@miketaylr miketaylr Fixes #958. Declare onTouchStart, onTouchEnd, onTouchMove as function…
… expressions.

This way they get hoisted properly and we don't rely on undefined
behavior (which doesn't work in some browsers).
40b8818
@miketaylr

@gudinne @mufaddalmw can you give the latest commit (miketaylr@40b8818) in my branch a test to see if that fixes it?

@mufaddalmw

I found one strange thing, yesterday I checked flexslider in windows 7 and mac which is having firefox 28.0 and it works great, seems like flexslider having issue in windows 8 firefox only.

@miketaylr @mattyza , Can you double check with this?

@mufaddalmw

Hi @miketaylr, I just checked your latest built, its working great in firefox with swipe support in touch devices. Thanks for your work, appreciated!

@robme

@miketaylr Your last commit is working for me in both Firefox and swipe on iPad/iPhone. Thanks.

@mufaddalmw
@miketaylr

Cool, thanks everyone for testing. Would be nice if this got merged... or is this project unmaintained?

@croggers55

I downloaded and installed the 2.2.2 version of jquery.flexslider.js. This fixed the next, prev, and paging controls, but in Firefox (Mac) 27 and 28 slides are still not transitioning by cross-fading (unless I configure with touch: false).

@shaekuronen shaekuronen referenced this issue in shaekuronen/jquery.flexloader
Closed

Not working on Firefox #1

@jehoshua02

@stevenaanen Thank you! I knew the problem had to be with Firefox, not the library or other browsers.

@elmaluf

I solved it copying the code from jQuery FlexSlider v2.2.2: jquery.flexslider-min.js in https://github.com/woothemes/FlexSlider/blob/master/jquery.flexslider-min.js

@fnagel fnagel referenced this issue from a commit in fnagel/FlexSlider
@miketaylr miketaylr Fixes #958. Declare onTouchStart, onTouchEnd, onTouchMove as function…
… expressions.

This way they get hoisted properly and we don't rely on undefined
behavior (which doesn't work in some browsers).
026db9d
@fnagel

Strongly recommend to add miketaylr@40b8818. Works for me on Android Tablet using Firefox 31.

@mrcoles

@miketaylr thank you very much for miketaylr@40b8818

It just saved me a great deal of time :smile:

@moraleida

miketaylr@40b8818 also worked for me on Android mobile running Firefox 31. Thanks!

@naved2

thanks Daynos it working for me :)

@karlcow

It would be nice to get @miketaylr fix into the release code.
It creates issues with site such as http://book.dmkt-sp.jp/top on Firefox Android.

@yareckon

Yeah, can you guys pull this in to master please? Using
miketaylr@40b8818 works, but for instance the .min version is not patched there.

@pribilinskiy

@miketaylr Please update the fix for v2.3.0

@miketaylr

@pribilinskiy happy to do so. @jeffikus: is there any chance of this getting merged into master?

@jeffikus jeffikus self-assigned this
@jeffikus jeffikus added this to the 2.5.0 milestone
@miketaylr

I just ran into this on another site in the wild (http://mobile.suntory.co.jp). Let me update my patch against master and update the PR.

@miketaylr miketaylr referenced this issue from a commit in miketaylr/FlexSlider
@miketaylr miketaylr Fixes #958. Declare onTouchStart, onTouchEnd, onTouchMove as function…
… expressions.

This way they get hoisted properly and we don't rely on undefined
behavior (which doesn't work in some browsers).
77a004d
@miketaylr miketaylr referenced this issue from a commit in miketaylr/FlexSlider
@miketaylr miketaylr Fixes #958. Declare onTouchStart, onTouchEnd, onTouchMove as function…
… expressions.

This way they get hoisted properly and we don't rely on undefined
behavior (which doesn't work in some browsers).
887f6ff
@miketaylr

Just rebased my PR @ #986 against the latest master.

@mattyza mattyza added the in progress label
@jeffikus
Collaborator

I did a new pull request #1335 for this on release branch 2-5-0 so I can further test this.

@jeffikus jeffikus closed this
@mattyza mattyza removed the in progress label
@miketaylr

Great! Thanks @jeffikus

@jeffikus
Collaborator

@miketaylr once again I'm really grateful for your pull request and contribution to the project :) super happy when I get stuff like this! :dancers:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.