Support for <a rel="noreferrer"> functionality

RESOLVED FIXED in mozilla33

Status

()

Core
DOM: Core & HTML
--
enhancement
RESOLVED FIXED
8 years ago
2 years ago

People

(Reporter: teoli, Unassigned)

Tracking

(Blocks: 1 bug, {dev-doc-complete, html5})

Trunk
mozilla33
dev-doc-complete, html5
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [parity-webkit], URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20091119 Minefield/3.7a1pre
Build Identifier: 

When rel="noreferrer" is added to a link, the referrer shouldn't be sent. See URL to have a testcase.

Reproducible: Always

Steps to Reproduce:
1. Link to testcase
2. Click on first link the referrer should have been sent.
3. Click on second link the referrer shouldn't have been sent.
Actual Results:  
The click on the second link lead to the sending of the referrer

Expected Results:  
The click on the second link doesn't lead to the sending of the referrer

Webkit support it in its latest nightly.

Updated

8 years ago
Component: HTML: Parser → Networking
QA Contact: parser → networking
Component: Networking → DOM: Core & HTML
QA Contact: networking → general
"[HTML5] " in summary means the HTML5 parser. "html5" as a keyword means general HTML5 spec compliance.
Keywords: html5
Summary: [HTML5] Support for <a rel="noreferrer"> functionality → Support for <a rel="noreferrer"> functionality

Comment 2

8 years ago
Need to verify that HTML5 makes sense here, but this sounds like a good idea.
And simple to implement. One could probably clear referrer here http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#10909 if <a> has noreferrer.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 3

8 years ago
Or use INTERNAL_LOAD_FLAGS_DONT_SEND_REFERRER with InternalLoad

Comment 4

8 years ago
Ah, it changes also 'opener' handling.

Comment 5

8 years ago
See http://webkit.org/blog/907/webkit-nightlies-support-html5-noreferrer-link-relation/ for information on Webkits implementation.
Created attachment 427735 [details] [diff] [review]
patch

Updated

8 years ago
Attachment #427735 - Flags: review?(Olli.Pettay)

Updated

8 years ago
Assignee: nobody → m_kato
Status: NEW → ASSIGNED

Comment 7

8 years ago
Comment on attachment 427735 [details] [diff] [review]
patch

>@@ -11245,6 +11258,19 @@ nsDocShell::OnLinkClickSync(nsIContent *
>     }
>   }
> 
>+  PRUint32 flags = INTERNAL_LOAD_FLAGS_NONE;
>+  if (IsElementAnchor(aContent)) {
>+    nsCOMPtr<nsIAtom> relAtom = do_GetAtom("rel");
>+    if (relAtom) {
>+      nsAutoString value;
>+      aContent->GetAttr(kNameSpaceID_None, relAtom, value);
>+      if (value.LowerCaseEqualsLiteral("noreferrer")) {
Why not use nsIContent's  AttrValueIs(...)

Comment 8

8 years ago
Comment on attachment 427735 [details] [diff] [review]
patch

>@@ -7728,6 +7737,10 @@ nsDocShell::InternalLoad(nsIURI * aURI,
>                     isNewWindow = PR_TRUE;
>                     aFlags |= INTERNAL_LOAD_FLAGS_FIRST_LOAD;
>                 }
>+
>+                // set opener object to null for noreferrer
>+                if (aFlags & INTERNAL_LOAD_FLAGS_NO_OPENER)
>+                    piNewWin->SetOpenerWindow(nsnull, PR_FALSE);
>             }
> 
>             nsCOMPtr<nsIWebNavigation> webNav = do_GetInterface(newWin);
>@@ -11245,6 +11258,19 @@ nsDocShell::OnLinkClickSync(nsIContent *
>     }
>   }
> 
>+  PRUint32 flags = INTERNAL_LOAD_FLAGS_NONE;
>+  if (IsElementAnchor(aContent)) {
>+    nsCOMPtr<nsIAtom> relAtom = do_GetAtom("rel");
>+    if (relAtom) {
>+      nsAutoString value;
>+      aContent->GetAttr(kNameSpaceID_None, relAtom, value);
>+      if (value.LowerCaseEqualsLiteral("noreferrer")) {
>+        flags |= INTERNAL_LOAD_FLAGS_DONT_SEND_REFERRER |
>+                 INTERNAL_LOAD_FLAGS_NO_OPENER;
>+      }
>+    }
>+  }
>+


If I read the code correctly, this doesn't do the right thing.
opener should be set null only if a new browsing context is created.
What if noreferrer is used to open page in an existing window?
(<a href="foobar.html target='foo'>first click</a>
<a href="foobar2.html target='foo' ref="noreferrer">second click</a>)

And do we want LowerCaseEqualsLiteral even in XHTML?
Attachment #427735 - Flags: review?(Olli.Pettay) → review-

Comment 9

8 years ago
Is there an updated patch coming?

Comment 10

7 years ago
Bump.
Kato-san, are you planning to update the patch?
Whiteboard: [parity-webkit][wanted2.1?]
Version: unspecified → Trunk
after 2.0, I will make new patch.
Ping?

Comment 14

6 years ago
http://www.whatwg.org/specs/web-apps/current-work/multipage/links.html#link-type-noreferrer

There should not only be no "Referer:" in the HTTP request but also there should be no value assigned to the JS variable "document.referrer".

Updated

6 years ago
Blocks: 666746
Makoto, are you still planning to fix this?
Whiteboard: [parity-webkit][wanted2.1?] → [parity-webkit]
no time this year.   It is OK if someone take this bug.
Assignee: m_kato → nobody
Status: ASSIGNED → NEW

Updated

5 years ago
Duplicate of this bug: 797142

Comment 18

5 years ago
Bump and subscribe
Blocks: 61660
See Also: → bug 704320

Comment 19

4 years ago
outdated link
See Also: → bug 999754
Created attachment 8425463 [details] [diff] [review]
don't send a Referer header for rel="noreferrer" links

This patch is simply an update of Kato-san's work.  I don't think it addresses
smaug's review comment 8, even though it moves the check for
INTERNAL_LOAD_FLAGS_NO_OPENER.  But I'm going to ask smaug just to make sure.
Attachment #427735 - Attachment is obsolete: true
Attachment #8425463 - Flags: feedback?(bugs)
Comment on attachment 8425463 [details] [diff] [review]
don't send a Referer header for rel="noreferrer" links

>+static bool
>+IsElementAnchor(nsIContent* content)
aContent

>+{
>+  // Make sure we are dealing with either an <A> or <AREA> element in the HTML
>+  // or XHTML namespace.
>+  if (!content->IsHTML())
>+    return false;
{}

>+  nsIAtom *nameAtom = content->Tag();
* goes with the type

>+  if (IsElementAnchor(aContent)) {
>+    if (nsCOMPtr<nsIAtom> relAtom = do_GetAtom("rel")) {
We have nsGkAtoms::rel


>+      if (aContent->AttrValueIs(kNameSpaceID_None, relAtom,
>+                                NS_LITERAL_STRING("noreferrer"),
>+                                aContent->IsHTML() && aContent->IsInHTMLDocument() ?
If element is anchor, it should be IsHTML(), right? Perhaps just assert somewhere here that it is IsHTML()
Attachment #8425463 - Flags: feedback?(bugs) → feedback+
Created attachment 8437117 [details] [diff] [review]
don't send a Referer header for rel="noreferrer" links

I'm going to take your f+ as an indication that it did address comment 8.  This
version of the patch addresses the review comments.
Attachment #8425463 - Attachment is obsolete: true
Attachment #8437117 - Flags: review?(bugs)
Comment on attachment 8437117 [details] [diff] [review]
don't send a Referer header for rel="noreferrer" links

>+    if (aContent->AttrValueIs(kNameSpaceID_None, nsGkAtoms::rel,
>+                              NS_LITERAL_STRING("noreferrer"),
>+                              aContent->IsInHTMLDocument() ?
>+                              eIgnoreCase : eCaseMatters)) {
some indentation for eIgnoreCase...
Attachment #8437117 - Flags: review?(bugs) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4d2928e0713
Flags: in-testsuite+
Like many other tests in this directory apparently, this test is perma-fail on B2G desktop. Skipped.
https://hg.mozilla.org/integration/mozilla-inbound/rev/85a48222098a

https://tbpl.mozilla.org/php/getParsedLog.php?id=41532723&tree=Mozilla-Inbound
https://hg.mozilla.org/mozilla-central/rev/e4d2928e0713
https://hg.mozilla.org/mozilla-central/rev/85a48222098a
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
(Reporter)

Updated

3 years ago
Keywords: dev-doc-needed

Updated

3 years ago
Depends on: 1031264
(Reporter)

Comment 27

3 years ago
I like to document bugs that I opened half a decade ago. :-)

Doc updated:
https://developer.mozilla.org/en-US/docs/Web/HTML/Link_types
https://developer.mozilla.org/en-US/Firefox/Releases/33
Keywords: dev-doc-needed → dev-doc-complete

Comment 28

3 years ago
It might be advisable to document that Firefox is only half-way there, because this does not yet work when opening a link via context menu (Bug #1031264). People who read the above announcement and find out for themselves that right-click does not work might feel that Mozilla Devs are kidding them.
(Reporter)

Comment 29

3 years ago
will69. Thx; added a note.
Comment hidden (spam)
Depends on: 1094449

Comment 31

3 years ago
I think this change changed behavior how tabs are ordered.

rel=noreferer -> middleclick on link -> opens as last tab
rel="" -> middleclick on link -> opens as tab next to the current tab

I think it breaks the expected behavior of browser.tabs.insertRelatedAfterCurrent = true
Could you please file a new bug about that issue and make the new bug to block this one, thanks.

Updated

3 years ago
Depends on: 1118502
No longer depends on: 1118502
You need to log in before you can comment on or make changes to this bug.