1425280 - Fix localization note mismatch in devtools

Status: RESOLVED FIXED

(DevTools :: General, enhancement, P3)

Comment 1

[Julian Descottes [:jdescottes]]

Try to catch localization note mistakes early (e.g. https://bugzilla.mozilla.org/show_bug.cgi?id=1423687#c18). This can be done using a mochitest similar to https://searchfox.org/mozilla-central/source/browser/base/content/test/static/browser_misused_characters_in_strings.js

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: Bug 1425280 - add a test to catch localization note mismatch

Review commit: https://reviewboard.mozilla.org/r/207704/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/207704/

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1425280 - Fix localization note mismatch in devtools files;

Review commit: https://reviewboard.mozilla.org/r/207706/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/207706/

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1425280 - Fix localization notes (non-devtools files)

Review commit: https://reviewboard.mozilla.org/r/207708/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/207708/

Comment 5

[Julian Descottes [:jdescottes]]

Comment on attachment 8936967 [details]
Bug 1425280 - add a test to catch localization note mismatch

I would like to have a test similar to browser_misused_characters_in_strings.js but focused on catching localization note issues. Initially I thought I would only add it for devtools, but that might be interesting for m-c so let me know what you think.

The test itself is not very strict. It only checks that when there _is_ a localization note:
- it matches a definition in the dtd/properties file
- it is not duplicated with another localization note (which is usually what happens when you copy paste another string)

Comment 6

[Francesco Lodolo [:flod]]

Comment on attachment 8936967 [details]
Bug 1425280 - add a test to catch localization note mismatch

The idea sounds good (gosh, so many errors), but I'd like to hear Pike's thoughts on the actual implementation, where I can't really be that useful.

Comment 7

[Axel Hecht [:Pike]]

Comment on attachment 8936967 [details]
Bug 1425280 - add a test to catch localization note mismatch

https://reviewboard.mozilla.org/r/207704/#review213620

In bug 1353680, I'm adding a linter for l10n, based on existing parsers. I'm actually rebasing that patch these days, so it's not that that bug is as stale as it looks.

This test should be part of that infrastructure, and I really don't want us to have more variants of trying to parse our l10n files.

Thus I'm giving this an r-, as much as this obviously helped you to find bugs.

Comment 8

[Julian Descottes [:jdescottes]]

I will block this on your linter bug and will revisit once the linter infrastructure is ready.

Comment 9

[Julian Descottes [:jdescottes]]

Repurposing to land the devtools-only fixes.

Comment 10

[Julian Descottes [:jdescottes]]

Comment on attachment 8936968 [details]
Bug 1425280 - Fix localization note mismatch in devtools files;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/207706/diff/1-2/

Comment 11

[Patrick Brosset <:pbro>]

Comment on attachment 8936968 [details]
Bug 1425280 - Fix localization note mismatch in devtools files;

https://reviewboard.mozilla.org/r/207706/#review215400

Thank you for catching and fixing all of these inconsistencies Julian.

Comment 12

[Julian Descottes [:jdescottes]]

Thanks for the review! I'll keep monitoring bug 1353680, hopefully we can have a linter in the future to avoid reintroducing this kind of issues.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/889f51eea06e
Fix localization note mismatch in devtools files;r=pbro

Comment 14

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/889f51eea06e