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

Cesium Viewer crashes on MacOS (Safari) and iOS (all) with rapid mouse movement #5691

Closed
mramato opened this issue Jul 28, 2017 · 5 comments
Closed
Labels
good first issue An opportunity for first time contributors type - bug

Comments

@mramato
Copy link
Contributor

mramato commented Jul 28, 2017

The Viewer reference app uses replaceState pretty much every time the camera moves, this leads to a security error on all Safari versions and results in a crash. Just move them camera a whole bunch and you'll crash. Basically, you aren't allowed to call replaceState or otherwise much with history more than 100 times over 30 seconds.

While Apple having a check like this is questionable, we probably shouldn't be calling it that often to begin with.

NOTE: This only applies to the CesiumViewer reference app, which updates the URL to match the camera position, not the Viewer widget.

@mramato mramato added good first issue An opportunity for first time contributors type - bug labels Jul 28, 2017
avalan4e57 added a commit to avalan4e57/cesium that referenced this issue Jul 31, 2017
The issue is about fixing safari crash on mouse move because of using
replaceState to often.

There was a syntax error in using setTimeout function where interval
value was on the third place in func parameters list when it should
be on second.
@avalan4e57
Copy link

Hello. I'm new to contributing on open source projects. Don't know if I got you exactly, but there's a pull request with how I see the fix of the problem. By the way I can't test the stuff locally because I get this type of errors in browser js console error for: Cesium/Shaders/.... A 404 one if to be exact. Can you please help me with that?

@hpinkos
Copy link
Contributor

hpinkos commented Jul 31, 2017

Thanks for looking into the problem @avalan4e57! For running the code locally, see the information in our Build Guide. When you're ready to open a pull request, take a look at our contributing documentation for guidelines.

I saw your commit and I think that's a good start (that code is clearly incorrect and you're change fixes that), but I think we can make it even better by listening to the Camera.changed event instead of moveStart/moveEnd, and using a setTimeout/clearTimeout instead of a setInterval to make sure replaceState is only called at most once a second.

And yes, the camera parameter passed to the setInterval function looks like it can be removed

avalan4e57 added a commit to avalan4e57/cesium that referenced this issue Jul 31, 2017
Some fixes to previous commit as @hpinkos recommended

Replaced eventListener moveStart / moveEnd with Camera.changed
event listener. Replaced setInterval with setTimeout function in
that event.
@avalan4e57
Copy link

Thanks for reply @hpinkos ! Everything works ok on my local now. Made some changes there. And there's a question in the comment to some code lines in the commit. So, if everything is ok, guess I need to make a pull request since I've read already contributing documentation?

@hpinkos
Copy link
Contributor

hpinkos commented Jul 31, 2017

@avalan4e57 Go ahead and open the pull request. It'll be easier for us to review it that way instead of looking at your individual commits

avalan4e57 added a commit to avalan4e57/cesium that referenced this issue Jul 31, 2017
Set 'updateTimer = undefined' after 'clearTimeout(updateTimer)' call
@avalan4e57
Copy link

avalan4e57 commented Aug 1, 2017

Made pull request #5699 yesterday. Maybe I should mention about it here.

hpinkos pushed a commit that referenced this issue Aug 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue An opportunity for first time contributors type - bug
Projects
None yet
Development

No branches or pull requests

3 participants