Bug 190475 - Restrict browsing context lookup by name to frames that are related to one another
Summary: Restrict browsing context lookup by name to frames that are related to one an...
Status: RESOLVED FIXED
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: 190481
  Show dependency treegraph
 
Reported: 2018-10-11 12:01 PDT by Chris Dumez
Modified: 2018-10-15 08:35 PDT (History)
11 users (show)

See Also:


Attachments
Patch (19.73 KB, patch)
2018-10-11 12:14 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (23.63 KB, patch)
2018-10-11 13:16 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (30.66 KB, patch)
2018-10-12 09:43 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (32.61 KB, patch)
2018-10-12 10:21 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (33.35 KB, patch)
2018-10-12 16:38 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-11 12:01:14 PDT
When a new browsing context is created via <a target="x" rel="noopener"> or window.open("", "x", "noopener"), we should create a new top-level browsing context instead of one auxiliary one.
As a result, the requester's browsing context and new new browsing context would not be target each other by frame name (e.g. via anchor target or window.open target).

This aligns us with Firefox and Chrome. The change was discussed at:
- https://github.com/whatwg/html/issues/1826

This allows process swapping in more cases.
Comment 1 Chris Dumez 2018-10-11 12:14:49 PDT
Created attachment 352063 [details]
Patch
Comment 2 Chris Dumez 2018-10-11 13:16:37 PDT
Created attachment 352069 [details]
Patch
Comment 3 Daniel Bates 2018-10-11 20:04:02 PDT
Comment on attachment 352069 [details]
Patch

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

> Source/WebCore/page/Page.cpp:479
> +bool Page::isAuxiliaryBrowsingContext() const

I like the old name better. It is clear what openedByDOM means on first glance: it was opened by the page not by the user. The new name is very generic and is ambiguous. I mean you could describe both a child window opened programmatically by the page and a child window opened by a user that control-clicks a hyperlink in a pge and select Open in New Window as an auxiliaries window, but this method is only talking about the former. The old name did not led itself to this ambiguity.
Comment 4 Chris Dumez 2018-10-11 20:32:27 PDT
(In reply to Daniel Bates from comment #3)
> Comment on attachment 352069 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=352069&action=review
> 
> > Source/WebCore/page/Page.cpp:479
> > +bool Page::isAuxiliaryBrowsingContext() const
> 
> I like the old name better. It is clear what openedByDOM means on first
> glance: it was opened by the page not by the user. The new name is very
> generic and is ambiguous. I mean you could describe both a child window
> opened programmatically by the page and a child window opened by a user that
> control-clicks a hyperlink in a pge and select Open in New Window as an
> auxiliaries window, but this method is only talking about the former. The
> old name did not led itself to this ambiguity.

ok, I will keep openedByDOM as is. However, I will still need to add isAuxiliaryBrowsingContext because this is a different concept (defined in the HTML spec) and it is the concept I need in this patch for targeting.
Comment 5 Chris Dumez 2018-10-11 20:37:58 PDT
(In reply to Chris Dumez from comment #4)
> (In reply to Daniel Bates from comment #3)
> > Comment on attachment 352069 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=352069&action=review
> > 
> > > Source/WebCore/page/Page.cpp:479
> > > +bool Page::isAuxiliaryBrowsingContext() const
> > 
> > I like the old name better. It is clear what openedByDOM means on first
> > glance: it was opened by the page not by the user. The new name is very
> > generic and is ambiguous. I mean you could describe both a child window
> > opened programmatically by the page and a child window opened by a user that
> > control-clicks a hyperlink in a pge and select Open in New Window as an
> > auxiliaries window, but this method is only talking about the former. The
> > old name did not led itself to this ambiguity.
> 
> ok, I will keep openedByDOM as is. However, I will still need to add
> isAuxiliaryBrowsingContext because this is a different concept (defined in
> the HTML spec) and it is the concept I need in this patch for targeting.

I will think about this more and come up with a better proposal.
Comment 6 Chris Dumez 2018-10-12 09:43:09 PDT
Created attachment 352175 [details]
Patch
Comment 7 Chris Dumez 2018-10-12 10:21:50 PDT
Created attachment 352179 [details]
Patch
Comment 8 Chris Dumez 2018-10-12 16:38:51 PDT
Created attachment 352238 [details]
Patch
Comment 9 WebKit Commit Bot 2018-10-12 17:34:04 PDT
Comment on attachment 352238 [details]
Patch

Rejecting attachment 352238 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 352238, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 5000 characters of output:
M	LayoutTests/fast/dom/Window/a-rel-noopener-expected.txt
	M	LayoutTests/fast/dom/Window/area-rel-noopener-expected.txt
	M	LayoutTests/fast/dom/Window/resources/rel-noopener.js
	M	LayoutTests/http/tests/navigation/no-referrer-reset-expected.txt
	M	LayoutTests/http/tests/navigation/no-referrer-subframe-expected.txt
	M	LayoutTests/http/tests/navigation/no-referrer-target-blank-expected.txt
	M	LayoutTests/http/tests/navigation/resources/no-referrer-helper.php
	M	LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/html/browsers/windows/noreferrer-window-name-expected.txt
	M	LayoutTests/platform/wk2/imported/w3c/web-platform-tests/html/browsers/windows/noreferrer-window-name-expected.txt
	M	Source/WebCore/ChangeLog
	M	Source/WebCore/loader/FrameLoader.cpp
	M	Source/WebCore/page/FrameTree.cpp
	M	Source/WebCore/page/FrameTree.h
	M	Source/WebCore/rendering/HitTestResult.cpp
	M	Source/WebKit/ChangeLog
	M	Source/WebKit/WebProcess/Plugins/PluginView.cpp
	M	Source/WebKitLegacy/mac/ChangeLog
	M	Source/WebKitLegacy/mac/WebView/WebFrame.mm
	M	Source/WebKitLegacy/win/WebFrame.cpp

ERROR from SVN:
A repository hook failed: Commit blocked by pre-commit hook (exit code 1) with output:
Commits are currently disabled while we update infrastructure.
W: 6fa332b5c98cebd27fdbad50d89fcc1880c2ed70 and refs/remotes/origin/master differ, using rebase:
:040000 040000 7398bec2c5a03c3cdd66212347b41c9b06b65704 92eda367eb67787361b8f4da2a96959390505aa7 M	LayoutTests
:040000 040000 4a7575fc0576fe76a4bd0a476790490d9ad09727 db349c7d53434e63104b36e19511df8a3253ad7d M	Source
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.


Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	A	LayoutTests/http/tests/dom/new-window-can-target-opener-expected.txt
	A	LayoutTests/http/tests/dom/new-window-can-target-opener.html
	A	LayoutTests/http/tests/dom/noopener-window-cannot-target-opener-expected.txt
	A	LayoutTests/http/tests/dom/noopener-window-cannot-target-opener.html
	A	LayoutTests/http/tests/dom/noopener-window-not-targetable-expected.txt
	A	LayoutTests/http/tests/dom/noopener-window-not-targetable.html
	A	LayoutTests/http/tests/dom/noopener-window-not-targetable2-expected.txt
	A	LayoutTests/http/tests/dom/noopener-window-not-targetable2.html
	A	LayoutTests/http/tests/dom/noreferrer-window-not-targetable-expected.txt
	A	LayoutTests/http/tests/dom/noreferrer-window-not-targetable.html
	A	LayoutTests/http/tests/dom/opened-window-not-targetable-after-disowning-opener-expected.txt
	A	LayoutTests/http/tests/dom/opened-window-not-targetable-after-disowning-opener.html
	A	LayoutTests/http/tests/dom/resources/new-window-can-target-opener-win.html
	A	LayoutTests/http/tests/dom/resources/noopener-window-cannot-target-opener-win.html
	M	LayoutTests/ChangeLog
	M	LayoutTests/fast/dom/Window/a-rel-noopener-expected.txt
	M	LayoutTests/fast/dom/Window/area-rel-noopener-expected.txt
	M	LayoutTests/fast/dom/Window/resources/rel-noopener.js
	M	LayoutTests/http/tests/navigation/no-referrer-reset-expected.txt
	M	LayoutTests/http/tests/navigation/no-referrer-subframe-expected.txt
	M	LayoutTests/http/tests/navigation/no-referrer-target-blank-expected.txt
	M	LayoutTests/http/tests/navigation/resources/no-referrer-helper.php
	M	LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/html/browsers/windows/noreferrer-window-name-expected.txt
	M	LayoutTests/platform/wk2/imported/w3c/web-platform-tests/html/browsers/windows/noreferrer-window-name-expected.txt
	M	Source/WebCore/ChangeLog
	M	Source/WebCore/loader/FrameLoader.cpp
	M	Source/WebCore/page/FrameTree.cpp
	M	Source/WebCore/page/FrameTree.h
	M	Source/WebCore/rendering/HitTestResult.cpp
	M	Source/WebKit/ChangeLog
	M	Source/WebKit/WebProcess/Plugins/PluginView.cpp
	M	Source/WebKitLegacy/mac/ChangeLog
	M	Source/WebKitLegacy/mac/WebView/WebFrame.mm
	M	Source/WebKitLegacy/win/WebFrame.cpp

ERROR from SVN:
A repository hook failed: Commit blocked by pre-commit hook (exit code 1) with output:
Commits are currently disabled while we update infrastructure.
W: 6fa332b5c98cebd27fdbad50d89fcc1880c2ed70 and refs/remotes/origin/master differ, using rebase:
:040000 040000 7398bec2c5a03c3cdd66212347b41c9b06b65704 92eda367eb67787361b8f4da2a96959390505aa7 M	LayoutTests
:040000 040000 4a7575fc0576fe76a4bd0a476790490d9ad09727 db349c7d53434e63104b36e19511df8a3253ad7d M	Source
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.


Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit
Updating OpenSource
Current branch master is up to date.

Full output: https://webkit-queues.webkit.org/results/9560045
Comment 10 WebKit Commit Bot 2018-10-15 07:56:10 PDT
Comment on attachment 352238 [details]
Patch

Rejecting attachment 352238 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 352238, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 5000 characters of output:
ed.txt
	M	LayoutTests/http/tests/navigation/resources/no-referrer-helper.php
	M	LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/html/browsers/windows/noreferrer-window-name-expected.txt
	M	LayoutTests/platform/wk2/imported/w3c/web-platform-tests/html/browsers/windows/noreferrer-window-name-expected.txt
	M	Source/WebCore/ChangeLog
	M	Source/WebCore/loader/FrameLoader.cpp
	M	Source/WebCore/page/FrameTree.cpp
	M	Source/WebCore/page/FrameTree.h
	M	Source/WebCore/rendering/HitTestResult.cpp
	M	Source/WebKit/ChangeLog
	M	Source/WebKit/WebProcess/Plugins/PluginView.cpp
	M	Source/WebKitLegacy/mac/ChangeLog
	M	Source/WebKitLegacy/mac/WebView/WebFrame.mm
	M	Source/WebKitLegacy/win/WebFrame.cpp

ERROR from SVN:
Merge conflict during commit: Conflict at '/trunk/LayoutTests/ChangeLog'
W: 505b2ab3d2d4d24b8734323771dfd4aa3d52d924 and refs/remotes/origin/master differ, using rebase:
:040000 040000 ff77979ff58f768c05db733d8f29b42359038beb d1a543eadbd9333c7f42d831c625908d4ad0d3e1 M	LayoutTests
:040000 040000 1d8cc1e683136c74215912d073b83f084c32c4af 56273f4a65fcc62a748c915bf4452ab155095092 M	Source
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.


Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	A	LayoutTests/http/tests/dom/new-window-can-target-opener-expected.txt
	A	LayoutTests/http/tests/dom/new-window-can-target-opener.html
	A	LayoutTests/http/tests/dom/noopener-window-cannot-target-opener-expected.txt
	A	LayoutTests/http/tests/dom/noopener-window-cannot-target-opener.html
	A	LayoutTests/http/tests/dom/noopener-window-not-targetable-expected.txt
	A	LayoutTests/http/tests/dom/noopener-window-not-targetable.html
	A	LayoutTests/http/tests/dom/noopener-window-not-targetable2-expected.txt
	A	LayoutTests/http/tests/dom/noopener-window-not-targetable2.html
	A	LayoutTests/http/tests/dom/noreferrer-window-not-targetable-expected.txt
	A	LayoutTests/http/tests/dom/noreferrer-window-not-targetable.html
	A	LayoutTests/http/tests/dom/opened-window-not-targetable-after-disowning-opener-expected.txt
	A	LayoutTests/http/tests/dom/opened-window-not-targetable-after-disowning-opener.html
	A	LayoutTests/http/tests/dom/resources/new-window-can-target-opener-win.html
	A	LayoutTests/http/tests/dom/resources/noopener-window-cannot-target-opener-win.html
	M	LayoutTests/ChangeLog
	M	LayoutTests/fast/dom/Window/a-rel-noopener-expected.txt
	M	LayoutTests/fast/dom/Window/area-rel-noopener-expected.txt
	M	LayoutTests/fast/dom/Window/resources/rel-noopener.js
	M	LayoutTests/http/tests/navigation/no-referrer-reset-expected.txt
	M	LayoutTests/http/tests/navigation/no-referrer-subframe-expected.txt
	M	LayoutTests/http/tests/navigation/no-referrer-target-blank-expected.txt
	M	LayoutTests/http/tests/navigation/resources/no-referrer-helper.php
	M	LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/html/browsers/windows/noreferrer-window-name-expected.txt
	M	LayoutTests/platform/wk2/imported/w3c/web-platform-tests/html/browsers/windows/noreferrer-window-name-expected.txt
	M	Source/WebCore/ChangeLog
	M	Source/WebCore/loader/FrameLoader.cpp
	M	Source/WebCore/page/FrameTree.cpp
	M	Source/WebCore/page/FrameTree.h
	M	Source/WebCore/rendering/HitTestResult.cpp
	M	Source/WebKit/ChangeLog
	M	Source/WebKit/WebProcess/Plugins/PluginView.cpp
	M	Source/WebKitLegacy/mac/ChangeLog
	M	Source/WebKitLegacy/mac/WebView/WebFrame.mm
	M	Source/WebKitLegacy/win/WebFrame.cpp

ERROR from SVN:
Merge conflict during commit: Conflict at '/trunk/LayoutTests/ChangeLog'
W: 505b2ab3d2d4d24b8734323771dfd4aa3d52d924 and refs/remotes/origin/master differ, using rebase:
:040000 040000 ff77979ff58f768c05db733d8f29b42359038beb d1a543eadbd9333c7f42d831c625908d4ad0d3e1 M	LayoutTests
:040000 040000 1d8cc1e683136c74215912d073b83f084c32c4af 56273f4a65fcc62a748c915bf4452ab155095092 M	Source
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.


Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit
Updating OpenSource
From https://git.webkit.org/git/WebKit
   380292d5256..0adfe55c838  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 237103 = 380292d525682cca4e526545dc94c84b20ba2440
r237104 = 6157765c1985fca826feeb674b1cc9d643a07717
r237105 = 0adfe55c838a3d9733642308a214e698ab8310a1
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: https://webkit-queues.webkit.org/results/9582276
Comment 11 WebKit Commit Bot 2018-10-15 08:34:49 PDT
Comment on attachment 352238 [details]
Patch

Clearing flags on attachment: 352238

Committed r237112: <https://trac.webkit.org/changeset/237112>
Comment 12 WebKit Commit Bot 2018-10-15 08:34:51 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Radar WebKit Bug Importer 2018-10-15 08:35:31 PDT
<rdar://problem/45271874>