> 6.1.4 Security
> http://www.whatwg.org/specs/web-apps/current-work/multipage/browsers.
> html#security-nav
> It would be useful if this section had a note saying: 
> 
> "If browsing context A's active document's active sandboxing flag set has
> the sandboxed navigation browsing context flag set, then these conditions
> are further restricted by the sanboxing rules."
> 
> This could include a link to the "6.4 Sandboxing" section and/or possibly
> the "6.6.1 Navigating across documents" section.

I've added a note that's a bit vaguer. The problem is the above isn't entirely accurate; e.g. window.close() isn't affected by the navigation sandboxing but _is_ affected by this algorithm.


> 6.1.6 Browsing context names
> http://www.whatwg.org/specs/web-apps/current-work/multipage/browsers.
> html#browsing-context-names
> While the non-normative table includes the effects of sandboxing on the
> selection of the browsing context, the normative text that follows doesn't,
> it only mentions the sandboxed auxiliary navigation browsing context flag
> and the copying of flags to a new browsing context.
> It would be clearer if rules 2, 3 and 4 were updated to include the
> sandboxing.
> Again possibly with links to the relevant sections for more detail.

I've added a note that points out that the sandboxing is handled by other algorithms. I don't want to move around where sandboxing is done because this is a very fragile part of the spec and I don't want to break it.


> It would also be useful if this update made it clear about the desired
> behaviour if the chosen browsing context is inaccessible due to sandboxing.
> I believe the desired behaviour is to:
>  * choose the browsing context using the name or keyword as normal
>  * if the chosen browsing context is not allowed due to sandboxing, then
> choose no browsing context
> However, in this circumstance (if allow-popups is specified), do not open a
> new browsing context as if none had been found.

Isn't this what the spec says already?


> One final request.
> Could another a few rows be added to the non-normative table:
>  * name that exists and is not top and not an ancestor or descendant of
> current | specified browsing context | specified browsing context | none |
> none | none | none
>  * name that exists and is an auxiliary created by current | specified
> auxiliary | specified auxiliary | specified auxiliary | specified auxiliary
> | specified auxiliary | specified auxiliary
>  * name that exists and is an auxiliary not created by current | specified
> auxiliary | specified auxiliary | none | none | none | none

I used different wording, but I tried to add these. I hope they're right!

Checked in as WHATWG revision r7828.
Check-in comment: Try to clarify browsing context rules
http://html5.org/tools/web-apps-tracker?from=7827&to=7828

(In reply to comment #1)

Thanks for taking the time to look at this.

> > 6.1.4 Security
> > http://www.whatwg.org/specs/web-apps/current-work/multipage/browsers.
> > html#security-nav
> > It would be useful if this section had a note saying: 
> > 
> > "If browsing context A's active document's active sandboxing flag set has
> > the sandboxed navigation browsing context flag set, then these conditions
> > are further restricted by the sanboxing rules."
> > 
> > This could include a link to the "6.4 Sandboxing" section and/or possibly
> > the "6.6.1 Navigating across documents" section.
> 
> I've added a note that's a bit vaguer. The problem is the above isn't
> entirely accurate; e.g. window.close() isn't affected by the navigation
> sandboxing but _is_ affected by this algorithm.
> 

Thanks.
I'm surprised that window.close() isn't affected by sandboxing.
I would have thought that if a browsing context is sandboxed without allow-top-navigation, stopping it from navigating the top level, then it shouldn't be able to close the whole window either.

Although, off the top of my head, if it isn't caught by the general sandboxing code, I can't think of any specific code that deals with it at the moment.

> 
> > 6.1.6 Browsing context names
> > http://www.whatwg.org/specs/web-apps/current-work/multipage/browsers.
> > html#browsing-context-names
> > While the non-normative table includes the effects of sandboxing on the
> > selection of the browsing context, the normative text that follows doesn't,
> > it only mentions the sandboxed auxiliary navigation browsing context flag
> > and the copying of flags to a new browsing context.
> > It would be clearer if rules 2, 3 and 4 were updated to include the
> > sandboxing.
> > Again possibly with links to the relevant sections for more detail.
> 
> I've added a note that points out that the sandboxing is handled by other
> algorithms. I don't want to move around where sandboxing is done because
> this is a very fragile part of the spec and I don't want to break it.
> 

Thanks.  I agree that it would be difficult to work in the sandboxing cleanly (mainly why I didn't try and make suggestions for this bit myself :) ) and it would duplicate things from elsewhere.
However I wonder if an extra note after Step 4 is needed, see below ...

> 
> > It would also be useful if this update made it clear about the desired
> > behaviour if the chosen browsing context is inaccessible due to sandboxing.
> > I believe the desired behaviour is to:
> >  * choose the browsing context using the name or keyword as normal
> >  * if the chosen browsing context is not allowed due to sandboxing, then
> > choose no browsing context
> > However, in this circumstance (if allow-popups is specified), do not open a
> > new browsing context as if none had been found.
> 
> Isn't this what the spec says already?

I'm not sure it covers the second point.
Assume we're sandboxed with allow-popups.
If a name is used that is for a browsing context that we are normally allowed to navigate (Step 4), but can't because of sandboxing.
At the moment, people might think you go to Step 5 and because allow-popups is specified then we open a new window.
This isn't what the non-normative table suggests and I think it's correct.

> 
> 
> > One final request.
> > Could another a few rows be added to the non-normative table:
> >  * name that exists and is not top and not an ancestor or descendant of
> > current | specified browsing context | specified browsing context | none |
> > none | none | none
> >  * name that exists and is an auxiliary created by current | specified
> > auxiliary | specified auxiliary | specified auxiliary | specified auxiliary
> > | specified auxiliary | specified auxiliary
> >  * name that exists and is an auxiliary not created by current | specified
> > auxiliary | specified auxiliary | none | none | none | none
> 
> I used different wording, but I tried to add these. I hope they're right!

I think the second from last row, in the new table, catches more cases than it should.
For example you are "allowed to navigate" your child even when sandboxed, but you are not its "one permitted sandboxed navigator". 
But, I'm guessing by "other name" you're implying a name that hasn't been caught by the previous rows?
Come to think of it you must be, because the same would apply for the row four from bottom, if you weren't.


Wow ... I noticed in the diff that you've added me to the Acknowledgements.
I wasn't expecting that ... thanks.

Regarding window.close(), that method is blocked by all kinds of other security checks already; I'm not sure it really matters if a sandboxed iframe can call it unexpectedly. But we could change that. If you think window.close() should be affected by sandboxing, can you file a separate bug for that? Thanks!

Re step 4 and the table, I need to look at that more closely.

(In reply to comment #4)
> Regarding window.close(), that method is blocked by all kinds of other
> security checks already; I'm not sure it really matters if a sandboxed
> iframe can call it unexpectedly. But we could change that. If you think
> window.close() should be affected by sandboxing, can you file a separate bug
> for that? Thanks!

I was just thinking aloud really, but when I have time I'll try and investigate the existing checks around window.close() and see if they already cover off any potential sandboxing issues.
If I think there are any gaps, I'll raise a separate bug with examples.

I've had a look at the restrictions around window.close().

In the specification at http://www.w3.org/html/wg/drafts/html/master/browsers.html#dom-window-close, it states:

"The close() method on Window objects should, if the corresponding browsing context A is script-closable and the browsing context of the script that invokes the method is allowed to navigate the browsing context A, close the browsing context A."

Now the "allowed to navigate" rules at http://www.w3.org/html/wg/drafts/html/master/browsers.html#allowed-to-navigate, don't explicitly include the sandboxing rules, but with the new note that you have added, you could take this to mean that sandboxing should stop it.

I did a quick test to see what happens in a sandbox with allow-scripts, but not allow-top-navigation.

Firefox, IE10, Chrome and Opera all seem to block the direct navigation of top with window.open(..., "_top").  (I was slightly surprised by this as I didn't think Opera implemented sandboxing.)

Only Chrome stops the window.top.close().  It gives the same error as for the navigation, which complains that you are trying to navigate top without allow-top-navigation.

Chrome's behaviour seems correct to me and you could argue that with the latest changes this is what the spec implies.
So, if you think that this is the desired behaviour, I'll raise a bug against Firefox to get this fixed.

If you think the spec needs changing to make this behaviour more explicit then I'll raise that as a separate bug here.

Sorry about the relatively long post.
I hope it all makes sense.

Cheers,
Bob

Checked in as WHATWG revision r7939.
Check-in comment: Clarify further
http://html5.org/tools/web-apps-tracker?from=7938&to=7939

Please do file a separate bug for the window.close() thing. I don't know what the right conclusion for it is but it deserves being examined.

(In reply to comment #3)
> > 
> > > It would also be useful if this update made it clear about the desired
> > > behaviour if the chosen browsing context is inaccessible due to 
> > > sandboxing.
> > > I believe the desired behaviour is to:
> > >  * choose the browsing context using the name or keyword as normal
> > >  * if the chosen browsing context is not allowed due to sandboxing, then
> > > choose no browsing context
> > > However, in this circumstance (if allow-popups is specified), do not open 
> > > a new browsing context as if none had been found.
> > 
> > Isn't this what the spec says already?
> 
> I'm not sure it covers the second point.
> Assume we're sandboxed with allow-popups.
> If a name is used that is for a browsing context that we are normally
> allowed to navigate (Step 4), but can't because of sandboxing.

You are "allowed to navigate" it. The sandboxing doesn't affect whether you're "allowed to navigate" it. It is handled much later.

I've tried to make this clearer by updating the note I think we added earlier (see patch in previous comment).


> I think the second from last row, in the new table, catches more cases than
> it should.
> For example you are "allowed to navigate" your child even when sandboxed,
> but you are not its "one permitted sandboxed navigator".
> But, I'm guessing by "other name" you're implying a name that hasn't been
> caught by the previous rows?
> Come to think of it you must be, because the same would apply for the row
> four from bottom, if you weren't.

Yeah, the list is supposed to be "first matches" (otherwise describing the cases becomes impractical). I've tried different labels for the last three, let me know if they're any clearer. Patch in next comment.

Please don't hesitate to reopen this bug if you see any problems with the fixes in this bug. (And please do open a new one for window.close() if you think it needs looking at.)

Checked in as WHATWG revision r7940.
Check-in comment: Try different labels
http://html5.org/tools/web-apps-tracker?from=7939&to=7940

(In reply to comment #8)
> Please do file a separate bug for the window.close() thing. I don't know
> what the right conclusion for it is but it deserves being examined.

I've raised bug 22319.

> You are "allowed to navigate" it. The sandboxing doesn't affect whether
> you're "allowed to navigate" it. It is handled much later.
> 
> I've tried to make this clearer by updating the note I think we added
> earlier (see patch in previous comment).

Thanks Ian, that makes it crystal clear.
Also, I've realised that I had missed the fact that you had modified the comment under the table in the last change.
This explains that the rules below don't include all the sandboxing restrictions that are in the table.


On a slightly different note, I wonder if giving the label "allowed to navigate" to the set of browsing contexts described in section "6.1.4 Security", causes unnecessary confusion.  Given the fact that it may turn out that you can't actually navigate them.
Maybe something like "allowed to access" or some other term would be better ... just a thought.

 
> Yeah, the list is supposed to be "first matches" (otherwise describing the
> cases becomes impractical). I've tried different labels for the last three,
> let me know if they're any clearer. Patch in next comment.

Yes, I think that makes it clear, thanks again.

Do I change the Status to VERIFIED or CLOSED, or does someone else handle that?

(In reply to comment #10)
> On a slightly different note, I wonder if giving the label "allowed to
> navigate" to the set of browsing contexts described in section "6.1.4
> Security", causes unnecessary confusion.

It certainly makes confusion. Whether it's necessary is just a matter of whether we can find something less confusing. I'm open to other names; file a bug if you find an accurate yet not confusing one.


> Maybe something like "allowed to access" 

That's confusing too, though. In fact possibly more so, since really you can access any browsing context you can get a reference to.


> Do I change the Status to VERIFIED or CLOSED, or does someone else handle
> that?

RESOLVED, VERIFIED, and CLOSED are synonyms for the purposes of WHATWG bugs.

(In reply to comment #11)

> It certainly makes confusion. Whether it's necessary is just a matter of
> whether we can find something less confusing. I'm open to other names; file
> a bug if you find an accurate yet not confusing one.

That's a fair point, if a have a flash of inspiration I'll raise another bug.

I changed the terms around since this bug, by the way. Please open a new bug if the new terminology isn't better.