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

Firefox 26.0 onTouchStart vs ontouchstart #958

Closed
ghost opened this Issue Dec 19, 2013 · 42 comments

Comments

Projects
None yet
@ghost
Copy link

ghost commented Dec 19, 2013

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

This comment has been minimized.

Copy link

Daynos commented Jan 6, 2014

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 added a commit to Daynos/FlexSlider that referenced this issue Jan 6, 2014

@Daynos Daynos referenced this issue Jan 6, 2014

Open

Fix Issue #958 #970

@robme

This comment has been minimized.

Copy link

robme commented Jan 16, 2014

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

@mattyza

This comment has been minimized.

Copy link
Member

mattyza commented Jan 16, 2014

There are plans, yes.

If you have a fix, please do submit a pull request on the “master” branch.

Thanks. :)

Matt Cohen
Chief Product Officer at WooThemes

http://woothemes.com/
http://matty.co.za/

On Thursday 16 January 2014 at 1:24 PM, Rob wrote:

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


Reply to this email directly or view it on GitHub (#958 (comment)).

@miketaylr

This comment has been minimized.

Copy link
Contributor

miketaylr commented Jan 16, 2014

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 added a commit to miketaylr/FlexSlider that referenced this issue Jan 16, 2014

Fixes woocommerce#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).
@robme

This comment has been minimized.

Copy link

robme commented Jan 20, 2014

I tested your fix and it works fine for me.

@amwelles

This comment has been minimized.

Copy link

amwelles commented Jan 21, 2014

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

@miketaylr

This comment has been minimized.

Copy link
Contributor

miketaylr commented Jan 21, 2014

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

@banago

This comment has been minimized.

Copy link

banago commented Jan 21, 2014

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

This comment has been minimized.

Copy link

hrod commented Feb 3, 2014

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

miketaylr added a commit to miketaylr/FlexSlider that referenced this issue Feb 3, 2014

Fixes woocommerce#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).

miketaylr added a commit to miketaylr/FlexSlider that referenced this issue Feb 3, 2014

Fixes woocommerce#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).
@miketaylr

This comment has been minimized.

Copy link
Contributor

miketaylr commented Feb 3, 2014

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

@robme

This comment has been minimized.

Copy link

robme commented Feb 4, 2014

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

This comment has been minimized.

Copy link

hrod commented Feb 4, 2014

Ditto what robme said.

@miketaylr

This comment has been minimized.

Copy link
Contributor

miketaylr commented Feb 4, 2014

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

@robme

This comment has been minimized.

Copy link

robme commented Feb 5, 2014

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

This comment has been minimized.

Copy link

tony8891 commented Mar 7, 2014

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

This comment has been minimized.

Copy link

stevenaanen commented Mar 8, 2014

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

This comment has been minimized.

Copy link

mufaddalmw commented Mar 19, 2014

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

This comment has been minimized.

Copy link

mufaddalmw commented Mar 20, 2014

@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

This comment has been minimized.

Copy link
Contributor

miketaylr commented Mar 20, 2014

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

@gudinne

This comment has been minimized.

Copy link

gudinne commented Mar 20, 2014

I am also having the same issue as @mufaddalmw.

Thanks for your work on this @miketaylr!!

miketaylr added a commit to miketaylr/FlexSlider that referenced this issue Mar 21, 2014

Fixes woocommerce#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).
@miketaylr

This comment has been minimized.

Copy link
Contributor

miketaylr commented Mar 21, 2014

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

@mufaddalmw

This comment has been minimized.

Copy link

mufaddalmw commented Mar 23, 2014

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

This comment has been minimized.

Copy link

mufaddalmw commented Mar 26, 2014

Yeah thats correct.
Thanks Mike.
On 26 Mar 2014 15:17, "Rob" notifications@github.com wrote:

@miketaylr https://github.com/miketaylr Your last commit is working in
both Firefox and swipe on iPad/iPhone. Thanks.

Reply to this email directly or view it on GitHubhttps://github.com//issues/958#issuecomment-38672139
.

@miketaylr

This comment has been minimized.

Copy link
Contributor

miketaylr commented Mar 26, 2014

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

@croggers55

This comment has been minimized.

Copy link

croggers55 commented Apr 4, 2014

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).

@jehoshua02

This comment has been minimized.

Copy link

jehoshua02 commented Jun 19, 2014

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

@elmaluf

This comment has been minimized.

Copy link

elmaluf commented Jul 11, 2014

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 added a commit to fnagel/FlexSlider that referenced this issue Aug 11, 2014

Fixes woocommerce#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).
@fnagel

This comment has been minimized.

Copy link

fnagel commented Aug 11, 2014

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

@mrcoles

This comment has been minimized.

Copy link

mrcoles commented Aug 14, 2014

@miketaylr thank you very much for miketaylr@40b8818

It just saved me a great deal of time 😄

@moraleida

This comment has been minimized.

Copy link

moraleida commented Aug 30, 2014

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

@naved2

This comment has been minimized.

Copy link

naved2 commented Dec 5, 2014

thanks Daynos it working for me :)

@karlcow

This comment has been minimized.

Copy link

karlcow commented Dec 9, 2014

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

This comment has been minimized.

Copy link

yareckon commented Dec 30, 2014

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

@pribilinskiy

This comment has been minimized.

Copy link

pribilinskiy commented Feb 14, 2015

@miketaylr Please update the fix for v2.3.0

@miketaylr

This comment has been minimized.

Copy link
Contributor

miketaylr commented Feb 23, 2015

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

@jeffikus jeffikus self-assigned this Feb 24, 2015

@jeffikus jeffikus added this to the 2.5.0 milestone Feb 24, 2015

@miketaylr

This comment has been minimized.

Copy link
Contributor

miketaylr commented Apr 28, 2015

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 added a commit to miketaylr/FlexSlider that referenced this issue Apr 28, 2015

Fixes woocommerce#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).

miketaylr added a commit to miketaylr/FlexSlider that referenced this issue Apr 28, 2015

Fixes woocommerce#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).
@miketaylr

This comment has been minimized.

Copy link
Contributor

miketaylr commented Apr 28, 2015

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

@mattyza mattyza added the Build Tools label May 19, 2015

jeffikus added a commit that referenced this issue May 19, 2015

Merge pull request #1335 from miketaylr/958-firefox-fix
Fixes #958. Declare onTouchStart, onTouchEnd, onTouchMove as function…
@jeffikus

This comment has been minimized.

Copy link
Member

jeffikus commented May 19, 2015

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 May 19, 2015

@mattyza mattyza removed the Build Tools label May 19, 2015

@miketaylr

This comment has been minimized.

Copy link
Contributor

miketaylr commented May 19, 2015

Great! Thanks @jeffikus

@jeffikus

This comment has been minimized.

Copy link
Member

jeffikus commented May 20, 2015

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

floq-design added a commit to floq-design/FlexSlider that referenced this issue Aug 5, 2015

Merge commit '8ed61acca0a1c1f1242ebb0cdb8291be6d4a0d91' into sass
* commit '8ed61acca0a1c1f1242ebb0cdb8291be6d4a0d91':
  Closes woocommerce#843 - adds npm components and package json for registry with npm. Watch for new 2.5.0 tag before using.
  Closes woocommerce#843 - adds bower components and bower json for registry with bower. Watch for new 2.5.0 tag before using.
  Closes woocommerce#819 - Adds caption demo file
  Closes woocommerce#1321 - adds license file GPLv2 and later
  Merge changes between demo site on svn and git repo.
  Closes woocommerce#1143 - adds customDirectionNav param, fixes demo's and adds new demo for custom direction navigation controls. Updates changelog and readme.
  Updates changelog, readme, bumps support for jQuery 1.4.2 to 1.7+ Closes woocommerce#1312
  Closes woocommerce#1293 - CSS fix for pausePlay play icon.
  Working branch pushed to GitHub. Version updated.
  Fixes woocommerce#958. Declare onTouchStart, onTouchEnd, onTouchMove as function expressions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment