[webkit-reviews] review granted: [Bug 137635] Referrer Policy: Update <meta name="referrer"> values to match the spec : [Attachment 239679] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Mike West
<mkwst at chromium.org>'s request for review:
Bug 137635: Referrer Policy: Update <meta name="referrer"> values to match the
spec
https://bugs.webkit.org/show_bug.cgi?id=137635

Attachment 239679: Patch
https://bugs.webkit.org/attachment.cgi?id=239679&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=239679&action=review


Thank you!

> Alexey, do you think adopting to the standard requires a PSA to webkit-dev?

I don't know if it's formally required, however I think that it would be
helpful to give people a heads up.

> Source/WebCore/ChangeLog:10
> +	   defines different keywords than we originally implemented. We should

> +	   support them.

...because this is the only version of the spec that is actively maintained?

Anyone can publish thousands of specs on github, which means that the existence
of a spec alone has no impact on what browsers do. Knowing your history with
referrer policy, I can figure out more, yet it's helpful to have a more
complete rationale in ChangeLog.

> Source/WebCore/dom/Document.cpp:3048
> +    if (equalIgnoringCase(policy, "no-referrer") ||
equalIgnoringCase(policy, "never"))
> +	   setReferrerPolicy(ReferrerPolicyNever);
> +    else if (equalIgnoringCase(policy, "unsafe-url") ||
equalIgnoringCase(policy, "always"))
> +	   setReferrerPolicy(ReferrerPolicyAlways);

I think that a comment explaining that we support multiple spec versions would
help here. Otherwise, it will be very difficult to figure out for someone
looking at this code.