Move about:debugging l10n strings alongside other devtools localizations

RESOLVED FIXED in Firefox 68

Status

enhancement
P1
normal
RESOLVED FIXED
9 months ago
2 months ago

People

(Reporter: birtles, Assigned: jdescottes)

Tracking

(Depends on 1 bug, Blocks 1 bug)

unspecified
Firefox 68
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(16 attachments, 1 obsolete attachment)

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
(Reporter)

Description

9 months ago
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.

Comment 1

9 months ago
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 ;-)
(Assignee)

Updated

8 months ago
(Assignee)

Updated

8 months ago
No longer blocks: remote-debugging-ng-m1
(Assignee)

Updated

7 months ago
(Assignee)

Updated

7 months ago
Depends on: 1498149
Priority: P3 → P2
(Assignee)

Comment 2

3 months ago

Can only be done once the UX bugs are finished.

Priority: P2 → P3
Priority: P3 → P2
(Assignee)

Updated

2 months ago
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Priority: P2 → P1
(Assignee)

Comment 3

2 months ago

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

(Assignee)

Comment 4

2 months ago

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.

(Assignee)

Comment 5

2 months ago

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

(Assignee)

Comment 6

2 months ago

Depends on D23865

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

(Assignee)

Comment 7

2 months ago

Depends on D23867.

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

(Assignee)

Comment 9

2 months ago

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

(Assignee)

Comment 10

2 months ago

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.

(Assignee)

Comment 12

2 months ago

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

(Assignee)

Comment 18

2 months ago

Queue can be pulled from

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

(Assignee)

Comment 20

2 months ago

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

(Assignee)

Comment 21

2 months ago

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

(Assignee)

Comment 22

2 months ago

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).

(Assignee)

Comment 26

2 months ago

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.

(Assignee)

Comment 27

2 months ago

Depends on D23878

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

Attachment #9052253 - Attachment is obsolete: true

Comment 29

2 months ago
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.