CSP lacks l10n support

RESOLVED FIXED in mozilla16

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mgoodwin, Assigned: mgoodwin)

Tracking

(Blocks: 1 bug)

12 Branch
mozilla16
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

5 years ago
Error console messages for CSP Violations cannot currently be localized.
We should wait until CSP 1.0 is standardized by W3C before we freeze any strings for localization -- that way we won't have to re-translate anything if the spec causes the strings to change.
(Assignee)

Comment 2

5 years ago
(In reply to Sid Stamm [:geekboy] from comment #1)
> We should wait until CSP 1.0 is standardized by W3C before we freeze any
> strings for localization -- that way we won't have to re-translate anything
> if the spec causes the strings to change.

This doesn't stop us working on localized string support though, does it?
(Assignee)

Comment 3

5 years ago
Created attachment 635353 [details] [diff] [review]
l10n support for current CSP errors and warnings

Is this the right approach? Any comments welcome.
(Assignee)

Comment 4

5 years ago
Created attachment 635474 [details] [diff] [review]
l10n support for current CSP errors and warnings (with placeholder l10n text removed)

Fixed a potential issue with formatStringFromName (according to https://developer.mozilla.org/en/nsIStringBundle, the number of strings must be the same in the call and the properties file). Also removed placeholder text from the properties file (now strings for en-US are identical, down to spelling errors, to the previous hardcoded entries).
(Assignee)

Updated

5 years ago
Attachment #635353 - Attachment is obsolete: true
(Assignee)

Comment 5

5 years ago
Created attachment 635682 [details] [diff] [review]
add l10n support to existing CSP errors and warnings

CSPLocalizer borrows from WebConsoleUtils.l10n in browser/devtools/webconsole/WebConsoleUtils.jsm (it's a useful way of seeing a failure is due to an issue with getting the localized string). Is it ok to do this?
Attachment #635474 - Attachment is obsolete: true
(Assignee)

Comment 6

5 years ago
Created attachment 637521 [details] [diff] [review]
add l10n support to existing CSP errors and warnings

This leaves CSP functionality completely unchanged; it simply takes the strings from CSPUtils.jsm and contentSecurityPolicy.js and moves them to a properties file.

Latest version updated to work with the webconsole CSP changes.
Attachment #635682 - Attachment is obsolete: true
Attachment #637521 - Flags: review?(bzbarsky)
Comment on attachment 637521 [details] [diff] [review]
add l10n support to existing CSP errors and warnings

Is this changing just strings that appear in our error console, or also reports sent to servers?
(Assignee)

Comment 8

5 years ago
(In reply to Boris Zbarsky (:bz) from comment #7)
> Comment on attachment 637521 [details] [diff] [review]
> add l10n support to existing CSP errors and warnings
> 
> Is this changing just strings that appear in our error console, or also
> reports sent to servers?

These are just the strings in calls to CSPWarning and CSPError, so this is only for error console (and web console).
Comment on attachment 637521 [details] [diff] [review]
add l10n support to existing CSP errors and warnings

r=me
Attachment #637521 - Flags: review?(bzbarsky) → review+

Comment 10

5 years ago
Try run for 22a2ff7b02c3 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=22a2ff7b02c3
Results (out of 227 total builds):
    exception: 1
    success: 193
    warnings: 31
    failure: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mgoodwin@mozilla.com-22a2ff7b02c3
(Assignee)

Comment 11

5 years ago
Created attachment 637850 [details] [diff] [review]
add l10n support to existing CSP errors and warnings (fixed)

I noticed a problem with contentSecurityPolicy.js:558 - the reference to this.innerWindowID was a) in the wrong place (should have been a param to CSPWarning, not getFormatStr) and b) wouldn't work anyway ('this' is the CSPReportRedirectSink not ContentSecurityPolicy).

I'll be working at getting non-violation related CSP errors and warnings into the WebConsole as my next piece of work on bug 712859 so I've just removed it for for the time being.
Attachment #637521 - Attachment is obsolete: true
Attachment #637850 - Flags: review?(bzbarsky)
Comment on attachment 637850 [details] [diff] [review]
add l10n support to existing CSP errors and warnings (fixed)

r=me
Attachment #637850 - Flags: review?(bzbarsky) → review+
Comment on attachment 637850 [details] [diff] [review]
add l10n support to existing CSP errors and warnings (fixed)

On behalf of Mark, https://tbpl.mozilla.org/?tree=Try&rev=4f2a0156a99b looks fairly green to me.
Attachment #637850 - Flags: checkin?(sstamm)
Pushed to mozilla-inbound.
https://hg.mozilla.org/integration/mozilla-inbound/rev/63212e0a927b
Target Milestone: --- → mozilla16
Attachment #637850 - Flags: checkin?(sstamm) → checkin+
https://hg.mozilla.org/mozilla-central/rev/63212e0a927b
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Depends on: 770176
FWIW, we shouldn't extract strings without reviewing them for proper English. Having follow up bugs like 771076 makes life harder than necessary.

I personally poke Matej Novak in case of doubt on English strings and use of language, did so now in the follow up bug.
QA Contact: general

Updated

5 years ago
QA Contact: general
(Assignee)

Comment 17

5 years ago
(In reply to Axel Hecht [:Pike] from comment #16)
> FWIW, we shouldn't extract strings without reviewing them for proper
> English. Having follow up bugs like 771076 makes life harder than necessary.

Duly noted.
Blocks: 493857
You need to log in before you can comment on or make changes to this bug.