Opened 8 months ago

Last modified 6 months ago

#33892 merge_ready defect

Add brandProductName to torbutton brand localization files

Reported by: acat Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: TorBrowserTeam202006R
Cc: tbb-team Actual Points: 0.75
Parent ID: #33533 Points:
Reviewer: Sponsor:

Description

brandProductName was already present in ESR68, but we did not update torbutton brand.dtd or brand.properties files. For 68, this does not break the browser, since brandProductName is not used as an xml entity (DTD), and the .properties usage is in a path we probably do not hit.

Currently, this is string is used in several additional places, and starting Firefox 70 it's used as a DTD entity, which prevents the browser from starting if it's not defined.

Child Tickets

Change History (5)

comment:1 Changed 8 months ago by acat

Keywords: TorBrowserTeam202004R added; TorBrowserTeam202004 removed
Status: newneeds_review

comment:2 in reply to:  1 Changed 8 months ago by mcs

Keywords: TorBrowserTeam202004 added; TorBrowserTeam202004R removed
Status: needs_reviewneeds_information

Replying to acat:

Patch in https://github.com/acatarineu/torbutton/commit/8205657aaa54fb438553ab715d6ea067e3668f3c

It looks like you were not 100% consistent when copying the strings from brandShortName to brandProductName. The names do not match for these locales: fa, lt, ms, nl, zh-CN (note that for zh-CN, the existing product name strings are translated in the properties file but not in the dtd). Is that what you intended?

Another approach would be to only update the en-US locale files; then our translators might take another look at the existing strings when they add a translation for brandProductName.

comment:3 Changed 8 months ago by acat

Actual Points: 0.75
Keywords: TorBrowserTeam202004R added; TorBrowserTeam202004 removed
Status: needs_informationneeds_review

Thanks, nice catch. I took the string from the already existing -brand-product-name in tor-browser-brand.ftl, but I didn't realize that it's not localized in Transifex, so the strings there are a bit outdated.

I revised the patch to use brandShortName from .properties and .dtd, instead. Not sure what to do with the inconsistency that you mentioned for zh-CN. The script I used to revise the patch just took it from the respective .properties or .dtd file, so brandProductName is also translated in .properties but not in .dtd.

I was thinking that perhaps it would be worth to take one brand file as the source (say brand.properties), and autogenerate the other two (brand.dtd and tor-browser-brand.ftl) when importing translations. At least for tor-browser-brand.ftl I think we should do it, since Fluent is not supported in Transifex, so this would be one way to keep it up to date when importing new translations.

So in my 33533+2 branch I added a couple of commits:

https://github.com/acatarineu/torbutton/commit/9e2c819e15c93362b27e0cb1d33d27dba9dfbaad is the revised one for this patch.

https://github.com/acatarineu/torbutton/commit/04f8130c112facef3e204f568b2077ca1b045c07 is some extra patch I did for autogenerating tor-browser-brand.ftl. I also ran the autogenerate part of import-translations.sh (without fetching for new translations), so there are actually some changes to tor-browser-brand.ftl. We can also do this in a separate ticket, or just drop it if you think there's a better approach for this.

comment:4 Changed 7 months ago by mcs

Status: needs_reviewmerge_ready

r=brade,r=mcs
These changes look good to Kathy and me, including the additions to import-translations.sh to generate the tor-browser-brand.ftl file. We will leave it to sysrqb or gk to decide if they want a separate ticket for that part.

comment:5 Changed 6 months ago by gk

Keywords: TorBrowserTeam202006R added; TorBrowserTeam202004R removed

Moving to TorBrowserTeam202006R to keep them on our radar.

Note: See TracTickets for help on using tickets.