Closed Bug 1487581 Opened 2 years ago Closed 1 year ago

Move about:debugging l10n strings alongside other devtools localizations

Categories

(DevTools :: about:debugging, enhancement, P1)

enhancement

Tracking

(firefox68 fixed)

RESOLVED FIXED
Firefox 68
Tracking Status
firefox68 --- fixed

People

(Reporter: birtles, Assigned: jdescottes)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(16 files, 1 obsolete file)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
In bug 1485901 we went out of our way to make our localization out-of-band until the UI stablizes. This is in order to avoid annoying localizers with constant flux as well as removing the need to get l10n sign-off every time we add a string (something we expect to do a lot in the coming weeks).

Once the strings have stabilized, however, we should move them alongside the other devtools localizations and rename the .notftl file to .ftl so that localization tools pick them up.

I'm mostly filing this bug because, as pointed out in bug 1485901 comment 8, when renaming the .notftl file to .ftl we should be careful NOT TO `hg mv` IT! Instead we should `cp; hg add; hg rm`. That way the version history will be lost, but that's a good thing because otherwise l10n toolchains may try to look it up and get confused by the .notftl file.
Thanks for filing this.

Technical detail, hg addremove does rename detection. hg add; hg rm should do what we want, hg addremove -s0, too. Not specifying -s will detect the rename ;-)
No longer blocks: remote-debugging-ng-m1
Depends on: 1498149
Priority: P3 → P2

Can only be done once the UX bugs are finished.

Priority: P2 → P3
Priority: P3 → P2
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Priority: P2 → P1

Follow up to Bug 1505124. L10n cleanup before moving the strings

Depends on D23863. We want to enable network locations by default.
Remove all the logic and l10n strings related to this before moving the l10n strings.

Depends on D23864. Wifi preference is not used anymore, removing it as well for consistency.

Depends on D23865

Some localized strings missing in the notftl file. Adding them before mowing the strings.

Depends on D23867.

As described in https://projectfluent.org/fluent/guide/selectors.html selectors should rather be used for quantities.

Depends on D23869. Follow up to https://bugzilla.mozilla.org/show_bug.cgi?id=1505128
Localization is missing for this button, adding it here.

Depends on D23870

@daisuke this comes from your bug at https://bugzilla.mozilla.org/show_bug.cgi?id=1529518
You have not landed yet, so you can either review the patch here or fold it in your queue.
I am fine with both solutions.

Depends on D23872.

If you want to test the patch, I will post a try link afterwards to pull a valid queue
since my work depends on another bug that is almost ready to land but not yet landed.

Then you can open aboutdebugging-new by going to about:debugging-new

Queue can be pulled from

hg pull https://hg.mozilla.org/try/ -r 213b8701ece0

new patch queue at hg pull https://hg.mozilla.org/try/ -r 74251a452adb

Green try except for all_files_referenced, which still had a ref to the removed ftl. Now fixed on phabricator.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6832d3a63b3f65ec145b3d77218d8c2a0457cd18

Hi Francesco,

I have question regarding the last patch, to avoid hardcoding "This Firefox".

I added the patch because I agree that for a rebranded Firefox, mentioning "Firefox" everywhere doesn't make sense. I think the usage of about:debugging on rebranded Firefox versions should be low, but still, it would be better to use a correct brand name.

From a product perspective, we would prefer to stick to Firefox, even for Nightly. This way our documentation on MDN would be consistent with the UI, regardless of the channel you are on. I think the current patch is acceptable, but I'd still like to check with you for alternatives before committing to it.

Right now, brand.properties (and equivalent) doesn't have a string that would only say Firefox for all the Firefox release channels.

One option would be to introduce a new string that always says Firefox. My assumption is that it will be difficult to find a good name for this string however, and it might be confusing to consumers of the file who are not working on about:debugging.

Another option would be to do it using a selector in our ftl file:

-aboutdebugging-brand-name =
    { -brand-short-name ->
        [Firefox] Firefox
        [Nightly] Firefox
       *[other] { -brand-short-name }
    }

Last option would be to create this "aboutdebugging brand name" programmatically in JS and expect it as a variable in every ftl string that needs to say "Firefox".

What is your opinion about this?

Flags: needinfo?(francesco.lodolo)

(In reply to Julian Descottes [:jdescottes] from comment #22)

Right now, brand.properties (and equivalent) doesn't have a string that would only say Firefox for all the Firefox release channels.

Good question. So far we always hard-coded these, but that doesn't solve the problem for someone wanting to build and rebrand the source code, or unofficial versions.

Maybe the solution is to have a term in brand.ftl that it's identical in all release channels, and we can use that

-nochannel-brand = Firefox

One option would be to introduce a new string that always says Firefox. My assumption is that it will be difficult to find a good name for this string however, and it might be confusing to consumers of the file who are not working on about:debugging.

A clear comment would help.

Another option would be to do it using a selector in our ftl file:

That wouldn't really solve the rebranded builds. Instead of fixing one file, you would need to fix also this one.

Last option would be to create this "aboutdebugging brand name" programmatically in JS and expect it as a variable in every ftl string that needs to say "Firefox".

Same here regarding rebranded builds. At that point, you should just write "Firefox" in the strings (as it was before the patch).

Maybe we're overthinking this, and we should just use "Firefox" in the string. After all, that's already happening in other parts of the code, more visible than DevTools (e.g. new profile-per-install feature).

@Pike
Do you have any thoughts about this? The question is about having strings like "This Firefox", where we want "Firefox" across all versions of Firefox, but also avoid hard-coding if possible.

Flags: needinfo?(francesco.lodolo) → needinfo?(l10n)

My recommendation would be to use -brand-shorter-name to deal with dev edition, and take the hit on Nightly, https://searchfox.org/mozilla-central/search?q=-brand-short&case=false&regexp=false&path=browser%2Fbranding%2F**%2Fbrand.ftl.

A couple of general comments:

I had a hard time making sense of the comment. The missing "be" in "This should the same string" was the least part of that.

There seems to be some uses of DOM overlay-like code, but not really DOM overlays? I see a number of <a>Troubleshooting</a> w/out data-l10n-name. As we enhance our support for Fluent in our l10n ecosystem, those won't get the shared love. https://github.com/projectfluent/fluent.js/wiki/DOM-Overlays#functional-elements has more info. Even if you had to roll your own, it'd might be a good idea to be doing so in a compatible manner.

Flags: needinfo?(l10n)

(In reply to Axel Hecht [:Pike] from comment #24)

I had a hard time making sense of the comment. The missing "be" in "This should the same string" was the least part of that.

Which comment?

There seems to be some uses of DOM overlay-like code, but not really DOM overlays? I see a number of <a>Troubleshooting</a> w/out data-l10n-name. As we enhance our support for Fluent in our l10n ecosystem, those won't get the shared love. https://github.com/projectfluent/fluent.js/wiki/DOM-Overlays#functional-elements has more info. Even if you had to roll your own, it'd might be a good idea to be doing so in a compatible manner.

I assume they're using Fluent React, and that doesn't have DOM overlays? I believe stas has looked at the code in the past (I haven't, yet).

Thanks for the feedback!

My recommendation would be to use -brand-shorter-name to deal with dev edition, and take the hit on Nightly, https://searchfox.org/mozilla-central/search?q=-brand-short&case=false&regexp=false&path=browser%2Fbranding%2F**%2Fbrand.ftl.

Ok we will keep the current patch then and use -brand-shorter-name.

I had a hard time making sense of the comment. The missing "be" in "This should the same string" was the least part of that.

True, it is hard to understand, I will try to simplify the comment for about-debugging-this-firefox-runtime-name.

Depends on D23878

New strings that landed in the notftl file since the review started.

Attachment #9052253 - Attachment is obsolete: true
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/603ea56b9c22
Rename leftover connect-* strings to setup-* strings;r=ladybenko
https://hg.mozilla.org/integration/autoland/rev/aa66570a3055
Remove preference to disable network locations section;r=ladybenko
https://hg.mozilla.org/integration/autoland/rev/244fe8379400
Remove the wifi debugging preference;r=ladybenko
https://hg.mozilla.org/integration/autoland/rev/a62f6205fe9a
Add missing localized strings for Setup page;r=ladybenko
https://hg.mozilla.org/integration/autoland/rev/87244f1a558f
Stop using selectors for the aboutdebugging page title;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/2b048084fad5
Stop using selectors for the service worker status;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/edb593a7f2d7
Add localization for Disconnect button;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/470b06ab35bd
Fix localization for extension fields;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/94e78aba14dd
Create aboutdebugging.ftl and use it as localization file in aboutdebugging-new;r=daisuke,flod
https://hg.mozilla.org/integration/autoland/rev/43745ab22c07
Move l10n strings for Setup page to aboutdebugging.ftl;r=flod
https://hg.mozilla.org/integration/autoland/rev/c2941df6fb43
Move l10n strings for Sidebar to aboutdebugging.ftl;r=flod
https://hg.mozilla.org/integration/autoland/rev/08444737c455
Move l10n strings for page title to aboutdebugging.ftl;r=flod
https://hg.mozilla.org/integration/autoland/rev/aa3917fd744f
Move l10n strings for the Runtime page to aboutdebugging.ftl;r=flod
https://hg.mozilla.org/integration/autoland/rev/22dba673d12d
Move l10n strings for Debug Targets to aboutdebugging.ftl;r=flod
https://hg.mozilla.org/integration/autoland/rev/8623271908e0
Remove aboutdebugging.notftl file;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/c475753fb341
Stop using hardcoded Firefox string in aboutdebugging;r=flod,ladybenko
You need to log in before you can comment on or make changes to this bug.