Extract strings using WebExtension i18n library #2344
Conversation
There was a problem hiding this comment.
I didn't add a comment to all strings because it would get messy, but I'd suggest a different approach in naming these strings.
If a string is going to be used as a title, let's use Title in the ID, that will spare us comments in the localization file.
Note that we'll still need to manually add some comments later, e.g. for "Download"
| "loginErrorDetails": { "message": "Your shot was not saved. There was an error authenticating with the server." }, | ||
| "loginConnectionErrorDetails": { "message": "There may be a problem with the service or your network connection." }, | ||
| "unshootablePageError": { "message": "Page cannot be screenshotted." }, | ||
| "unshootablePageErrorDetails": { "message": "This is not a normal web page, and Page Shot cannot capture screenshots from it." }, |
There was a problem hiding this comment.
Unrelated copy question: "capture screenshots from it"? I would expect "of it" here.
There was a problem hiding this comment.
"capture screenshot from it" and "capture screenshot of it" read basically the same to me; I have no opinion.
| "loginConnectionErrorDetails": { "message": "There may be a problem with the service or your network connection." }, | ||
| "unshootablePageError": { "message": "Page cannot be screenshotted." }, | ||
| "unshootablePageErrorDetails": { "message": "This is not a normal web page, and Page Shot cannot capture screenshots from it." }, | ||
| "recursivePageShotShotError": { "message": "You can't take a shot of a Page Shot page!" }, |
There was a problem hiding this comment.
I suggest to use proper apostrophes instead of straight quotes.
You can’t take a shot of a Page Shot page!
For reference, such a string would fail tests on Firefox and be rejected.
| "selectorCancel": { "message": "Cancel" }, | ||
| "selectorDownload": { "message": "Download" }, | ||
| "contextMenuText": { "message": "Create Page Shot" }, | ||
| "notificationLinkCopied": { "message": "Link Copied" }, |
There was a problem hiding this comment.
Can we use notificationLinkCopiedTitle to clarify the role of this string? That would spare us a localization comment in the .properties file
| "unshootablePageErrorDetails": { "message": "This is not a normal web page, and Page Shot cannot capture screenshots from it." }, | ||
| "recursivePageShotShotError": { "message": "You can't take a shot of a Page Shot page!" }, | ||
| "genericError": { "message": "Page Shot went haywire." }, | ||
| "genericErrorTryAgain": { "message": "Try again or take a shot on another page?" } |
| }, | ||
| SHOT_PAGE: { | ||
| title: "You can't take a shot of a Page Shot page!" | ||
| title: browser.i18n.getMessage("recursivePageShotShotError") |
There was a problem hiding this comment.
How come SHOT_PAGE and MY_SHOTS don't have a info, and use a really long title instead?
There was a problem hiding this comment.
@flodolo Thanks very much for your feedback here. A lot of the error messages reuse the same strings, so I thought it would be best to avoid duplication, and it seemed awkward in the code for error message X to use a string named ERROR_Y. However, I wasn't thinking that the primary audience for the l10n keys is actually the localizers, not the engineers. I'll overhaul the names to reflect their usage in the app, rather than awkwardly summarize their English language content.
There was a problem hiding this comment.
Oh, and to your question at this line - I'm not sure why some errors don't have a second component, probably just an arbitrary implementation choice
jaredhirsch
force-pushed
the
extract-strings
branch
from
930a954
to
94147e3
Compare
|
@ianb Mind taking a look? |
There was a problem hiding this comment.
Code looks good to me. As mentioned before, when you are ready to generate messages.json you could do that similarly to how inlineSelectionCss.js is generated. Though for multiple files it'll be a little more complicated... but if you do that first part then I can do the multiple file stuff in the Makefile.
- Add pontoon-to-webext.js script from bwinton/SnoozeTabs repo and related npm dependencies (to be removed when/if that script is published on npm as a standalone module). - Add extracted strings to a properties file at the location expected by Pontoon. - Add a pontoon-to-webextension build step to the Makefile - Also, replace the hand made messages.json with the more nicely-formatted script output. This commit, together with the fix for mozilla-services#2344, closes mozilla-services#2294.
- Add pontoon-to-webext.js script from bwinton/SnoozeTabs repo and related npm dependencies (to be removed when/if that script is published on npm as a standalone module). - Add extracted strings to a properties file at the location expected by Pontoon. - Add a pontoon-to-webextension build step to the Makefile - Also, replace the hand made messages.json with the more nicely-formatted script output. This commit, together with the fix for mozilla-services#2344, closes mozilla-services#2294.
- Add pontoon-to-webext.js script from bwinton/SnoozeTabs repo and related npm dependencies (to be removed when/if that script is published on npm as a standalone module). - Add extracted strings to a properties file at the location expected by Pontoon. - Add a pontoon-to-webextension build step to the Makefile - Also, replace the hand made messages.json with the more nicely-formatted script output. This commit, together with the fix for mozilla-services#2344, closes mozilla-services#2294.
- Add pontoon-to-webext.js script from bwinton/SnoozeTabs repo and related npm dependencies (to be removed when/if that script is published on npm as a standalone module). - Add extracted strings to a properties file at the location expected by Pontoon. - Add a pontoon-to-webextension build step to the Makefile - Also, replace the hand made messages.json with the more nicely-formatted script output. This commit, together with the fix for mozilla-services#2344, closes mozilla-services#2294.
- Add pontoon-to-webext.js script from bwinton/SnoozeTabs repo and related npm dependencies (to be removed when/if that script is published on npm as a standalone module). - Add extracted strings to a properties file at the location expected by Pontoon. - Add a pontoon-to-webextension build step to the Makefile - Also, replace the hand made messages.json with the more nicely-formatted script output. This commit, together with the fix for mozilla-services#2344, closes mozilla-services#2294.
jaredhirsch
force-pushed
the
extract-strings
branch
from
94147e3
to
b3df1c6
Compare
- Add pontoon-to-webext.js script from bwinton/SnoozeTabs repo and related npm dependencies (to be removed when/if that script is published on npm as a standalone module). - Add extracted strings to a properties file at the location expected by Pontoon. - Add a pontoon-to-webextension build step to the Makefile - Also, replace the hand made messages.json with the more nicely-formatted script output. This commit, together with the fix for mozilla-services#2344, closes mozilla-services#2294.
- Add pontoon-to-webext.js script from bwinton/SnoozeTabs repo and related npm dependencies (to be removed when/if that script is published on npm as a standalone module). - Add extracted strings to a properties file at the location expected by Pontoon. - Add a pontoon-to-webextension build step to the Makefile - Also, replace the hand made messages.json with the more nicely-formatted script output. This commit, together with the fix for mozilla-services#2344, closes mozilla-services#2294.
- Add pontoon-to-webext.js script from bwinton/SnoozeTabs repo and related npm dependencies (to be removed when/if that script is published on npm as a standalone module). - Add extracted strings to a properties file at the location expected by Pontoon. Remove the webextension-formatted strings from git. - Add a pontoon-to-webextension build step to the Makefile. This commit, together with the fix for mozilla-services#2344, closes mozilla-services#2294.
- Add pontoon-to-webext.js script from bwinton/SnoozeTabs repo and related npm dependencies (to be removed when/if that script is published on npm as a standalone module). - Add extracted strings to a properties file at the location expected by Pontoon. Remove the webextension-formatted strings from git. - Add a pontoon-to-webextension build step to the Makefile. This commit, together with the fix for mozilla-services#2344, closes mozilla-services#2294.
Fixes part of #2294. Doesn't yet manage the transformation from a pontoon .properties file to this messages.json format.
I haven't been super consistent about the l10n keys. If anyone has strong opinions here, I am open to suggestions (cc @flodolo).