1631944 - Add Lockwise as a keyword for login and password preferences

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P2)

Description

[Matthew N. [:MattN]]

Users may use the brand name to find the settings but currently they get no matches.

Comment 1

[Matthew N. [:MattN]]

Attachment: Bug 1631944 - Add Lockwise as a keyword for login and password preferences. r=jaws

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/75b214b78ff4
Add Lockwise as a keyword for login and password preferences. r=jaws,fluent-reviewers

Comment 3

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/75b214b78ff4

Comment 4

[Francesco Lodolo [:flod]]

Commenting in the bug for the NI.

This is the second time this happens (patch created, reviewed and landed within a few hours, and in the middle of the night for EU). That's not great, to be honest, it defies the point of watching out for issues in l10n patches and catch them before they land.

https://phabricator.services.mozilla.com/D71830#2184769

Yes, can you do this in code, especially since we will want to uplift this to 76.

And here's the issue. I will push back uplifting this patch to 76, because we're past the deadline to accept sign-offs for beta. And, given it already landed in mozilla-central, I'm currently blocked in exposing any other string until this is clarified. The alternative is asking sheriffs to back this out, which is what I would normally do.

The comment implies that the search keyword should be defined in the code, but this is adding a new string with an attribute. All other search keywords are defined in code, why should this be different?

I'm fine in keeping the patch as is, and migrate the string, but it can't be uplifted. If that's the goal, this should be backed out ASAP.

Comment 5

[Matthew N. [:MattN]]

The plan isn't to uplift the patch that landed on m-c (unless you said that it was fine since there isn't really a new string to translate unless .searchkeywords = { -lockwise-brand-short-name } isn't sufficient for some locales). I didn't end up doing the change in code, hence the .ftl change. If we can't uplift the m-c patch then that's fine and we can make a separate change in code with no .ftl change for beta if we really want.

I also didn't originally plan to uplift this, that was Jared's idea in the review.

(In reply to Francesco Lodolo [:flod] from comment #4)

All other search keywords are defined in code, why should this be different?

It seemed better for l10n to have the keyword associated with the element and we didn't have Fluent when search keywords were first added. Doing this in also code seemed a bit risky because the string we want to use -lockwise-brand-short-name is defined in Fluent (whereas all other search keywords are in .properties) and the related code which sets the keywords are in synchronous code. Adding asynchronous code in that block seemed like it could cause regressions without further fixes/investigations.

Comment 6

[Francesco Lodolo [:flod]]

Thanks for the quick reply, looks like we can keep the patch as is then. I rely on your expertise to decide when it comes to code (e.g. around async code), that's not my area. Exposing the keyword makes definitely sense if it's different from the actual label.

The plan isn't to uplift the patch that landed on m-c (unless you said that it was fine since there isn't really a new string to translate unless .searchkeywords = { -lockwise-brand-short-name } isn't sufficient for some locales).

That's why I said that landing l10n patches so quickly is an issue, our systems are far from straightforward and only a couple of people fully understand them.

This patch would have been upliftable a week ago. Now, sign-offs for Beta are closed, which means we can't take more l10n updates and ship them in 76. This would result in every language falling back to English also for the section header, unless we make exceptions and update sign-offs past the expected deadline.

Comment 7

[Matthew N. [:MattN]]

(In reply to Francesco Lodolo [:flod] from comment #6)

This patch would have been upliftable a week ago. Now, sign-offs for Beta are closed, which means we can't take more l10n updates and ship them in 76. This would result in every language falling back to English also for the section header, unless we make exceptions and update sign-offs past the expected deadline.

OK, Jared and I weren't sure if this case would be fine for an exception (to run the migration in beta) given that the new string isn't user-facing and is unlikely to be changed in most locales. We weren't going to try uplift this without asking you anyways.

Comment 8

[Cosmin Muntean [:cmuntean], Ecosystem QA]

I have verified this issue and the "Logins and Passwords" section is correctly displayed in "about:preferences" page if searching after "Lockwise" keyword. I have verified this using the latest Nightly 77.0a1 "en-US", "de", "it" and "ru" builds (Build ID: 20200430082621) on Windows 10 x64, Mac 10.14.6 and Ubuntu 18.04 x64.