Jean-Yves Perrier [:teoli] Reporter
	Description • 15 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.

	Henri Sivonen (:hsivonen)
	Comment 1 • 15 years ago

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

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 2 • 15 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.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 3 • 15 years ago

Or use INTERNAL_LOAD_FLAGS_DONT_SEND_REFERRER with InternalLoad

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 4 • 15 years ago

Ah, it changes also 'opener' handling.

	d
	Comment 5 • 15 years ago

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

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 7 • 14 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(...)

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 8 • 14 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?

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 9 • 14 years ago

Is there an updated patch coming?

	d
	Comment 10 • 14 years ago

Bump.

	:Ms2ger (he/him; ⌚ UTC+1/+2)
	Comment 11 • 14 years ago

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

	Makoto Kato [:m_kato]
	Comment 12 • 14 years ago

after 2.0, I will make new patch.

	:Ms2ger (he/him; ⌚ UTC+1/+2)
	Comment 13 • 13 years ago

Ping?

	Stefan
	Comment 14 • 13 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".

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 15 • 13 years ago

Makoto, are you still planning to fix this?

	Makoto Kato [:m_kato]
	Comment 16 • 13 years ago

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

	Ñull
	Comment 18 • 11 years ago

Bump and subscribe

	sjw
	Comment 19 • 11 years ago

outdated link

	Nathan Froyd [:froydnj]
	Comment 20 • 10 years ago

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.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 21 • 10 years ago

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

	Nathan Froyd [:froydnj]
	Comment 22 • 10 years ago

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.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 23 • 10 years ago

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

	Nathan Froyd [:froydnj]
	Comment 24 • 10 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 25 • 10 years ago

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

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 26 • 10 years ago

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

	Jean-Yves Perrier [:teoli] Reporter
	Comment 27 • 10 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

	will69
	Comment 28 • 10 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.

	Jean-Yves Perrier [:teoli] Reporter
	Comment 29 • 10 years ago

will69. Thx; added a note.

	The 8472
	Comment 31 • 9 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

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 32 • 9 years ago

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