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

what happens if an initial about:blank inherits a controller, but its navigation request does not match any service worker? #1232

Open
wanderview opened this issue Nov 22, 2017 · 14 comments

Comments

@wanderview
Copy link
Member

wanderview commented Nov 22, 2017

Consider the following situation.

// -------
// foo.com/scope1/index.html
<iframe id="child" srv="/scope2/index.html"></iframe>
<script>
  // controlled by a service worker with scope foo.com/scope1
  assert(!!navigator.serviceWorker.controller)

  let frame = document.getElementById('child');

  // twiddle a value on the global
  frame.contentWindow.foo = 'bar';

  // the initial about:blank should inherit the controller
  assert(frame.contentWindow.navigator.serviceWorker.scriptURL ===
             navigator.serviceWorker.controller.scriptURL);

// -----------
// foo.com/scope2/index.html
// no service worker controlling this scope

// does this log "bar" as we would normally expect after sharing the about:blank global?
console.log(self.foo);

// does this log null or undefined as it normally would if the page was loaded without a parent
console.log(navigator.serviceWorker.controller);
</script>

This situation normally requires replacing the about:blank document with the final document so the global is shared. You can see this with the foo global being passed through the load.

We also agreed to inherit the service worker to initial about:blank windows, though.

When the final load is not under a controlled scope, but the initial about:blank was controlled via parent inheritance, what should the final navigator.serviceWorker.controller be?

The natural answer is "clear the controller" if the final load does not make a scope. This is somewhat unprecedented, though. Nowhere else in the system do we allow a client to go from controlled to uncontrolled. There is no event or anything that this is happening. We would also have to deal with the page stashing the controller reference in another global property, etc.

I'd like to suggest that we require a new global to be created in this situation. If the initial about:blank is controlled, but the final load should not be controlled, then we don't do the document replacement. Instead we treat it similar to other cases (cross-origin, etc) where a new global is created.

@jakearchibald @jungkees @annevk What do you think?

@wanderview
Copy link
Member Author

To clarify, what I'm proposing would result in both iframe console.log() statements printing undefined or null.

@annevk
Copy link
Member

annevk commented Nov 22, 2017

@bzbarsky might have an opinion, but in principle this seems fine.

(We could also associate the controller with a document... though then in Firefox document.open() might be problematic with it replacing the global, but not the document.)

@wanderview
Copy link
Member Author

I would prefer not to associate the controller with the document. Even if we did that, though, we still have the issues with the no other operation going from controlled to uncontrolled state. Its pretty much the same problem as just clearing the controller from the global.

@bzbarsky
Copy link

I suspect not reusing the global in this case should be OK.

The other option would be to try to change the spec model of iframe loading such that the case above creates a document with the scope2/index.html URL to start with, and it's not controlled to start with. I believe that IE/Edge do something like that. Doing that would require some careful thought and compat testing and so forth though....

@wanderview
Copy link
Member Author

FWIW, here is the gecko bug to handle this:

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

The other option would be to try to change the spec model of iframe loading such that the case above creates a document with the scope2/index.html URL to start with, and it's not controlled to start with. I believe that IE/Edge do something like that. Doing that would require some careful thought and compat testing and so forth though....

The initial about:blank stuff is already rife with interop problems. I really don't think I have time to take this sort of thing on.

I'd like to proceed with disabling global re-use when going from controlled to uncontrolled for now. Its a somewhat niche case and seems the safest thing to do for now.

@jungkees
Copy link
Collaborator

I feel a bit unconfortable about making the SW matching affect the decision of the client reuse. I think they are orthogonal, and adding that behavior sounds like we make another edge case behavior.

The natural answer is "clear the controller" if the final load does not make a scope.

I think this makes sense complying with the "navigation matching always wins" rule.

Nowhere else in the system do we allow a client to go from controlled to uncontrolled.

Firing a controllerchange event to navigator.serviceWorker when clearing the controller would work?


Another edge case is when an iframe navigates to a non-initial about:blank document. I think this is a normal cross-origin navigation case where the non-initial about:blank document lands on a newly created client with a SW set to null. But when it navigates from an initial about:blank to a non-initial about:blank, I'm not sure whether we should keep the normal behavior or special case it to use the inherited controller.

@annevk
Copy link
Member

annevk commented Nov 23, 2017

Typically non-initial about:blank would not be cross-origin.

@jungkees
Copy link
Collaborator

I thought the initial about:blank document has the same-origin to the parent (as inherited), and the non-initial about:blank document created during the navigate has an opaque origin?

@annevk
Copy link
Member

annevk commented Nov 23, 2017

@jungkees
Copy link
Collaborator

  • Chrome, Firefox: the parent's origin
  • Edge: undefined (IE 11: error)
    Can anyone test and add Safari?

So, we have a compat issue for this too. @annevk, what's your expectation about the desired spec behavior? It currently says an opaque origin if I read it correctly.

@annevk
Copy link
Member

annevk commented Nov 23, 2017

Per the HTML Standard it takes the origin of the document responsible for navigating it. So this matches my expectations.

@wanderview
Copy link
Member Author

Right now when conrollerchange fires the .controller property is always set and never null. I'm worried sites would break if this invariant changes.

I think avoiding about:blank replacement would be less breaking because its unlikely many modern sites using service workers are actually relying on this feature.

Can we at least agree that:

  1. The initial about:blank should be controlled by the parent's service worker. And:
  2. The final document load should not be controlled by any service worker if its URL does not match a scope?

How we get from (1) to (2) is up for discussion. For now, though, I'm testing for these. I'm need to do something to make this happen. I'm leaning towards doing the new global for now, but willing to change to a different system if we can decide on a clean way to do it.

Note, if we can spec a way to cleanly clear the controller we could also enable the case of immediate unregister() if the site opts into it.

@jungkees
Copy link
Collaborator

Per the HTML Standard it takes the origin of the document responsible for navigating it.

I think you mean https://html.spec.whatwg.org/#origin:document-12? That considered, the non-initial about:blank document has the same origin as the initial about:blank. And thus the initial client is getting reused. Sounds good. (not a special case.)

Can we at least agree that:

  1. The initial about:blank should be controlled by the parent's service worker. And:
  2. The final document load should not be controlled by any service worker if its URL does not match a scope?

Agreed.

Note, if we can spec a way to cleanly clear the controller we could also enable the case of immediate unregister() if the site opts into it.

Interesting idea. This option can be discussed in #1204 as well.

@wanderview
Copy link
Member Author

Until we settle on something I'm going to be doing the minimal amount of code changes to meet the sanity conditions of:

  1. The initial about:blank should be controlled by the parent's service worker. And:
  2. The final document load should not be controlled by any service worker if its URL does not match a scope?

In this lame first version in gecko the controller will appear to get reset, the page will share its global, and the Clients API will see a different client ID for the final loaded page. This approach is not what I recommend as a spec solution, but I didn't want to write significant code until we agreed on a direction. Its a corner case that is only really observable under some very precise conditions, so I don't think this will be a problem in the short term.

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

4 participants