878790 - Undo backout of 821877

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Garrett Robinson [:grobinson]]

Bug 873187 backed out Bug 821877 (and by extension Bug 863878) in Aurora because the bug to flip the CSP 1.0 pref was not landed in the expected timeframe. Shortly after the back out, however, CSP 1.0 was landed and uplifted to Aurora in Bug 763879. Now I want to "backout the backout" and restore the original patches to Aurora/Firefox23.

The patches were never backed out of mozilla-central and it is unaffected.

Comment 1

[Garrett Robinson [:grobinson]]

Attachment: Patch 1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 873187
User impact if declined: We would not warn web developers about using deprecated CSP headers
Testing completed (on m-c, etc.): Green try run on mozilla-aurora
Risk to taking this patch (and alternatives if risky): Minimal, would affect CSP only
String or IDL/UUID changes made by this patch: None

This patch restores the changes made in Bug 821877 and Bug 863878, which were backed out but now no longer need to be. These patches were r+'ed and landed before their backout.

I patched mozilla-aurora with this and pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=f8de346e8e90. It was green.

Comment 2

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/158f14d242d7

Comment 3

[Ian Neal]

Was this particular change intended? http://hg.mozilla.org/releases/mozilla-aurora/diff/158f14d242d7/dom/locales/en-US/chrome/security/security.properties

Comment 4

[Dwayne Bailey]

The "revert" highlighted in comment 3 causes translation regressions.

Comment 5

[Garrett Robinson [:grobinson]]

(In reply to Ian Neal from comment #3)
> Was this particular change intended?
> http://hg.mozilla.org/releases/mozilla-aurora/diff/158f14d242d7/dom/locales/
> en-US/chrome/security/security.properties

No, that appears to be an failure of Mercurial's merge algorithm. As you can see, it adds a redundant copy of those three strings. I apologize for not catching this when I prepared the patch. What can we do to resolve this? I can make a patch to remove the redundant addition to security.properties, if that would be helpful.

Comment 6

[Dwayne Bailey]

(In reply to Garrett Robinson [:grobinson] from comment #5)
> (In reply to Ian Neal from comment #3)
> > Was this particular change intended?
> > http://hg.mozilla.org/releases/mozilla-aurora/diff/158f14d242d7/dom/locales/
> > en-US/chrome/security/security.properties
> 
> No, that appears to be an failure of Mercurial's merge algorithm. As you can
> see, it adds a redundant copy of those three strings. I apologize for not
> catching this when I prepared the patch. What can we do to resolve this? I
> can make a patch to remove the redundant addition to security.properties, if
> that would be helpful.

That would be best, but ideally quickly if this is in nightly, in Aurora 23 as long as it doesn't cause any churn.  Most translators in Aurora 23 have honestly probably worked around this already.

Comment 7

[Garrett Robinson [:grobinson]]

The duplicate entries in the .properties files were resolved by the translators shortly after the last comment above.