Bug 190251 - Passing noopener=NOOPENER to window.open() should cause the new window to not have an opener
Summary: Passing noopener=NOOPENER to window.open() should cause the new window to not...
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-10-03 09:41 PDT by Chris Dumez
Modified: 2018-10-10 09:32 PDT (History)
8 users (show)

See Also:


Attachments
Patch (4.79 KB, patch)
2018-10-03 09:43 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>