Closed Bug 1267258 Opened 4 years ago Closed 4 years ago

Add license header and localization comments to toolkit/global/extensions.properties

Categories

(WebExtensions :: Untriaged, defect, P2)

defect

Tracking

(firefox49 fixed)

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: flod, Assigned: kmag)

References

Details

(Whiteboard: triaged)

Attachments

(1 file)

File was created in bug 1254194
http://hg.mozilla.org/mozilla-central/diff/58fc5fb221e0/toolkit/locales/en-US/chrome/global/extensions.properties

It's missing a license header, but more important is missing localization comments explaining what variables are replaced with.

Can you please add comments and context? For example I have no idea why a string like this one has : after the variable

> csp.error.illegal-protocol = '%1$S' directive contains a forbidden %2$S: protocol source
Summary: Add license header and localization comments to → Add license header and localization comments to toolkit/global/extensions.properties
Assignee: nobody → kmaglione+bmo
Flags: blocking-webextensions+
Whiteboard: triaged
Flags: blocking-webextensions+
Comment on attachment 8745113 [details]
MozReview Request: Bug 1267258: Add licence/localization note comments to extensions.properties. r?flod

https://reviewboard.mozilla.org/r/48855/#review45743

Thanks, only need to fix the string references but with comments it looks a lot better. It's good to

You might also want to use “%S” instead of '%S', based on bug 1259859 recently landed (no need to change string IDs if you do).

::: toolkit/locales/en-US/chrome/global/extensions.properties:7
(Diff revision 1)
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  csp.error.missing-directive = Policy is missing a required '%S' directive
>  
> +#LOCALIZATION NOTE (ccsp.error.illegal-protocol) %1$S is the name of a CSP directive, such as "script-src". %2$S is the name of a CSP keyword, usually 'unsafe-inline'.

Wrong string reference: csp.error.illegal-keyword

::: toolkit/locales/en-US/chrome/global/extensions.properties:13
(Diff revision 1)
>  csp.error.illegal-keyword = '%1$S' directive contains a forbidden %2$S keyword
>  
> +#LOCALIZATION NOTE (ccsp.error.illegal-protocol) %2$S a protocol name, such as "http", which appears as "http:", as it would in a URL.
>  csp.error.illegal-protocol = '%1$S' directive contains a forbidden %2$S: protocol source
>  
> +#LOCALIZATION NOTE (ccsp.error.illegal-protocol) %2$S a protocol name, such as "http", which appears as "http:", as it would in a URL.

Wrong string reference.

::: toolkit/locales/en-US/chrome/global/extensions.properties:16
(Diff revision 1)
>  csp.error.illegal-protocol = '%1$S' directive contains a forbidden %2$S: protocol source
>  
> +#LOCALIZATION NOTE (ccsp.error.illegal-protocol) %2$S a protocol name, such as "http", which appears as "http:", as it would in a URL.
>  csp.error.missing-host = %2$S: protocol requires a host in '%1$S' directives
>  
> +#LOCALIZATION NOTE (ccsp.error.illegal-protocol) %1$S is the name of a CSP directive, such as "script-src". %2$S is the name of a CSP source, usually 'self'.

Same here.

::: toolkit/locales/en-US/chrome/global/extensions.properties:19
(Diff revision 1)
>  csp.error.missing-host = %2$S: protocol requires a host in '%1$S' directives
>  
> +#LOCALIZATION NOTE (ccsp.error.illegal-protocol) %1$S is the name of a CSP directive, such as "script-src". %2$S is the name of a CSP source, usually 'self'.
>  csp.error.missing-source = '%1$S' must include the source %2$S
>  
> +#LOCALIZATION NOTE (ccsp.error.illegal-protocol) %2$S a protocol name, such as "http", which appears as "http:", as it would in a URL.

Same here.
Attachment #8745113 - Flags: review?(francesco.lodolo)
https://reviewboard.mozilla.org/r/48855/#review45743

Done. Thanks.

> Wrong string reference: csp.error.illegal-keyword

Sorry about that.
Comment on attachment 8745113 [details]
MozReview Request: Bug 1267258: Add licence/localization note comments to extensions.properties. r?flod

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48855/diff/1-2/
Attachment #8745113 - Flags: review?(francesco.lodolo)
Comment on attachment 8745113 [details]
MozReview Request: Bug 1267258: Add licence/localization note comments to extensions.properties. r?flod

https://reviewboard.mozilla.org/r/48855/#review45943

It looks great, thanks.
Attachment #8745113 - Flags: review?(francesco.lodolo) → review+
Oops. Looks like this failed to land because of a merge conflict and I didn't notice.
https://hg.mozilla.org/integration/fx-team/rev/10e00a4afa507911c110ca779a0769922f9e9ea8
Bug 1267258: Add licence/localization note comments to extensions.properties. r=flod
https://hg.mozilla.org/mozilla-central/rev/10e00a4afa50
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.