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

respondWith uses the wrong targetRealm #1055

Open
isonmad opened this issue Jan 16, 2017 · 4 comments
Open

respondWith uses the wrong targetRealm #1055

isonmad opened this issue Jan 16, 2017 · 4 comments

Comments

@isonmad
Copy link
Contributor

isonmad commented Jan 16, 2017

Since 6c2b3f1 both FetchEvent.respondWith and ForeignFetchEvent.respondWith say

   1. Let |targetRealm| be the <a>relevant Realm</a> of the <a>context object</a>.

But the relevant Realm of the context object is always going to be the service worker itself, isn't it?

It looks like targetRealm should be 'event.request's request's client's Realm.'

@jakearchibald
Copy link
Contributor

But the relevant Realm of the context object is always going to be the service worker itself, isn't it?

Looking at the spec, I think that's the intention. @jungkees?

@mkruisselbrink
Copy link
Collaborator

I'm not sure... looking at the PR that added this text it seems the goal was to somehow transfer the stream to a different realm, which isn't what the current text does. @yutakahirano or @domenic probably know best what was intended?

@isonmad
Copy link
Contributor Author

isonmad commented Apr 1, 2017

Looking at the spec, I think that's the intention.

How do you figure that? It makes no sense to copy from a ReadableStream that already exists in the service worker to a second, newly constructed ReadableStream... that is also created in the service worker.

Isn't the whole point of respondWith supposed to send a Response from the SW to the client, not to itself?

@domenic
Copy link
Contributor

domenic commented Apr 24, 2017

This indeed looks like a bug. The idea is to send it to the client's realm. The fix @isonmad suggests in the OP seems correct to me (I guess "this FetchEvent's request's request's client's Realm" is the most correct way of getting at things.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants