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

Test current Realm (still to be defined in more detail) #1381

Merged
merged 2 commits into from
Oct 13, 2016
Merged

Conversation

annevk
Copy link
Member

@annevk annevk commented Nov 11, 2014

Create a test based on http://web.mit.edu/bzbarsky/www/testcases/global-object-association/createImageData.html to see what the global object is for various things.

@hoppipolla-critic-bot
Copy link

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

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.

@annevk
Copy link
Member Author

annevk commented Nov 11, 2014

@bzbarsky this is what we discussed yesterday I think. It seems browsers are actually fairly consistent apart from that one thing in Chrome. Perhaps @inexorabletash has some insight into that.

(Also curious, Safari throws a DOMException "NotSupportedError" for createProcessingInstruction.)

@bzbarsky
Copy link
Contributor

@annevk Thanks for writing up this test!

Some other things I suspect are worth checking:

  1. The global of crypto API Promise return values when methods from one global are applied to the crypto.subtle of another global.

  2. The global of FontFace/FontFaceSet promises.

  3. The global of getElementsByTagName return values.

  4. The global of NamedNodeMap.prototype.getNamedItem return values.

  5. The return value of getImageData (instead of createImageData).

  6. The return value of things like createImageBitmap where it's supported; both the Promise returned and the value the Promise is eventually resolved with. In particular, whether it depends on the global of the argument.

  7. The global of the return value of HTMLCanvasElement.prototype.getContext.

@annevk
Copy link
Member Author

annevk commented Nov 12, 2014

@bzbarsky it seems FontFace's load() method would just return the object itself so not useful to test. Crypto does the same as all other methods. createImageBitmap and FontFaceSet do not appear to be supported.

https://dump.testsuite.org/wpt/current-realm-promises.html tests an asynchronous crypto method and createImageBitmap.

@bzbarsky
Copy link
Contributor

FontFace's load() returns a Promise which fulfills to the FontFace itself. So testing the global of the fulfillment value is not interesting. But testing the global of the Promise is.

@annevk
Copy link
Member Author

annevk commented Nov 12, 2014

@bzbarsky fair, added. Seems to behave as the other methods in Chrome. Fails in Firefox of course.

@bzbarsky
Copy link
Contributor

@annevk Try testing with the "layout.css.font-loading-api.enabled" preference set to true in Firefox.

@annevk
Copy link
Member Author

annevk commented Nov 12, 2014

@bzbarsky same as other methods with that enabled.

@annevk
Copy link
Member Author

annevk commented Nov 20, 2015

Rebased and squashed this. @bzbarsky, should we commit this? I don't really remember the context anymore and whether Web IDL covers this already or not.

@bzbarsky
Copy link
Contributor

@annevk The context is that IDL requires specs to define which global their objects are associated with but specs by and large don't do that. UA behavior is not terribly consistent in practice on some of the more interesting cases.

So we need to get several things to happen:

  1. Specs actually define the behavior.
  2. Tests test for that behavior.
  3. UAs implement the behavior.

The PR here is for item #2 of that, right?

@annevk
Copy link
Member Author

annevk commented Nov 20, 2015

It is, but I also remember having a proposal for how 1 could be largely solved by IDL except for a few cases. I think that's why we ended up writing these tests, but unfortunately I didn't link from this PR to that discussion.

@bzbarsky
Copy link
Contributor

Item 1 (God, I wish github didn't mangle '#' for me) can mostly be solved by saying that the associated global of an object created via a DOM API call is the associated global of the "this" value, I think. Trying to recall what the exceptions to that would be....

Or were we talking about using the callee function Realm except for some exceptions?

@annevk
Copy link
Member Author

annevk commented Nov 20, 2015

A DOM API call being a call going through IDL? Anyway, I don't remember what exactly it was we thought we should do here. Just that I wanted IDL to handle it and you wanted to see tests first.

@bzbarsky
Copy link
Contributor

Well, I wanted to see what browsers do right now, so we could evaluate what the handling in IDL should look like.

@annevk
Copy link
Member Author

annevk commented Nov 23, 2015

https://www.w3.org/Bugs/Public/show_bug.cgi?id=24652 is the corresponding specification bug for this set of tests.

@bzbarsky
Copy link
Contributor

To add to the list from #1381 (comment):

  1. The global of the async-created key returned from WebCrypto stuff. Note that webcrypto is known non-interoperable across browsers; see https://www.w3.org/Bugs/Public/show_bug.cgi?id=29514

@annevk
Copy link
Member Author

annevk commented May 30, 2016

@jgraham according to @bzbarsky we routinely land tests not covered by standards. Perhaps this could be one of those?

@ayg ayg closed this Aug 28, 2016
@ayg ayg deleted the current-Realm branch August 28, 2016 13:58
@annevk
Copy link
Member Author

annevk commented Aug 28, 2016

Please explain what you did here @ayg.

@ayg
Copy link
Contributor

ayg commented Aug 28, 2016

@annevk I accidentally did git push --mirror to the w3c repo instead of mine, due to incorrect .git/config (since fixed), and didn't have any idea how to fix it. I e-mailed @jgraham and @Ms2ger because they were the people I thought of off the top of my head, but if you know how to fix it, please do.

@annevk
Copy link
Member Author

annevk commented Aug 28, 2016

No 😟

@gsnedders gsnedders restored the current-Realm branch August 28, 2016 20:16
@gsnedders
Copy link
Member

15:01 < aryehgregor> Um, I might have done something wrong by mistake.
15:01 < aryehgregor> I think I pushed to the wrong repo?

@annevk
Copy link
Member Author

annevk commented Oct 13, 2016

Going to land this per discussion with @jgraham on IRC (#whatwg).

@annevk annevk merged commit 1d81fe1 into master Oct 13, 2016
@annevk annevk deleted the current-Realm branch October 13, 2016 14:56
@web-platform-tests web-platform-tests deleted a comment from Anamazina Nov 14, 2022
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.

7 participants