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

Specify realm for object creation #1213

Open
domenic opened this issue Jan 24, 2022 · 8 comments
Open

Specify realm for object creation #1213

domenic opened this issue Jan 24, 2022 · 8 comments
Assignees

Comments

@domenic
Copy link
Member

domenic commented Jan 24, 2022

As pointed out by @mgaudet in Matrix, the spec is not clear on what realms things should be created in, and the result is not interoperable across browsers.

To audit:

  • All references to Web IDL "new"
  • All references to "a new promise" / "a promise rejected with" / "a promise resolved with"
  • All raw ES-spec object creation using %% intrinsics, although some of these might be able to be replaced with the stuff introduced in Add and improve operations on BufferSources webidl#987

Per whatwg/webidl#135 the intention is to use the relevant realm of this. Alternately we could fix that issue at its source per whatwg/webidl#135 (comment) ... that's quite tempting.

@domenic domenic self-assigned this Jan 24, 2022
@ricea
Copy link
Collaborator

ricea commented Jan 25, 2022

Currently Blink's implementation is quite confused here, so it would be nice to have it clarified.

@minfrin
Copy link

minfrin commented Apr 16, 2022

Spent a while hitting my head on this, and not found a way around it yet:

https://bugzilla.mozilla.org/show_bug.cgi?id=1757066

The two errors encountered are:

TypeError: controller.enqueue is not a function
Error: Permission denied to access property "then"

Streams can't work with web extensions on Firefox, which is a real pity - as streams are needed to handle the arbitrary size limits in the web extensions API.

@ricea
Copy link
Collaborator

ricea commented Apr 18, 2022

TypeError: controller.enqueue is not a function Error: Permission denied to access property "then"

Streams can't work with web extensions on Firefox, which is a real pity - as streams are needed to handle the arbitrary size limits in the web extensions API.

This seems to be an issue with web extensions on Firefox rather than anything we can fix in the streams standard.

@saschanaz
Copy link
Member

saschanaz commented Mar 30, 2023

The new test in web-platform-tests/wpt#38875 implicitly requires TransformStream to be created with the caller realm, which doesn't match with Gecko's this behavior. Perhaps we should finally get some normative spec here.

That said, what should happen when creating a promise with a detached realm?

@ricea
Copy link
Collaborator

ricea commented Mar 31, 2023

The intent of the test is that it be created in the realm of the constructor. Probably we should add some comments.

Edit: Incidentally, the point of the test is that Chromium fails it, so we haven't actually tested the success case. The Chromium-only test https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/wpt_internal/streams/transform-streams/invalid-realm.window.js documents our actual behaviour.

@saschanaz
Copy link
Member

saschanaz commented Apr 3, 2023

But wpt.fyi says it passes on Chrome? (Adding a failing test sounds weird, such test should at least be marked as tentative.)

@ricea
Copy link
Collaborator

ricea commented Apr 4, 2023

But wpt.fyi says it passes on Chrome? (Adding a failing test sounds weird, such test should at least be marked as tentative.)

The test previously worked in Chrome, but updating our implementation to use read requests broke it (we switched from using promises to enqueuing microtasks directly, and it turns out that has different semantics in V8). Currently the Chrome dev channel is still on version 113, which is why it shows as passing on wpt.fyi. It will show as failing once the dev channel moves on to version 114.

As the test relies on behaviour that is not yet specced, I think you are right that it should be tentative. However, in general I think when we find a test gap it's good to write a test for it, even if for some reason we can't pass that test.

@saschanaz
Copy link
Member

As the test relies on behaviour that is not yet specced, I think you are right that it should be tentative. However, in general I think when we find a test gap it's good to write a test for it, even if for some reason we can't pass that test.

+1. For now I'd like to mark it as tentative to prevent confusion from our side.

saschanaz added a commit to web-platform-tests/wpt that referenced this issue Apr 4, 2023
ricea pushed a commit to web-platform-tests/wpt that referenced this issue Apr 4, 2023
cookiecrook pushed a commit to cookiecrook/wpt that referenced this issue Apr 8, 2023
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 15, 2023
…d-realm.tentative.window.js, a=testonly

Automatic update from web-platform-tests
Rename invalid-realm.window.js to invalid-realm.tentative.window.js (#39353)

This part is not specced nor has a consensus. See also whatwg/streams#1213
--

wpt-commits: bff237ce580756b51b90d22e21745e10e7fcc177
wpt-pr: 39353
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants