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 update 55 in Android - fabric js zoom breaking #3687

Closed
vxrai opened this issue Feb 12, 2017 · 8 comments · Fixed by #3690
Closed

Chrome update 55 in Android - fabric js zoom breaking #3687

vxrai opened this issue Feb 12, 2017 · 8 comments · Fixed by #3690

Comments

@vxrai
Copy link

vxrai commented Feb 12, 2017

After updating to the latest version of chrome 55 in Android (mine is HTC 816, but the issue is reflecting in other devices as well), pinch zoom functionality is broken in the latest version of fabricjs.

Whats new in chrome update 55 related to touch events is here - https://developers.google.com/web/updates/2016/11/nic55

A similar issue was there in the jquery panzoom library after the latest chrome 55 update - timmywil/panzoom#310

On zooming an object on canvas, the entire browser responds to the event and zooms instead.
@kangax @asturur any clue about this issue?

@asturur
Copy link
Member

asturur commented Feb 12, 2017

can you try the latest master? i inserted something about the touch events that now are to be made passive.

@vxrai
Copy link
Author

vxrai commented Feb 13, 2017

HI @asturur thanks for your prompt reply.
I did that but still facing the same issue.
The object on canvas does zoom but also zooms the browser. Also the url bar of the browser slides up (disappears) when touch and drag the object upwards and when we touch and drag the object downwards the url browser bar slides down (appears).
It seems that the touch events on the canvas now conflict with the touch events of the browser.

@Rycochet
Copy link

It's a breaking change in Chrome latest version - hopefully they change their minds on it - but it's forcing document.addEventListener("touch*", fn, {passive:true}). See the discussion here - WICG/interventions#18

It's easy to replicate in desktop Chrome by opening the Dev panel, and turning on device emulation (the tablet / phone icon in the top left).

The end result of their change is that you cannot call event.preventDefault() in the event handler if it's attached to document or document.body, and it's almost impossible to work around in a cross-browser manner as a library. They suggest adding touch-action:none to the html/body element's css, but the simplest of tests prove this doesn't work.

@asturur
Copy link
Member

asturur commented Feb 13, 2017

So is what we did for one event but we have to do for them all. So all touch event. good. Doing it now.

setting { passive: false } and add also touch-action to the canvas since for edge is necessary i guess.

@Rycochet
Copy link

Unfortunately Edge (and IE) don't support an options argument for addEventListener(), in addition, some older browsers require a boolean useCapture argument - which are incompatible with each other - sadly I don't know which older browsers (and not really got the time to put together some tests on BrowserStack), but got a feeling it's either IE <9 or Safari on Windows - I sincerely hope that I'm wrong on that front.

@asturur
Copy link
Member

asturur commented Feb 13, 2017

yes but edge needs the touch-action.
I have a mail from a microsoft team that is pinging open source libs for this kind of changes.

@vxrai
Copy link
Author

vxrai commented Feb 13, 2017

Adding touch-action:none on the canvas didn't help in my case, but adding it in the main container worked for me.
@asturur - on which line we need to set { passive: false } ?

@asturur
Copy link
Member

asturur commented Feb 13, 2017

#3690

check this.
Need some change since the addEvent function in fabric does not expect options.
Nothing major. This evening probably 1.7.4 will be out with those changes and lot of minor bugfixing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants