Implement referrerpolicy attribute for anchor elements

Implement referrerpolicy attribute for anchor elements

The 'referrerpolicy' attribute controls the referrer for the outgoing
request when an anchor is clicked.

BUG=490608
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=199997

[estark, 2015-07-27 19:43:56]

estark@chromium.org changed reviewers:
+ jochen@chromium.org

[estark, 2015-07-27 19:43:56]

jochen, could you please take a look and let me know if this is on the right
track?

It should probably have more test cases, and obviously only implements the
attribute for anchors and not any of the other elements, but I wanted to get
some feedback on whether this looks like the right idea or not.

Thanks!

[jochen (gone - plz use gerrit), 2015-07-28 14:35:37]

https://codereview.chromium.org/1253413003/diff/1/Source/core/html/HTMLAnchor...
File Source/core/html/HTMLAnchorElement.idl (right):

https://codereview.chromium.org/1253413003/diff/1/Source/core/html/HTMLAnchor...
Source/core/html/HTMLAnchorElement.idl:32: [Reflect] attribute DOMString
referrerpolicy;
this should be a guarded by a runtime flag
(https://www.chromium.org/blink/runtime-enabled-features)

you should also send an intent to implement to blink-dev@

last but not least, you should use ReflectOnly to restrict the attribute to
valid values:
https://www.chromium.org/blink/webidl/blink-idl-extended-attributes#TOC-Refle...

[estark, 2015-07-29 20:54:41]

Thanks, Jochen.

https://codereview.chromium.org/1253413003/diff/1/Source/core/html/HTMLAnchor...
File Source/core/html/HTMLAnchorElement.idl (right):

https://codereview.chromium.org/1253413003/diff/1/Source/core/html/HTMLAnchor...
Source/core/html/HTMLAnchorElement.idl:32: [Reflect] attribute DOMString
referrerpolicy;
On 2015/07/28 14:35:36, jochen wrote:
> this should be a guarded by a runtime flag
> (https://www.chromium.org/blink/runtime-enabled-features)

Done.

> 
> you should also send an intent to implement to blink-dev@

Done.

> 
> last but not least, you should use ReflectOnly to restrict the attribute to
> valid values:
>
https://www.chromium.org/blink/webidl/blink-idl-extended-attributes#TOC-Refle...

Done.

[jochen (gone - plz use gerrit), 2015-07-30 14:34:09]

could you also add a test that reflects the valid values for referrerpolicy?

and what should document.createElement('a').referrerpolicy be?

can you add a test that the referrerpolicy attribute does not apply if there's a
click event listener on the anchor that then creates an XHR or opens a window?

[estark, 2015-07-31 23:35:37]

On 2015/07/30 14:34:09, jochen wrote:
> could you also add a test that reflects the valid values for referrerpolicy?

Done.

> 
> and what should document.createElement('a').referrerpolicy be?

I think it should be empty string (i.e. null policy)... does that mean that the
IDL should contain "" as a value in the ReflectOnly list?

> 
> can you add a test that the referrerpolicy attribute does not apply if there's
a
> click event listener on the anchor that then creates an XHR or opens a window?

Done.

[jochen (gone - plz use gerrit), 2015-08-03 15:48:48]

On 2015/07/31 at 23:35:37, estark wrote:
> On 2015/07/30 14:34:09, jochen wrote:
> > could you also add a test that reflects the valid values for referrerpolicy?
> 
> Done.
> 
> > 
> > and what should document.createElement('a').referrerpolicy be?
> 
> I think it should be empty string (i.e. null policy)... does that mean that
the IDL should contain "" as a value in the ReflectOnly list?

I don't know. Does it work without it?

> 
> > 
> > can you add a test that the referrerpolicy attribute does not apply if
there's a
> > click event listener on the anchor that then creates an XHR or opens a
window?
> 
> Done.

[estark, 2015-08-04 01:14:46]

On 2015/08/03 15:48:48, jochen wrote:
> On 2015/07/31 at 23:35:37, estark wrote:
> > On 2015/07/30 14:34:09, jochen wrote:
> > > could you also add a test that reflects the valid values for
referrerpolicy?
> > 
> > Done.
> > 
> > > 
> > > and what should document.createElement('a').referrerpolicy be?
> > 
> > I think it should be empty string (i.e. null policy)... does that mean that
> the IDL should contain "" as a value in the ReflectOnly list?
> 
> I don't know. Does it work without it?

It does work without it, but based on the discussion at
https://github.com/w3c/webappsec/pull/435/files#r35883345, it sounds like
specifying "" as the default value would make sense, in which case I think it
would be correct to set ReflectMissing and ReflectInvalid. So I did that.

> 
> > 
> > > 
> > > can you add a test that the referrerpolicy attribute does not apply if
> there's a
> > > click event listener on the anchor that then creates an XHR or opens a
> window?
> > 
> > Done.

[jochen (gone - plz use gerrit), 2015-08-04 13:47:53]

can you also add a test that verifies that value of
document.createElement('a').referrerpolicy?

with that, lgtm

[estark, 2015-08-04 16:38:56]

On 2015/08/04 13:47:53, jochen wrote:
> can you also add a test that verifies that value of
> document.createElement('a').referrerpolicy?
> 
> with that, lgtm

Done, thanks!

[estark, 2015-08-04 16:38:59]

The CQ bit was checked by estark@chromium.org

[estark, 2015-08-04 16:38:59]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1253413003/#ps140001
(title: "test referrerpolicy attribute of dynamically created anchor")

[commit-bot: I haz the power, 2015-08-04 16:39:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1253413003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1253413003/140001

[commit-bot: I haz the power, 2015-08-04 17:49:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-04 17:49:55]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/72540)

[estark, 2015-08-04 18:11:52]

The CQ bit was checked by estark@chromium.org

[estark, 2015-08-04 18:11:52]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1253413003/#ps160001
(title: "update webexposed layout tests")

[commit-bot: I haz the power, 2015-08-04 18:12:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1253413003/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1253413003/160001

[commit-bot: I haz the power, 2015-08-04 20:01:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-04 20:01:05]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/64995)

[estark, 2015-08-04 22:18:08]

The CQ bit was checked by estark@chromium.org

[commit-bot: I haz the power, 2015-08-04 22:18:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1253413003/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1253413003/160001

[commit-bot: I haz the power, 2015-08-04 22:22:13]

Committed patchset #9 (id:160001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=199997