Closed Bug 1608022 Opened 3 months ago Closed 2 months ago

Migrate browser-sets.inc to Fluent

Categories

(Firefox :: General, task, P1)

task

Tracking

()

RESOLVED FIXED
Firefox 74
Tracking Status
firefox74 --- fixed

People

(Reporter: zbraniecki, Assigned: zbraniecki)

References

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

Details

Attachments

(3 files)

After bug 1605467, we still have a number of DTD callsites in browser-sets.inc. Let's migrate it to Fluent.

Blocks: 1579477
Assignee: nobody → gandalf
Status: NEW → ASSIGNED

Henrik - several of those commandkeys are used by puppeteer and other bits of firefox-ui testing. Can you advise on how can we move forward with this? I'd like to avoid breaking testing, but I'm also worried about delaying this patch as it'll bitrot likely fairly quickly.

Flags: needinfo?(hskupin)

(In reply to Zibi Braniecki [:zbraniecki][:gandalf] from comment #3)

Henrik - several of those commandkeys are used by puppeteer and other bits of firefox-ui testing. Can you advise on how can we move forward with this? I'd like to avoid breaking testing, but I'm also worried about delaying this patch as it'll bitrot likely fairly quickly.

I'm kinda busy right now, but maybe you could send me a list of those entries? It would help me a lot for determining what needs to be done. Thanks.

Flags: needinfo?(hskupin) → needinfo?(gandalf)

I'm kinda busy right now, but maybe you could send me a list of those entries? It would help me a lot for determining what needs to be done.

  • tabCmd.commandkey and closeCmd.key used in testing/marionette/puppeteer/firefox/firefox_puppeteer/ui/browser/tabbar.py
  • pageInfoCmd.commandkey and closeCmd.key used in testing/marionette/puppeteer/firefox/firefox_puppeteer/ui/browser/window.py
  • addons.commandkey and searchFocus.commandkey used in testing/firefox-ui/tests/functional/keyboard_shortcuts/test_browser_window.py
  • newNavigatorCmd.key and privateBrowsingCmd.commandkey used in testing/marionette/puppeteer/firefox/firefox_puppeteer/ui/browser/window.py
  • openCmd.commandkey and reloadCmd.commandkey used in testing/marionette/puppeteer/firefox/firefox_puppeteer/ui/browser/toolbars.py
Flags: needinfo?(gandalf) → needinfo?(hskupin)

Will there be a replacement for those command keys? I assume so. And which API can be used to access those via data-l10n-id?

Will there be a replacement for those command keys?

Yup. All of those get migrated to browser/locales/en-US/browser/browserSets.ftl .

And which API can be used to access those via data-l10n-id?

Not sure if I understand? Fluent provides an extensive JS, Python and Rust API, but if you're asking how to retrieve the value of those at runtime, from the JS context, I'd expect it to be sth like:

async function getKeyValue(elemId) {
    await document.l10n.ready;
    let elem = document.getElementById(elemId);
    return elem.getAttriute("key");
}

I don't know how this testing code uses the DTD entities.

So what we basically need is a way to retrieve the key combination for eg a shortcut the user can press. This is done by using DTD entities right now to ensure that tests can work across different locales. Right now we do not run any localized builds in CI anymore since the update tests have been removed from our test matrix. Further I would like to get rid of the firefox-puppeteer Python package (see bug 1573383).

Based on that I would say we get rid of all the l10n specific code in firefox-puppeteer, and hard-code the keys in the API modules. That means there won't be any trouble for you in migrating the remaining DTD entities to Fluent. For that work please file a new bug under Marionette blocking the above mentioned one.

I currently don't have the time to work on that, but would be happy to mentor anyone who is interested in to fix it, and I will also review any patches.

Flags: needinfo?(hskupin)

Actually we also have a localization library in Marionette itself, which is questionable what to do with it. But it can also be discussed on the to file bug.

Depends on: 1610014

Thanks, filed bug 1610014 and set it as blocking this one.

Depends on: 1611128
Priority: -- → P1
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d538041cc083
Migrate the bookmark this page and edit bookmark strings to Fluent. r=fluent-reviewers,Gijs,flod
https://hg.mozilla.org/integration/autoland/rev/853e5ec59184
Migrate browser-sets to Fluent. r=fluent-reviewers,Gijs,flod
https://hg.mozilla.org/integration/autoland/rev/b7097a9f6a16
Remove obsolete toolbox.dtd. r=bgrins
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/15431d572cb8
Migrate the bookmark this page and edit bookmark strings to Fluent. r=fluent-reviewers,Gijs,flod
https://hg.mozilla.org/integration/autoland/rev/0096054a3156
Migrate browser-sets to Fluent. r=fluent-reviewers,Gijs,flod
https://hg.mozilla.org/integration/autoland/rev/8110627f166c
Remove obsolete toolbox.dtd. r=bgrins

Interesting.

So, the reason this errored is because we switched commandkey from f to F and now EventUtils.synthesizeAndWaitKey needs to receive F instead of f.

The reason I didn't notice it is because the test is intermittently flaky and had an intermittent bug filed against it.

Flags: needinfo?(gandalf)
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 74
Regressions: 1612612
Regressions: 1612662
See Also: → 1612679
Regressions: 1612679
See Also: 1612679
Regressions: 1612723
No longer regressions: 1612612
Regressions: 1613345
Regressions: 1614107
No longer regressions: 1614107
Regressions: 1612653
You need to log in before you can comment on or make changes to this bug.