Closed Bug 1358469 Opened 3 years ago Closed 3 years ago

Anchor element with rel=noreferrer and target=framename does not open in the iframe, but in a new tab

Categories

(Core :: DOM: Core & HTML, defect)

52 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr52 --- fixed
firefox53 --- wontfix
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: doliere.some, Assigned: bzbarsky)

References

Details

(Keywords: regression)

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20170413192749

Steps to reproduce:

<!DOCTYPE html>
<html>
<body>

<a href="https://www.youtube-nocookie.com/embed/keSUj7KpV6w" rel="noreferrer" target="ifrtarget">How to use Bugzilla</a>
<iframe name="ifrtarget" width="560" height="315"></iframe>

</body>
</html>



Actual results:

In Google Chrome, Opera, Edge, Chromium browsers, the Youtube Video link loads inside the iframe (because of the link target pointing to the iframe name).

In Firefox however, the link opened as a popup in a new tab. If I remove the rel attribute, then the link loads inside the iframe, otherwise it does not.


Expected results:

I was expecting the link to load inside the iframe since the link target and the iframe name match. 

I have used the  rel="noreferrer" to prevent the Referrer header from leaking to youtube.
Attached file 1358469.html
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=28681d252003e3110105473754da2f4097cb83a6&tochange=806054dd12bdcbdee81dbd75f1583156cef9b649

Boris Zbarsky — Bug 1222516 part 4. Implement support for rel=noopener on links. r=mconley
Boris Zbarsky — Bug 1222516 part 3. Rejigger our rel="noreferrer" support to avoid setting an opener altogether instead of setting one and then nulling it out. r=mconley
Blocks: 1222516
Status: UNCONFIRMED → NEW
Has Regression Range: --- → yes
Has STR: --- → yes
Component: Untriaged → DOM: Core & HTML
Ever confirmed: true
Flags: needinfo?(bzbarsky)
Keywords: regression
Product: Firefox → Core
Version: 53 Branch → 52 Branch
Thank you for the bug report!

The behavior we implement is the one that was agreed on for noreferrer in the spec discussions, but other browsers haven't updated their implementations yet.  See https://github.com/whatwg/html/issues/1826 for details.

If you just want to prevent the referrer being sent and don't want the other effects that rel="noreferrer" has, you can do:

  <a href="https://www.youtube-nocookie.com/embed/keSUj7KpV6w" referrerpolicy="no-referrer" target="ifrtarget">

See https://html.spec.whatwg.org/multipage/semantics.html#links-created-by-a-and-area-elements%3Areferrer-policy-attribute and see https://w3c.github.io/webappsec-referrer-policy/#referrer-policy for the list of possible referrerpolicy values.
Status: NEW → RESOLVED
Closed: 3 years ago
Flags: needinfo?(bzbarsky)
Resolution: --- → INVALID
That said, if this causes sufficient compat issues in practice we may need to get the spec to say something else.  I guess we did just ship this a few weeks ago for the first time...

If we do go that route, it's not really clear what the behavior should be; right now Chrome will target the iframe in the case above, but not target a named window created by that same link.  Not sure what it will do for a named window created by a _different_ link with the same named target, but I expect it targets it.  So basically, the behavior is "whatever fell out of their noopener implementation", rather than something that makes sense.  The spec proposal was an attempt to at least make things consistent.
Thank you for the pointers.

I am wondering about compatibility. I guess rel="noreferrer" and target="framename" are supported by all browsers. 
which is not yet the case of referrerpolicy. 

Until then, what can ensure this compatibility ?
Based on resolution, updating tracking flags so that this doesn't show on the release health dashboard.
Yeah, the lack of support for referrerpolicy in other UAs is a problem.  This can be worked around with script that tests for that support, but that's not great, I agree...

I'm hoping the Chrome folks actually talk to me on the spec issue here.  If they don't get back to me within a day or two I'll give up on them and revert the noreferrer behavior to the old one for subframes and make up something for windows.
The Chrome folks are not talking to me.  Going to revert most of this behavior change, per my specific proposal in https://github.com/whatwg/html/issues/1826#issuecomment-299240850
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Assignee: nobody → bzbarsky
Status: REOPENED → ASSIGNED
Comment on attachment 8864590 [details] [diff] [review]
Revert our web-incompatible change to rel=noreferrer targeting behavior

Review of attachment 8864590 [details] [diff] [review]:
-----------------------------------------------------------------

::: docshell/base/nsDocShell.cpp
@@ +9853,5 @@
>    if (!aWindowTarget.IsEmpty()) {
>      // Locate the target DocShell.
>      nsCOMPtr<nsIDocShellTreeItem> targetItem;
> +    // Only _self, _parent, and _top are supported in noopener case.  But we
> +    // have to be careful to not apply that to the noreferrer case.

Please add this bug number to the comment. It really isn't clear why we have to make this distinction from the comment, and AFAIK this isn't currently specced behavior (we're doing this change for webcompat reasons and then hope to spec it later IIRC).
Attachment #8864590 - Flags: review?(michael) → review+
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9d46ca4e00c2
Revert our web-incompatible change to rel=noreferrer targeting behavior.  r=mystor
Comment on attachment 8864590 [details] [diff] [review]
Revert our web-incompatible change to rel=noreferrer targeting behavior

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Web compat regression.
User impact if declined: On some pages, link clicks that should load in iframes will instead load in an entirely new tab.  Apart from the obvious "wrong behavior" factor, this can also break pages in other ways.
Fix Landed on Version: 55
Risk to taking this patch (and alternatives if risky): Low risk; restores
   something closer to Firefox 51 behavior for noreferrer links.
String or UUID changes made by this patch: None.

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1222516
[User impact if declined]: See above.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: No, but I verified in local build.
[Needs manual test from QE? If yes, steps to reproduce]:  No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: See above.
[String changes made/needed]: None.
Attachment #8864590 - Flags: approval-mozilla-esr52?
Attachment #8864590 - Flags: approval-mozilla-beta?
Attachment #8864590 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/9d46ca4e00c2
Status: ASSIGNED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment on attachment 8864590 [details] [diff] [review]
Revert our web-incompatible change to rel=noreferrer targeting behavior

Fix a web compat regression and include test. Beta54+. Should be in 54 beta 7.
Attachment #8864590 - Flags: approval-mozilla-beta?
Attachment #8864590 - Flags: approval-mozilla-beta+
Attachment #8864590 - Flags: approval-mozilla-aurora?
Attachment #8864590 - Flags: approval-mozilla-aurora-
(In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #12)
> [Is this code covered by automated tests?]: Yes.
> [Has the fix been verified in Nightly?]: No, but I verified in local build.
> [Needs manual test from QE? If yes, steps to reproduce]:  No.

Setting qe-verify- based on Boris' assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
Comment on attachment 8864590 [details] [diff] [review]
Revert our web-incompatible change to rel=noreferrer targeting behavior

fix a webcompat issue, esr52.2+
Attachment #8864590 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
You need to log in before you can comment on or make changes to this bug.