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

html: Add tests to verify BigInt structured clone integration #9565

Merged
merged 1 commit into from Feb 26, 2018

Conversation

littledan
Copy link
Contributor

@littledan littledan commented Feb 17, 2018

These tests are against a PR which has not yet landed: whatwg/html#3480

littledan added a commit to littledan/html that referenced this pull request Feb 17, 2018
The goal of this PR is to integrate BigInt with HTML serialization.

It seems that all primitives are already serialized and deserialized
in the current specification, so this patch only adds serialization
for BigInt wrappers. Note that BigInt, like (the unserializable)
Symbol does not have a new-able constructor: use of wrappers is
explicitly discouraged by the specification. Nevertheless, this patch
adds serialization support for consistency with other wrappers.

web-platform-tests against postMessage on BigInt are out for review in
web-platform-tests/wpt#9565
@domenic
Copy link
Member

domenic commented Feb 17, 2018

It might be good to put this in a separate file, if it's not too much trouble, so that browsers which don't implement the new syntax don't start suddenly failing all existing tests due to a SyntaxError in the same file.

@w3c-bots
Copy link

w3c-bots commented Feb 17, 2018

Build PASSED

Started: 2018-02-26 09:20:19
Finished: 2018-02-26 09:29:41

View more information about this build on:

littledan added a commit to littledan/html that referenced this pull request Feb 17, 2018
The goal of this PR is to integrate BigInt with HTML serialization.

This patch
- Adds BigInt to the "safe list" of primitives permitted for
  serialization.
- Adds serialization of BigInt wrappers, analogous to other wrappers.

Note that BigInt, like (the unserializable) Symbol does not have a
new-able constructor: use of wrappers is explicitly discouraged by
the specification. Nevertheless, this patch adds serialization support
for consistency with other wrappers.

web-platform-tests against postMessage on BigInt are out for review in
web-platform-tests/wpt#9565
littledan added a commit to littledan/html that referenced this pull request Feb 18, 2018
The goal of this PR is to integrate BigInt with HTML serialization.

This patch
- Adds BigInt to the "safe list" of primitives permitted for
  serialization.
- Adds serialization of BigInt wrappers, analogous to other wrappers.

Note that BigInt, like (the unserializable) Symbol does not have a
new-able constructor: use of wrappers is explicitly discouraged by
the specification. Nevertheless, this patch adds serialization support
for consistency with other wrappers.

web-platform-tests against postMessage on BigInt are out for review in
web-platform-tests/wpt#9565
@littledan
Copy link
Contributor Author

littledan commented Feb 18, 2018

@domenic Good idea, split out into a separate file.

littledan added a commit to littledan/html that referenced this pull request Feb 18, 2018
The goal of this PR is to integrate BigInt with HTML serialization.

This patch
- Adds BigInt to the "safe list" of primitives permitted for
  serialization.
- Adds serialization of BigInt wrappers, analogous to other wrappers.

Note that BigInt, like (the unserializable) Symbol does not have a
new-able constructor: use of wrappers is explicitly discouraged by
the specification. Nevertheless, this patch adds serialization support
for consistency with other wrappers.

web-platform-tests against postMessage on BigInt are out for review in
web-platform-tests/wpt#9565
@littledan
Copy link
Contributor Author

These tests are missing verification that BigInt and BigInt wrappers can be used as IndexedDB values (but not (yet) keys), although the PR in whatwg/html#3480 permits them to be used as such. The testing will come in a separate PR (once I figure out any other changes for BigInt that IndexedDB might need).

littledan added a commit to littledan/html that referenced this pull request Feb 22, 2018
The goal of this PR is to integrate BigInt with HTML serialization.

This patch
- Adds BigInt to the "safe list" of primitives permitted for
  serialization.
- Adds serialization of BigInt wrappers, analogous to other wrappers.

Note that BigInt, like (the unserializable) Symbol does not have a
new-able constructor: use of wrappers is explicitly discouraged by
the specification. Nevertheless, this patch adds serialization support
for consistency with other wrappers.

web-platform-tests against postMessage on BigInt are out for review in
web-platform-tests/wpt#9565
littledan added a commit to littledan/html that referenced this pull request Feb 23, 2018
The goal of this PR is to integrate BigInt with HTML serialization.

This patch
- Adds BigInt to the "safe list" of primitives permitted for
  serialization.
- Adds serialization of BigInt wrappers, analogous to other wrappers.

Note that BigInt, like (the unserializable) Symbol does not have a
new-able constructor: use of wrappers is explicitly discouraged by
the specification. Nevertheless, this patch adds serialization support
for consistency with other wrappers.

web-platform-tests against postMessage on BigInt are out for review in
web-platform-tests/wpt#9565
domenic pushed a commit to whatwg/html that referenced this pull request Feb 26, 2018
* Adds BigInt to the safelist of primitives permitted for serialization
* Adds serialization of BigInt wrappers, analogous to other wrappers

Tests: web-platform-tests/wpt#9565
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could definitely be simplified and modernized but I think as long as we add a comment explaining that it's just aping its sibling this'll do fine.

<head>
<meta content="text/html; charset=utf-8" http-equiv="content-type" />
<title>2.7 Safe passing of structured data</title>
<link rel="help" href="http://www.w3.org/TR/html5/common-dom-interfaces.html#safe-passing-of-structured-data" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
</head>
<body>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a note that this is supposed to be similar to its sibling, but is separate to avoid JS syntax errors? That'll help people who wonder at the strange style of the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@littledan
Copy link
Contributor Author

What's the convention here--should I land this myself now that it's been approved (seems like I have permissions), or wait for a maintainer to land it?

@annevk annevk merged commit c92168a into web-platform-tests:master Feb 26, 2018
@annevk
Copy link
Member

annevk commented Feb 26, 2018

You should feel free to land it yourself in the future. Please do include links to the standard change PR in the commit message as I did when landing. Makes it easier to understand things in the future.

alice pushed a commit to alice/html that referenced this pull request Jan 8, 2019
* Adds BigInt to the safelist of primitives permitted for serialization
* Adds serialization of BigInt wrappers, analogous to other wrappers

Tests: web-platform-tests/wpt#9565
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants