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

clarify when FetchEvent.clientId will or will not be set for navigations #1267

Open
wanderview opened this issue Jan 31, 2018 · 6 comments
Open

Comments

@wanderview
Copy link
Member

wanderview commented Jan 31, 2018

Over in #1266 the question of when exactly FetchEvent.clientId will or will not be populated for navigations came up.

It seems it must not be populated for:

  • If the navigation is initiated from a cross-origin Client
  • If there is no Client at all, like the user entering a URL in the tab and pressing enter.

We could potentially should a clientId for:

  • Same-origin client setting window.location
  • Same-origin client setting iframe.src
  • Same-origin client calling window.open()
  • Same-origin client calling clients.openWindow()

Edit: Also:

  • Client calling new Worker()
  • Client calling new SharedWorker() where no one else is attached yet
@wanderview
Copy link
Member Author

From an implementation point of view I'm a little uneasy overloading FetchEvent.clientId to mean both:

  • If a subresource requests, then this is the client that determines all permissions, CSP, referrer policy, etc.
  • If a non-subresource requests, then this is the client that started it, but it doesn't set any policy on this request.

Its just a bit non-ideal to try to use the same information for policy in one case, but exempt it from that policy in others. But maybe thats an implementation specific concern.

@mfalken
Copy link
Member

mfalken commented Jan 31, 2018

I kind of like the simplicity of clientId just being empty/null for all navigations. Do we know if people are really asking for a non-empty clientId for these cases? We might just save ourselves time/complexity by sticking with empty/null.

The big use case I'm hearing for the new clientId stuff is to implement resultingClientId, so you can track the whole page load (main resource + its subresources).

I guess Workers are an interesting point, though.

@wanderview
Copy link
Member Author

I believe its a use case @jakearchibald wants. We'll have to wait from him to surface from holiday to tell us, though.

@jungkees
Copy link
Collaborator

jungkees commented Feb 1, 2018

I agree the design decision depends on the use cases.

Just to clarify, the assumption I had is simply use the request's client for clientId. (Wasn't this the very reason why we added resultingClientId? Otherwise, we could use clientId for both subresource requests and non-subresource requests as @slightlyoff suggested.) For navigation requests, it should be the environment of the source browsing context's active document. For worker requests, it should be the environment where the Worker/SharedWorker constructors are called. So they can be retrieved from:

If the navigation is initiated from a cross-origin Client

There should be a source browsing context for this type of navigation requests although the client API won't be able to get a client instance from the clientId.

If there is no Client at all, like the user entering a URL in the tab and pressing enter.

The tab should have a Window and an environment at the event of pressing enter. But there's no definition what the source browsing context should be for this navigation request I guess. So, clientId should be the empty string unless we define what it should be otherwise.

Same-origin client setting window.location

The responsible browsing context specified by the incumbent settings object.

Same-origin client setting iframe.src

iframe element's node document's browsing context.

Same-origin client calling window.open()

The responsible browsing context specified by entry settings object.

Same-origin client calling clients.openWindow()

We should define this; it should be the newly created browsing context that the method navigate.

Client calling new Worker()

The environment where the Worker constructor is called.

Client calling new SharedWorker() where no one else is attached yet

The environment where the Worker constructor called.

For the other thoughts:

Its just a bit non-ideal to try to use the same information for policy in one case, but exempt it from that policy in others. But maybe thats an implementation specific concern.

I think that's rather natural based on the distinction between subresource requests and non-subresource (main resource) requests?

I kind of like the simplicity of clientId just being empty/null for all navigations.

If this is the case, dropping resultingClientId in favor of having clientId represent either a subresource request's client or a non-subresource request's client can be an option I guess.

Do we know if people are really asking for a non-empty clientId for these cases?

This seems like the key for decision.

@wanderview
Copy link
Member Author

If this is the case, dropping resultingClientId in favor of having clientId represent either a subresource request's client or a non-subresource request's client can be an option I guess.

I don't think we should try to merge the properties back together. They are semantically different and it doesn't cost anything to have two properties.

My inclination is to probably implement resultingClientId and keep navigation clientId empty at first.

Also note, exposing the clientId on navigations is a bit like referrer restricted to same-origin. It should probably respect the referrer policy. If the policy is set to no-referrer then I don't think we should ever set a navigation FetchEvent.clientId.

@wanderview
Copy link
Member Author

Also note, exposing the clientId on navigations is a bit like referrer restricted to same-origin. It should probably respect the referrer policy. If the policy is set to no-referrer then I don't think we should ever set a navigation FetchEvent.clientId.

Well, maybe it would be ok for iframes since you can get at the same-origin parent in that case.

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

No branches or pull requests

3 participants