530396 - Support for <a rel="noreferrer"> functionality

Status: RESOLVED FIXED

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

Description

[Jean-Yves Perrier [:teoli]]

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.

Comment 1

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

"[HTML5] " in summary means the HTML5 parser. "html5" as a keyword means general HTML5 spec compliance.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

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.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Or use INTERNAL_LOAD_FLAGS_DONT_SEND_REFERRER with InternalLoad

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Ah, it changes also 'opener' handling.

Comment 5

[d]

See http://webkit.org/blog/907/webkit-nightlies-support-html5-noreferrer-link-relation/ for information on Webkits implementation.

Comment 6

[Makoto Kato [:m_kato]]

Attachment: patch

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

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

[Olli Pettay [:smaug][[e-mail]]]

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?

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Is there an updated patch coming?

Comment 10

[d]

Bump.

Comment 11

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Kato-san, are you planning to update the patch?

Comment 12

[Makoto Kato [:m_kato]]

after 2.0, I will make new patch.

Comment 13

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Ping?

Comment 14

[Stefan]

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".

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Makoto, are you still planning to fix this?

Comment 16

[Makoto Kato [:m_kato]]

no time this year.   It is OK if someone take this bug.

Comment 18

[Ñull]

Bump and subscribe

Comment 19

[sjw]

outdated link

Comment 20

[Nathan Froyd [:froydnj]]

Attachment: 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.

Comment 21

[Olli Pettay [:smaug][[e-mail]]]

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()

Comment 22

[Nathan Froyd [:froydnj]]

Attachment: 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.

Comment 23

[Olli Pettay [:smaug][[e-mail]]]

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...

Comment 24

[Nathan Froyd [:froydnj]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e4d2928e0713

Comment 25

[Ryan VanderMeulen [:RyanVM]]

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

Comment 26

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e4d2928e0713
https://hg.mozilla.org/mozilla-central/rev/85a48222098a

Comment 27

[Jean-Yves Perrier [:teoli]]

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

Comment 28

[will69]

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.

Comment 29

[Jean-Yves Perrier [:teoli]]

will69. Thx; added a note.

Comment 31

[The 8472]

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

Comment 32

[Olli Pettay [:smaug][[e-mail]]]

Could you please file a new bug about that issue and make the new bug to block this one, thanks.