-
Notifications
You must be signed in to change notification settings - Fork 1
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
Fix check for support of structured clone #10
Conversation
Check was initially successful, but failed to remove its handler. On subsequent activity, the check for a Safari 5.1 defect would reset the structured clone supported flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you check if adding the handler is needed at all?
Can we just call postMessage and check for an error?
Works fine without the handler and is cleaner. Unit tests run. Runs fine in CODAP. I don't quite know how to exercise the failure case, though. |
Now, the checker sends an illegal message to itself and expects an exception to be thrown. It examines the result for keywords.
I've tried to decipher the caniuse details more. It seems too hard to figure out how to test the failure case. Basically we'd need Firefox version 5 or earlier I think. Since we are checking based on an exception and the whole bit of Safari code was because Safari actually doing the structure clone, but wasn't throwing the exception. And since the throwing of that exception is not something that is tested by the caniuse folks. It is really hard to know what we should actually test. Sorry for such a run around on this, but I now thing we should just drop the structured clone check all together. No one has responded on dev slack channel. As long as we are updating this code and we are going to have to do some testing of multiple browsers around it, we might as well go all of the way. |
Suggests current Edge doesn't support Structured Clone, but I haven't yet investigated further... |
I've reviewed: Modernizr/Modernizr#388 The tests linked above by @jsandoe provide a comprehensive view of structured clone support on the latest builds of the major desktop browsers. They generally seem to support it quite well. Unfortunately I can't tell if there is a way to use this site to find out how well and how long the stable versions of browsers have supported it. Anyhow I think we might as well stick with this PR as is and see how it works. At worst it should use strings on old versions of safari when it should be using structured clone. And previously all browsers were using strings because of the issue in the safari code. |
The structured clone check was initially successful, but failed to remove its handler.
On subsequent activity, the check for a Safari 5.1 defect would
reset the structured clone supported flag.