Bug 190251

Summary: Passing noopener=NOOPENER to window.open() should cause the new window to not have an opener
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: DOMAssignee: Chris Dumez <cdumez>
Status: RESOLVED WONTFIX    
Severity: Normal CC: achristensen, cdumez, commit-queue, darin, ggaren, rniwa, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Chris Dumez 2018-10-03 09:41:40 PDT
Passing noopener=NOOPENER to window.open() should cause the new window to not have an opener, similarly to noopener=1:
- https://html.spec.whatwg.org/#window-open-steps (step 5)

It does not matter what the value is, if there is a key named "noopener", then the new window should not have an opener.
Comment 1 Chris Dumez 2018-10-03 09:43:54 PDT
Created attachment 351528 [details]
Patch
Comment 2 WebKit Commit Bot 2018-10-03 11:24:03 PDT
Comment on attachment 351528 [details]
Patch

Clearing flags on attachment: 351528

Committed r236802: <https://trac.webkit.org/changeset/236802>
Comment 3 WebKit Commit Bot 2018-10-03 11:24:05 PDT
All reviewed patches have been landed.  Closing bug.
Comment 4 Radar WebKit Bug Importer 2018-10-03 11:25:29 PDT
<rdar://problem/44980240>
Comment 5 Darin Adler 2018-10-07 19:57:03 PDT
Comment on attachment 351528 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=351528&action=review

> Source/WebCore/page/WindowFeatures.cpp:161
>      else if (equalLettersIgnoringASCIICase(key, "noopener"))
> -        features.noopener = numericValue;
> +        features.noopener = true;

So even values like "0" and "no" mean yes? Do we have test coverage for those kinds of values?

Also seems that other boolean keys like menubar, toolbar, location, status, fullscreen, and scrollbars might need similar treatment. Would be good to make sure we have tests and make sure those are also right.
Comment 6 Chris Dumez 2018-10-08 08:08:35 PDT
Comment on attachment 351528 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=351528&action=review

>> Source/WebCore/page/WindowFeatures.cpp:161
>> +        features.noopener = true;
> 
> So even values like "0" and "no" mean yes? Do we have test coverage for those kinds of values?
> 
> Also seems that other boolean keys like menubar, toolbar, location, status, fullscreen, and scrollbars might need similar treatment. Would be good to make sure we have tests and make sure those are also right.

Yes, even values like "0" and "no" enable no opener as per the special case in the spec:
https://html.spec.whatwg.org/#window-open-steps (step 5): Let noopener be true if tokenizedFeatures contains an entry with the key "noopener".

imported/w3c/web-platform-tests/html/browsers/the-window-object/apis-for-creating-and-navigating-browsing-contexts-by-name/open-features-tokenization-noopener.html contains extensive testing for noopener, including "0" / "false" values.

The other keys you mention (like menubar) are currently missing from the spec:
- https://github.com/whatwg/html/issues/2464

However, other browsers seem to treat them like regular booleans so I do not think we should align them with noopener. AFAICT, noopener is the exception but I am not quite sure why (will comment on the spec).
Comment 7 Chris Dumez 2018-10-10 09:30:13 PDT
Reverted r236802 for reason:

Working on getting the HTML spec updated instead (https://github.com/whatwg/html/pull/4079)

Committed r237002: <https://trac.webkit.org/changeset/237002>