Please see: https://bugzilla.mozilla.org/show_bug.cgi?id=1302449#c1

Comment on attachment 8795410 [details] [diff] [review] Added deprecation warning for referrer-directive Hey Samriddhy, you would have to generate a *.patch which I can review. Please have a look at 'Creating a Patch' [1] which provides some guidance on how to do that. You would have to do something like: > hg export . > my-change.patch and upload that patch to bugzilla which I can then review. [1] https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch

Hey, sorry, I know I messed up things, was just working on the same, will fix that soon.

Comment on attachment 8795726 [details] [diff] [review] Change in file nsCSPParse.cpp and csp.properties Review of attachment 8795726 [details] [diff] [review]: ----------------------------------------------------------------- Looks good otherwise, please update my nit and flag me again so we can get this landed. thanks! ::: dom/locales/en-US/chrome/security/csp.properties @@ +83,1 @@ > Please make that match the other deprecation warning we already have: > deprecatedDirective = Directive ‘%1$S’ has been deprecated. Please use directive ‘%2$S’ instead. So Please update to: deprecatedReferrerDirective = Referrer Directive ‘%1$S’ has been deprecated. Please use the Referrer-Policy header instead.

Comment on attachment 8795832 [details] [diff] [review] Updated warning message Review of attachment 8795832 [details] [diff] [review]: ----------------------------------------------------------------- It seems you have re-export the patch because csp.properties appears twice in the patch. Please fix and re-flag me for review! ::: dom/locales/en-US/chrome/security/csp.properties @@ +77,4 @@ > # LOCALIZATION NOTE (ignoringReportOnlyDirective): > # %1$S is the directive that is ignored in report-only mode. > ignoringReportOnlyDirective = Ignoring sandbox directive when delivered in a report-only policy ‘%1$S’ > +# LOCALIZATION NOTE (deprecatedReferrerDirective): Looks good! @@ +78,5 @@ > # %1$S is the directive that is ignored in report-only mode. > ignoringReportOnlyDirective = Ignoring sandbox directive when delivered in a report-only policy ‘%1$S’ > +# LOCALIZATION NOTE (deprecatedReferrerDirective): > +# %1$S is the value of the deprecated Referrer Directive. > +deprecatedReferrerDirective = Support for Referrer Directive ‘%1$S’ is going to be removed soon. Why is that part still here?

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #7) > Comment on attachment 8795832 [details] [diff] [review] > Updated warning message > > Review of attachment 8795832 [details] [diff] [review]: > ----------------------------------------------------------------- > > It seems you have re-export the patch because csp.properties appears twice > in the patch. Please fix and re-flag me for review! > > ::: dom/locales/en-US/chrome/security/csp.properties > @@ +77,4 @@ > > # LOCALIZATION NOTE (ignoringReportOnlyDirective): > > # %1$S is the directive that is ignored in report-only mode. > > ignoringReportOnlyDirective = Ignoring sandbox directive when delivered in a report-only policy ‘%1$S’ > > +# LOCALIZATION NOTE (deprecatedReferrerDirective): > > Looks good! > > @@ +78,5 @@ > > # %1$S is the directive that is ignored in report-only mode. > > ignoringReportOnlyDirective = Ignoring sandbox directive when delivered in a report-only policy ‘%1$S’ > > +# LOCALIZATION NOTE (deprecatedReferrerDirective): > > +# %1$S is the value of the deprecated Referrer Directive. > > +deprecatedReferrerDirective = Support for Referrer Directive ‘%1$S’ is going to be removed soon. > > Why is that part still here? I combined the last two changesets to get the patch, that was the reason for redundant lines. Updated with new patch. Thanks!

Comment on attachment 8796150 [details] [diff] [review] Updated reviews Review of attachment 8796150 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, one thing I just realized, you haven't set a correct commit message. Something link: > Bug 1303682: Add deprecation warning before removing 'referrer' directive from CSP. r=ckerschb Please also mark older patches as 'obsolete' whenever you update a new version of a patch. Thanks!

Comment on attachment 8796172 [details] [diff] [review] Updated commit message Review of attachment 8796172 [details] [diff] [review]: ----------------------------------------------------------------- Awesome! Whenever you are ready you can now set 'checkin-needed' as a Keyword and someone from the sheriffs will check in the code for you. Congrats :-)

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #12) > Comment on attachment 8796172 [details] [diff] [review] > Updated commit message > > Review of attachment 8796172 [details] [diff] [review]: > ----------------------------------------------------------------- > > Awesome! Whenever you are ready you can now set 'checkin-needed' as a > Keyword and someone from the sheriffs will check in the code for you. > Congrats :-) Added the keyword 'checkin-needed'. Thanks!

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/64465dd73b97 Add deprecation warning before removing 'referrer' directive from CSP. r=ckerschb

Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2017/csp-referrer-directive-has-been-deprecated/