770176 - Typo in failedToParseUnrecognizedSource in http://mxr.mozilla.org/mozilla-central/source/dom/locales/en-US/chrome/security/csp.properties

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Alexander L. Slovesnik]

There is typo in "failedToParseUnrecognizedSource = Failed to parse unrecoginzied source %1$S"

s/unrecoginzied/unrecognized

Comment 1

[Mark Goodwin [:mgoodwin]]

Oops! I copy and pasted the typo from the original code (see https://bugzilla.mozilla.org/attachment.cgi?id=637850&action=diff - line 609 of the original CSPUtils.jsm).

I'll fix this in my patch for bug 770099.

Comment 2

[Alexander L. Slovesnik]

(In reply to Mark Goodwin [:mgoodwin] from comment #1)
> Oops! I copy and pasted the typo from the original code (see
> https://bugzilla.mozilla.org/attachment.cgi?id=637850&action=diff - line 609
> of the original CSPUtils.jsm).
> 
> I'll fix this in my patch for bug 770099.

Thanks. I'd also like to request to make "policy URI" stuff more consistent in this patch. Currently there are 3 variants: policy-uri, policy uri and policy URI. Can you use only one of them everywhere, like "policy URI"?

Comment 3

[Francesco Lodolo [:flod]]

Could you also explain why some strings start with lower cases? I took a look at the code but they don't seem to be concatenated to other strings.

Comment 4

[Mark Goodwin [:mgoodwin]]

(In reply to Alexander L. Slovesnik from comment #2)
> I'd also like to request to make "policy URI" stuff more consistent
> in this patch. Currently there are 3 variants: policy-uri, policy uri and
> policy URI. Can you use only one of them everywhere, like "policy URI"?

We may need 2 variants: "policy-uri" when we're talking about the csp directive - http://www.w3.org/TR/CSP/#policy-uri (which probably shouldn't be translated) and "policy URI" when we're talking about what the directive represents.  I've not done l10n bugs before, so I'm not sure how we normally go about this (obviously, I'm keen to learn so advice is welcome!).

(In reply to Francesco Lodolo [:flod] from comment #3)
> Could you also explain why some strings start with lower cases? I took a
> look at the code but they don't seem to be concatenated to other strings.

Bug 766569 was literally about providing support for l10n where there previously was none; I literally copied and pasted the (previously hardcoded) strings from the CSPUtils.jsm and contentSecurityPolicy.js files into the csp.properties file.  Any case oddities, inconsistencies and typos are lifted, wholesale, from those files.

In answer to your question, though, I expect there's no reason; previously these messages would only ever appear in the error console so maybe they weren't carefully checked when the code was written. We're working on making all of the errors and warnings available via the Web Developer console (see Bug 712859 and Bug 770099) so starting work on at least supporting l10n in the CSP code seemed to make sense...

That said, there will likely be more work on CSP in the near future. Standardization of CSP 1.0 by the W3C may result in some changes to some of these strings; this raises a question over the value of taking this beyond code support at this time. Again, I'd welcome people's thoughts on this.

Comment 5

[Mark Goodwin [:mgoodwin]]

Attachment: Bug 770176 - Typo in failedToParseUnrecognizedSource, also some case and consistency fixes

I'd tried to consistently make use of directive names (e.g. policy-uri rather than policy URI) where possible; this should make usage fairly unambiguous.

I also fixed any case and typo issues I came across. I'm unsure on how we normally punctuate warnings and errors in the web and error consoles; any guidance here would be useful.

Comment 6

[Francesco Lodolo [:flod]]

Comment on attachment 638631 [details] [diff] [review]
Bug 770176 - Typo in failedToParseUnrecognizedSource, also some case and consistency fixes

Policy should be to change key names when fixing things bigger than typos in English.

In this case I personally wouldn't have a problem leaving the original keys and sending a warning to dev-l10n. CCing Axel to check his opinion.

In general, there's a mix of "can't/cannot", "can't/couldn't", "could not/couldn't", not sure if you want to fix that as well.

Comment 7

[Mark Goodwin [:mgoodwin]]

Attachment: Bug 770176 - Typo in failedToParseUnrecognizedSource, also some case and consistency fixes (take 2)

Settled on can't, couldn't, etc. as those were the predominant forms in the original strings.

Comment 8

[Alexander L. Slovesnik]

Comment on attachment 638637 [details] [diff] [review]
Bug 770176 - Typo in failedToParseUnrecognizedSource, also some case and consistency fixes (take 2)

> # LOCALIZATION NOTE (notETLDPlus1):
> # %1$S is the ETLD of the report URI that could not be used
>-notETLDPlus1 = can't use report URI from non-matching eTLD+1: %1$S
>+notETLDPlus1 = Can't use report-uri with non-matching eTLD+1: %1$S
I suppose that "eTLD+1" shouldn't be translated? Can you add it to LOCALIZATION NOTE?

> # CSP Errors:
> policyURINotAlone = policy-uri directive can only appear alone
Does it mean that policy-uri directive can not be used with other directives? I have hard time when trying to
guess what "alone" means. Perhaps LOCALIZATION NOTE could help? 

> # LOCALIZATION NOTE (nonMatchingHost):
> # %1$S is the URI host that does not match
>-nonMatchingHost = can't fetch policy uri from non-matching hostname: %1$S
>+nonMatchingHost = Can't fetch policy. policy-uri has a non-matching hostname: %1$S
It means that hostname of policy-uri and hostname of %1$S don't match, right? Just checking.

> # LOCALIZATION NOTE (errorFetchingPolicy):
> # %1$S is the error that caused fetching to fail
>-errorFetchingPolicy = Error fetching policy-uri: %1$S
>-cspSourceNotURI = Provided argument is not an nsIURI
>-argumentIsNotString = Provided argument is not a string
>-selfDataNotProvided = Can't use 'self' if self data is not provided
>+errorFetchingPolicy = Error fetching policy: %1$S
>+cspSourceNotURI = Provided argument isn't an nsIURI
Should it be "a nsURI"? nsURI doesn't start with vowel.

> # LOCALIZATION NOTE (hostSourceWithoutData):
> # %1$S is the source
> hostSourceWithoutData = Can't create host-only source %1$S without 'self' data
Please, explain in LOCALIZATION NOTE what "host-only" means. I have no idea.

Comment 9

[Mark Goodwin [:mgoodwin]]

(In reply to Alexander L. Slovesnik from comment #8)
> > policyURINotAlone = policy-uri directive can only appear alone
> Does it mean that policy-uri directive can not be used with other
> directives? I have hard time when trying to
> guess what "alone" means. Perhaps LOCALIZATION NOTE could help? 

Yes, that is the case. I'll make that clear in the note.

> >+nonMatchingHost = Can't fetch policy. policy-uri has a non-matching hostname: %1$S
> It means that hostname of policy-uri and hostname of %1$S don't match,
> right? Just checking.

Not quite; %1$S is the host component of the policy-uri. This differs from the host of the document URI so CSP refuses to fetch the policy.  Should this be clearer?

> Should it be "a nsURI"? nsURI doesn't start with vowel.

Yes, it should. I'll change instances of "an nsIURI" to "a nsIURI" 

> > hostSourceWithoutData = Can't create host-only source %1$S without 'self' data
> Please, explain in LOCALIZATION NOTE what "host-only" means. I have no idea.

From reading the code around this message, it seems a source is CSP speak for a location a resource may be obtained from; a host-only source would be where neither scheme nor port are specified.

^^ Would something like this be an adequate description for the localization note?

Comment 10

[Alexander L. Slovesnik]

(In reply to Mark Goodwin [:mgoodwin] from comment #9)
> (In reply to Alexander L. Slovesnik from comment #8)
> > >+nonMatchingHost = Can't fetch policy. policy-uri has a non-matching hostname: %1$S
> > It means that hostname of policy-uri and hostname of %1$S don't match,
> > right? Just checking.
> 
> Not quite; %1$S is the host component of the policy-uri. This differs from
> the host of the document URI so CSP refuses to fetch the policy.  Should
> this be clearer?
Yes, thanks for explanation.


> > > hostSourceWithoutData = Can't create host-only source %1$S without 'self' data
> > Please, explain in LOCALIZATION NOTE what "host-only" means. I have no idea.
> 
> From reading the code around this message, it seems a source is CSP speak
> for a location a resource may be obtained from; a host-only source would be
> where neither scheme nor port are specified.
> 
> ^^ Would something like this be an adequate description for the localization
> note?
Yes.

Comment 11

[Mark Goodwin [:mgoodwin]]

Attachment: Bug 770176 - Typo in failedToParseUnrecognizedSource, also some case and consistency fixes (take 3)

I have:
1) Added more information on an eTLD+1 leaving translators the option of either leaving it as-is or translating should the term be unknown or not used in their language.
2) Added a note to explain what 'can only appear alone' means
3) Changed the nonMatchingHost message to be more explicit
4) Corrected "an nsIURI" to "a nsIURI"
5) Added a note to explain what a host-only source is

Comment 12

[Mark Goodwin [:mgoodwin]]

Who would be a suitable reviewer?

Comment 13

[Francesco Lodolo [:flod]]

I'd try with Axel ([e-mail])

Comment 14

[Robert Kaiser]

Bah, apparently bug 655131 was never checked in so that the typo made it through the string extraction to a separate file. It needs localizers to cry out until such fixes make it into "code", I guess. ;-)

Comment 15

[Axel Hecht [:Pike]]

Comment on attachment 638658 [details] [diff] [review]
Bug 770176 - Typo in failedToParseUnrecognizedSource, also some case and consistency fixes (take 3)

Review of attachment 638658 [details] [diff] [review]:
-----------------------------------------------------------------

r-, because I think that some of the strings actually changed their message in meaningful ways. We already have a bunch of localizations for these strings, so we'll want key changes for those, or follow up bugs.

It'd be also good to add an initial comment about what CSP actually is.

Matej, would you have a cycle to go over the strings here, too, there seem to be a few ad-hoc decisions that could benefit from a writer guy.

::: dom/locales/en-US/chrome/security/csp.properties
@@ +76,3 @@
>  # LOCALIZATION NOTE (errorFetchingPolicy):
>  # %1$S is the error that caused fetching to fail
> +errorFetchingPolicy = Error fetching policy: %1$S

Sure that you want to drop policy-uri as terminology here?

@@ +87,5 @@
>  # %1$S is the source that could not be parsed
>  couldntParseInvalidSource = Couldn't parse invalid source %1$S
>  # LOCALIZATION NOTE (hostSourceWithoutData):
> +# A source is a location a resource may be obtained from. A host-only source would be 
> +# a source where neither scheme nor port are specified.

I have a hard time understanding the first sentence here.

Comment 16

[Mark Goodwin [:mgoodwin]]

(In reply to Axel Hecht [:Pike] from comment #15)
> r-, because I think that some of the strings actually changed their message
> in meaningful ways. We already have a bunch of localizations for these
> strings, so we'll want key changes for those, or follow up bugs.

Understood; presumably we should now try to finalize what changes are needed then change key names to reflect the difference (as per Comment 6)?

Apologies for not spotting these things earlier; I'd (naively) assumed the hardcoded strings would all be OK :(

> ::: dom/locales/en-US/chrome/security/csp.properties
> @@ +76,3 @@
> >  # LOCALIZATION NOTE (errorFetchingPolicy):
> >  # %1$S is the error that caused fetching to fail
> > +errorFetchingPolicy = Error fetching policy: %1$S
> 
> Sure that you want to drop policy-uri as terminology here?

I think that's more correct, yes. The browser already has the policy-uri. It was the attempt to fetch the policy from this URI that failed.

> @@ +87,5 @@
> >  # %1$S is the source that could not be parsed
> >  couldntParseInvalidSource = Couldn't parse invalid source %1$S
> >  # LOCALIZATION NOTE (hostSourceWithoutData):
> > +# A source is a location a resource may be obtained from. A host-only source would be 
> > +# a source where neither scheme nor port are specified.
> 
> I have a hard time understanding the first sentence here.

It's tough to explain without assuming knowledge of CSP or an example (as you say, a description of CSP with links to relevant documentation would be helpful).  We could borrow language from the spec, e.g. "A source represents a location from which content of a certain type can be retrieved" (from http://www.w3.org/TR/CSP/#source-list).

I'll get working on a description of CSP for the notes in csp.properties.

Thanks.

Comment 17

[Axel Hecht [:Pike]]

The parsing problem I have is actually that the sentence starts with "A source is a location a resource" ... and then there may be a verb, and then you have to work yourself back. My initial mental reaction was to remove stuttered words from the sentence, which didn't help, and then it slowly dawned on me. I'd leave that up to word smiths to actually propose something constructive.

Comment 18

[Matej Novak [:matej]]

I think I got confused and made comments to the wrong version, but they still apply to the updated draft. Mostly it's a question of inconsistent capitals and periods (I'd use both). As for the contractions, I actually prefer not using them in this case. Happy to be overruled, but I'd suggest "could not," "can not" (two words), etc. in this case.

Let me know if you have any questions.

Comment 19

[Matej Novak [:matej]]

Comment on attachment 638658 [details] [diff] [review]
Bug 770176 - Typo in failedToParseUnrecognizedSource, also some case and consistency fixes (take 3)

Review of attachment 638658 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/locales/en-US/chrome/security/csp.properties
@@ +10,5 @@
>  # %1$S is the directive that has been violated.
>  directiveViolated = Directive %1$S violated
>  # LOCALIZATION NOTE (directiveViolatedWithURI):
>  # %1$S is the directive that has been violated.
>  # %2$S is the URI of the resource which violated the directive.

Should be "that violated the directive."

@@ -7,5 @@
>  # %1$S is the directive that has been violated.
>  directiveViolated = Directive %1$S violated
>  # LOCALIZATION NOTE (directiveViolatedWithURI):
>  # %1$S is the directive that has been violated.
>  # %2$S is the URI of the resource which violated the directive.

Should be "that violated the directive."

@@ -10,5 @@
>  # %1$S is the directive that has been violated.
>  # %2$S is the URI of the resource which violated the directive.
>  directiveViolatedWithURI = Directive %1$S violated by %2$S
>  # LOCALIZATION NOTE (triedToSendReport):
>  # %1$S is the URI we attempted to send a report to.

Should be "is the URI that we attempted to send a report to."

@@ -13,5 @@
>  # LOCALIZATION NOTE (triedToSendReport):
>  # %1$S is the URI we attempted to send a report to.
>  triedToSendReport = Tried to send report to invalid URI: "%1$S"
>  # LOCALIZATION NOTE (errorWas):
>  # %1$S is the error resulting from attempting to send the report

Period missing. This seems to be a recurring thing, so I won't mention it everywhere, but it should be consistent (i.e. any full sentence should end with a period).

Also, any complete sentence should start with a capital letter. This is also inconsistent throughout.

@@ -19,3 @@
>  # LOCALIZATION NOTE (couldNotParseReportURI):
> -# %1$S is the report URI that could not be parsed
> -couldNotParseReportURI = couldn't parse report URI: %1$S

Looks like there's still some could vs. couldn't inconsistency. In strings like these, I would avoid contractions and go with "could not" throughout.

@@ -26,2 @@
>  # %1$S is the option that could not be understood
> -doNotUnderstandOption = don't understand option '%1$S'.  Ignoring it.

Same goes here. I'd say "do not" instead.

@@ +32,2 @@
>  # LOCALIZATION NOTE (notETLDPlus1):
> +# eTLD should be understood by English speaking developers as effective top-level domain

"English-speaking" should be hyphenated.

@@ -28,3 @@
>  # LOCALIZATION NOTE (notETLDPlus1):
> -# %1$S is the ETLD of the report URI that could not be used
> -notETLDPlus1 = can't use report URI from non-matching eTLD+1: %1$S

Ditto for "can't." Let's make it "can not," two words.

@@ -37,4 @@
>  # LOCALIZATION NOTE (pageCannotSendReportsTo):
>  # %1$S is the URI of the page with the policy
>  # %2$S is the report URI that could not be used
> -pageCannotSendReportsTo = page on %1$S cannot send reports to %2$S

Again, "can not" should be two words throughout.

@@ +51,3 @@
>  allowOrDefaultSrcRequired = 'allow' or 'default-src' directive required but not present.  Reverting to "default-src 'none'"
>  # LOCALIZATION NOTE (failedToParseUnrecognizedSource):
>  # %1$S is the CSP Source that could not be parsed

Does "source" need to be capitalized here?

@@ -41,3 @@
>  allowOrDefaultSrcRequired = 'allow' or 'default-src' directive required but not present.  Reverting to "default-src 'none'"
>  # LOCALIZATION NOTE (failedToParseUnrecognizedSource):
>  # %1$S is the CSP Source that could not be parsed

Does "source" need to be capitalized here?

@@ +55,3 @@
>  # LOCALIZATION NOTE (reportPostRedirect):
>  # %1$S is the specified report URI before redirect
>  reportPostRedirect = Post of violation report to %1$S failed, as a redirect occurred

I'd make this "Post of violation report to %1$S failed; a redirect occurred."

@@ -45,3 @@
>  # LOCALIZATION NOTE (reportPostRedirect):
>  # %1$S is the specified report URI before redirect
>  reportPostRedirect = Post of violation report to %1$S failed, as a redirect occurred

I'd make this "Post of violation report to %1$S failed; a redirect occurred."

@@ +87,5 @@
>  # %1$S is the source that could not be parsed
>  couldntParseInvalidSource = Couldn't parse invalid source %1$S
>  # LOCALIZATION NOTE (hostSourceWithoutData):
> +# A source is a location a resource may be obtained from. A host-only source would be 
> +# a source where neither scheme nor port are specified.

I think there's an easy fix here: "A source is a location that a resource may be obtained from."

@@ -80,5 @@
>  # LOCALIZATION NOTE (sourceWithoutData):
>  # %1$S is the source
>  sourceWithoutData = Can't create source %1$S without 'self' data
>  # LOCALIZATION NOTE (couldntParseInvalidHost):
>  # %1$S is the host that's invalid

As with other contractions, this should be "that is"

Comment 20

[Axel Hecht [:Pike]]

Comment on attachment 638658 [details] [diff] [review]
Bug 770176 - Typo in failedToParseUnrecognizedSource, also some case and consistency fixes (take 3)

Mark, are Matej's comments helpful?

Comment 21

[Mark Goodwin [:mgoodwin]]

(In reply to Axel Hecht [:Pike] from comment #20)
> Mark, are Matej's comments helpful?

Yes, thankyou. I'll hopefully have time to get back to this later on this week (maybe tomorrow).

Comment 22

[Mark Goodwin [:mgoodwin]]

There's work going on in the CSP code that will likely render some fixes I make here (and any subsequent translation) useless; I'll revisit this when bug 783049 and bug 746978 are fixed.

Comment 23

[Sid Stamm [:geekboy or :sstamm]]

Can we land the typo fix now and take care of the rest later?

Comment 24

[Mark Goodwin [:mgoodwin]]

Attachment: Bug 770176 - Typo in failedToParseUnrecognizedSource

Alternative to attachment 638658 [details] [diff] [review] (as requested by sid).

Comment 25

[Mark Goodwin [:mgoodwin]]

(In reply to Sid Stamm [:geekboy] from comment #23)
> Can we land the typo fix now and take care of the rest later?

Would you like a separate bug to deal with the other changes that are needed (inconsistencies, CSP 1.0 additions / changed, etc.)?

Comment 26

[Sid Stamm [:geekboy or :sstamm]]

pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/412451c0f918

mgoodwin: please file a follow-up bug for this about the inconsistencies and other issues in the text we have to fix.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/412451c0f918