New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 618532 link

Starred by 3 users
Status: Assigned
Owner:
█ mkwst@chromium.org
Buried. Ping if important.
Cc:
ajha@chromium.org
est...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

noopener does not appear to be sending referrer info

Project Member Reported by n...@fb.com, Jun 9 2016 
On Wednesday we attempted adding rel="noopener" to our links rendered in Chrome. Our understanding was that this would not affect referrers, and would simply prevent the new window from having window.opener set. We got reports, however, that referrers weren't being sent in Chrome - and were able to reproduce this behaviour.
One point of note, is that in Chrome we couple this with using meta referrer. This is usually set to "origin-when-cross-origin", but is switched to "origin" when an external link is clicked. We wondered whether perhaps there was a bug that appeared when meta referrer was used in our way alongside noopener.

Comment 1 by kinuko@chromium.org, Jun 9 2016

Owner: mkwst@chromium.org
Mike-- probably you can take a look?

Comment 2 by mkwst@chromium.org, Jun 9 2016

Cc: est...@chromium.org
Status: Assigned (was: Untriaged)
I'll take a quick look today; CCing Emily for the potential overlap with referrer policy.

n8s@: Does this happen every time you use `noopener`, or only when `<meta>` is present?

Comment 3 by n...@fb.com, Jun 9 2016 

So actually it seems like I can repro this on any link that uses noopener regardless of the referrer policy, https://jsfiddle.net/ttau6zhf/ for example.

Comment 4 by mkwst@chromium.org, Jun 10 2016 

Yup. Our behavior here is incorrect; I've uploaded https://codereview.chromium.org/2058693002 for review.

Thanks for the report!
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 12 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4c82bb5386376f64c2a8eb9258ea397cba04b7d1

commit 4c82bb5386376f64c2a8eb9258ea397cba04b7d1
Author: mkwst <mkwst@chromium.org>
Date: Sun Jun 12 16:46:19 2016

Do not suppress referrers for '<a ... rel="noopener">'.

The current implementation of 'noopener' correctly split the referrer
and opener states in Blink, but missed a spot higher up the stack where
the two were still conflated. This patch fixes that issue, adds tests
to ensure we don't regress, and refactors the Blink-side code a bit so
that we're no longer passing the data around twice.

BUG=618532
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2058693002
Cr-Commit-Position: refs/heads/master@{#399391}

[modify] https://crrev.com/4c82bb5386376f64c2a8eb9258ea397cba04b7d1/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] https://crrev.com/4c82bb5386376f64c2a8eb9258ea397cba04b7d1/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/4c82bb5386376f64c2a8eb9258ea397cba04b7d1/third_party/WebKit/Source/core/loader/EmptyClients.h
[modify] https://crrev.com/4c82bb5386376f64c2a8eb9258ea397cba04b7d1/third_party/WebKit/Source/core/loader/FrameLoader.cpp
[modify] https://crrev.com/4c82bb5386376f64c2a8eb9258ea397cba04b7d1/third_party/WebKit/Source/core/page/ChromeClient.h
[modify] https://crrev.com/4c82bb5386376f64c2a8eb9258ea397cba04b7d1/third_party/WebKit/Source/core/page/CreateWindow.cpp
[modify] https://crrev.com/4c82bb5386376f64c2a8eb9258ea397cba04b7d1/third_party/WebKit/Source/core/page/CreateWindow.h
[modify] https://crrev.com/4c82bb5386376f64c2a8eb9258ea397cba04b7d1/third_party/WebKit/Source/web/ChromeClientImpl.cpp
[modify] https://crrev.com/4c82bb5386376f64c2a8eb9258ea397cba04b7d1/third_party/WebKit/Source/web/ChromeClientImpl.h

Comment 6 by mkwst@chromium.org, Jun 14 2016

Status: Fixed (was: Assigned)

Comment 7 by mkwst@chromium.org, Jun 14 2016

Labels: Merge-Request-52
Hello, friendly release managers! Would you mind letting me merge this small fix back to beta?

Comment 8 by tin...@google.com, Jun 14 2016

Labels: -Merge-Request-52 Merge-Approved-52 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M52 (branch: 2743)
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 14 2016

Labels: -merge-approved-52 merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f

commit fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f
Author: Mike West <mkwst@google.com>
Date: Tue Jun 14 07:28:56 2016

Do not suppress referrers for '<a ... rel="noopener">'.

The current implementation of 'noopener' correctly split the referrer
and opener states in Blink, but missed a spot higher up the stack where
the two were still conflated. This patch fixes that issue, adds tests
to ensure we don't regress, and refactors the Blink-side code a bit so
that we're no longer passing the data around twice.

BUG=618532
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2058693002
Cr-Commit-Position: refs/heads/master@{#399391}
(cherry picked from commit 4c82bb5386376f64c2a8eb9258ea397cba04b7d1)

Review URL: https://codereview.chromium.org/2067753002 .

Cr-Commit-Position: refs/branch-heads/2743@{#348}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] https://crrev.com/fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f/third_party/WebKit/Source/core/loader/EmptyClients.h
[modify] https://crrev.com/fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f/third_party/WebKit/Source/core/loader/FrameLoader.cpp
[modify] https://crrev.com/fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f/third_party/WebKit/Source/core/page/ChromeClient.h
[modify] https://crrev.com/fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f/third_party/WebKit/Source/core/page/CreateWindow.cpp
[modify] https://crrev.com/fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f/third_party/WebKit/Source/core/page/CreateWindow.h
[modify] https://crrev.com/fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f/third_party/WebKit/Source/web/ChromeClientImpl.cpp
[modify] https://crrev.com/fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f/third_party/WebKit/Source/web/ChromeClientImpl.h
Project Member

Comment 10 by bugdroid1@chromium.org, Jun 14 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f7939c2fff08f8f0d2700eea46ef94c57a985166

commit f7939c2fff08f8f0d2700eea46ef94c57a985166
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Tue Jun 14 20:13:09 2016

Revert "Do not suppress referrers for '<a ... rel="noopener">'." on M52 branch

The current implementation of 'noopener' correctly split the referrer
and opener states in Blink, but missed a spot higher up the stack where
the two were still conflated. This patch fixes that issue, adds tests
to ensure we don't regress, and refactors the Blink-side code a bit so
that we're no longer passing the data around twice.

BUG=618532,619856
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review URL: https://codereview.chromium.org/2063283002 .

Cr-Commit-Position: refs/branch-heads/2743@{#354}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/f7939c2fff08f8f0d2700eea46ef94c57a985166/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] https://crrev.com/f7939c2fff08f8f0d2700eea46ef94c57a985166/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/f7939c2fff08f8f0d2700eea46ef94c57a985166/third_party/WebKit/Source/core/loader/EmptyClients.h
[modify] https://crrev.com/f7939c2fff08f8f0d2700eea46ef94c57a985166/third_party/WebKit/Source/core/loader/FrameLoader.cpp
[modify] https://crrev.com/f7939c2fff08f8f0d2700eea46ef94c57a985166/third_party/WebKit/Source/core/page/ChromeClient.h
[modify] https://crrev.com/f7939c2fff08f8f0d2700eea46ef94c57a985166/third_party/WebKit/Source/core/page/CreateWindow.cpp
[modify] https://crrev.com/f7939c2fff08f8f0d2700eea46ef94c57a985166/third_party/WebKit/Source/core/page/CreateWindow.h
[modify] https://crrev.com/f7939c2fff08f8f0d2700eea46ef94c57a985166/third_party/WebKit/Source/web/ChromeClientImpl.cpp
[modify] https://crrev.com/f7939c2fff08f8f0d2700eea46ef94c57a985166/third_party/WebKit/Source/web/ChromeClientImpl.h
Project Member

Comment 11 by bugdroid1@chromium.org, Jun 15 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4c82bb5386376f64c2a8eb9258ea397cba04b7d1

commit 4c82bb5386376f64c2a8eb9258ea397cba04b7d1
Author: mkwst <mkwst@chromium.org>
Date: Sun Jun 12 16:46:19 2016

Do not suppress referrers for '<a ... rel="noopener">'.

The current implementation of 'noopener' correctly split the referrer
and opener states in Blink, but missed a spot higher up the stack where
the two were still conflated. This patch fixes that issue, adds tests
to ensure we don't regress, and refactors the Blink-side code a bit so
that we're no longer passing the data around twice.

BUG=618532
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2058693002
Cr-Commit-Position: refs/heads/master@{#399391}

[modify] https://crrev.com/4c82bb5386376f64c2a8eb9258ea397cba04b7d1/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] https://crrev.com/4c82bb5386376f64c2a8eb9258ea397cba04b7d1/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/4c82bb5386376f64c2a8eb9258ea397cba04b7d1/third_party/WebKit/Source/core/loader/EmptyClients.h
[modify] https://crrev.com/4c82bb5386376f64c2a8eb9258ea397cba04b7d1/third_party/WebKit/Source/core/loader/FrameLoader.cpp
[modify] https://crrev.com/4c82bb5386376f64c2a8eb9258ea397cba04b7d1/third_party/WebKit/Source/core/page/ChromeClient.h
[modify] https://crrev.com/4c82bb5386376f64c2a8eb9258ea397cba04b7d1/third_party/WebKit/Source/core/page/CreateWindow.cpp
[modify] https://crrev.com/4c82bb5386376f64c2a8eb9258ea397cba04b7d1/third_party/WebKit/Source/core/page/CreateWindow.h
[modify] https://crrev.com/4c82bb5386376f64c2a8eb9258ea397cba04b7d1/third_party/WebKit/Source/web/ChromeClientImpl.cpp
[modify] https://crrev.com/4c82bb5386376f64c2a8eb9258ea397cba04b7d1/third_party/WebKit/Source/web/ChromeClientImpl.h
Project Member

Comment 12 by bugdroid1@chromium.org, Jun 15 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f

commit fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f
Author: Mike West <mkwst@google.com>
Date: Tue Jun 14 07:28:56 2016

Do not suppress referrers for '<a ... rel="noopener">'.

The current implementation of 'noopener' correctly split the referrer
and opener states in Blink, but missed a spot higher up the stack where
the two were still conflated. This patch fixes that issue, adds tests
to ensure we don't regress, and refactors the Blink-side code a bit so
that we're no longer passing the data around twice.

BUG=618532
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2058693002
Cr-Commit-Position: refs/heads/master@{#399391}
(cherry picked from commit 4c82bb5386376f64c2a8eb9258ea397cba04b7d1)

Review URL: https://codereview.chromium.org/2067753002 .

Cr-Commit-Position: refs/branch-heads/2743@{#348}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] https://crrev.com/fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f/third_party/WebKit/Source/core/loader/EmptyClients.h
[modify] https://crrev.com/fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f/third_party/WebKit/Source/core/loader/FrameLoader.cpp
[modify] https://crrev.com/fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f/third_party/WebKit/Source/core/page/ChromeClient.h
[modify] https://crrev.com/fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f/third_party/WebKit/Source/core/page/CreateWindow.cpp
[modify] https://crrev.com/fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f/third_party/WebKit/Source/core/page/CreateWindow.h
[modify] https://crrev.com/fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f/third_party/WebKit/Source/web/ChromeClientImpl.cpp
[modify] https://crrev.com/fe741d1a0c260e05e914ef8fd7ebc17956f4ef7f/third_party/WebKit/Source/web/ChromeClientImpl.h
Project Member

Comment 13 by bugdroid1@chromium.org, Jun 15 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f7939c2fff08f8f0d2700eea46ef94c57a985166

commit f7939c2fff08f8f0d2700eea46ef94c57a985166
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Tue Jun 14 20:13:09 2016

Revert "Do not suppress referrers for '<a ... rel="noopener">'." on M52 branch

The current implementation of 'noopener' correctly split the referrer
and opener states in Blink, but missed a spot higher up the stack where
the two were still conflated. This patch fixes that issue, adds tests
to ensure we don't regress, and refactors the Blink-side code a bit so
that we're no longer passing the data around twice.

BUG=618532,619856
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review URL: https://codereview.chromium.org/2063283002 .

Cr-Commit-Position: refs/branch-heads/2743@{#354}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/f7939c2fff08f8f0d2700eea46ef94c57a985166/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] https://crrev.com/f7939c2fff08f8f0d2700eea46ef94c57a985166/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/f7939c2fff08f8f0d2700eea46ef94c57a985166/third_party/WebKit/Source/core/loader/EmptyClients.h
[modify] https://crrev.com/f7939c2fff08f8f0d2700eea46ef94c57a985166/third_party/WebKit/Source/core/loader/FrameLoader.cpp
[modify] https://crrev.com/f7939c2fff08f8f0d2700eea46ef94c57a985166/third_party/WebKit/Source/core/page/ChromeClient.h
[modify] https://crrev.com/f7939c2fff08f8f0d2700eea46ef94c57a985166/third_party/WebKit/Source/core/page/CreateWindow.cpp
[modify] https://crrev.com/f7939c2fff08f8f0d2700eea46ef94c57a985166/third_party/WebKit/Source/core/page/CreateWindow.h
[modify] https://crrev.com/f7939c2fff08f8f0d2700eea46ef94c57a985166/third_party/WebKit/Source/web/ChromeClientImpl.cpp
[modify] https://crrev.com/f7939c2fff08f8f0d2700eea46ef94c57a985166/third_party/WebKit/Source/web/ChromeClientImpl.h

Comment 14 by ajha@chromium.org, Jun 15 2016

Cc: ajha@chromium.org
Status: Assigned (was: Fixed)
Looks like Fix has been reverted from M-52 due to Build Compile failure Issue 619856 and issue still persists on the latest M-52(52.0.2743.41 /2743@{#361}) as tested on Windows-7. Screenshot is attached.

mkwst@: Could you please get this merged to M-52 again.
618532.png
163 KB View Download

Comment 15 by n...@fb.com, Aug 25 2016 

What was the status of this bug? We just tried things again in 52 and it looks like the fix never got merged in? Was it merged into 53?

Comment 16 by rbyers@chromium.org, Nov 18 2016

Components: Blink>SecurityFeature

Comment 17 by est...@chromium.org, Nov 10 2017

Labels: Hotlist-EnamelAndFriendsFixIt

Comment 18 by est...@chromium.org, Feb 18 2018

Labels: -Hotlist-EnamelAndFriendsFixIt
►

Sign in to add a comment