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

IndexedDB: Skip keypath tests for non-cloneable values #1411

Merged
merged 1 commit into from Nov 25, 2014
Merged

IndexedDB: Skip keypath tests for non-cloneable values #1411

merged 1 commit into from Nov 25, 2014

Conversation

inexorabletash
Copy link
Contributor

Per discussion in https://www.w3.org/Bugs/Public/show_bug.cgi?id=26492 we probably want to clone before extracting keys using keypaths. Chrome already does this, other implementers seem to agree. The spec is written in a non-algorithmic style, so it's ambiguous.

With this change, all potential key types are still tested as explicit keys. But a secondary test, embedding potential key values into an object and using implicit keys via key paths, is not done if the value is not cloneable, since it could fail with either DataCloneError (if cloning is done before extraction) or DataError (if cloning is done after extraction).

@hoppipolla-critic-bot
Copy link

Critic review: https://critic.hoppipolla.co.uk/r/3276

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

zqzhang added a commit that referenced this pull request Nov 25, 2014
IndexedDB: Skip keypath tests for non-cloneable values
@zqzhang zqzhang merged commit 8021caa into web-platform-tests:master Nov 25, 2014
@inexorabletash inexorabletash deleted the inexorabletash/key_invalid branch December 5, 2014 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants