1581185 - Migrate pageActionContextMenu in browser.xhtml to Fluent

Status: RESOLVED FIXED

(Firefox :: General, task, P1)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

As the next step, we want to validate the findings from https://bugzilla.mozilla.org/show_bug.cgi?id=1580978#c4 by migrating https://searchfox.org/mozilla-central/source/browser/base/content/browser.xhtml#568-586

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1581185 - Fix relative paths in migration script generation. r?bgrins

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1581185 - Migrate pageActionContextMenu in browser.xhtml to Fluent.

Depends on D45880

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

This was much easier than https://bugzilla.mozilla.org/show_bug.cgi?id=1580978#c4

I updated the script to fix two issues listed there:

• Remove leading "./" from the paths in migration script
• Append to FTL file if one already exists instead of creating a new one

My steps were:

• Extract the piece to a new file "./browser_pageAction.xhtml"

• Launch python python/l10n/convert_xul_to_fluent/convert.py 1581185 ./browser_pageAction.xhtml ./browser/locales/en-US/browser/browser.ftl ./ ./browser/locales/en-US/chrome/browser/browser.dtd "Migrate the pageActionContextMenu to FTL."

• Paste the resulting ./browser_pageAction.xhtml into browser.xhtml in place of the old chunk

• Manually remove the strings from DTD

Comment 4

[Brian Grinstead [:bgrins]]

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

This was much easier than https://bugzilla.mozilla.org/show_bug.cgi?id=1580978#c4

I updated the script to fix two issues listed there:

• Remove leading "./" from the paths in migration script
• Append to FTL file if one already exists instead of creating a new one

My steps were:

• Extract the piece to a new file "./browser_pageAction.xhtml"

• Launch python python/l10n/convert_xul_to_fluent/convert.py 1581185 ./browser_pageAction.xhtml ./browser/locales/en-US/browser/browser.ftl ./ ./browser/locales/en-US/chrome/browser/browser.dtd "Migrate the pageActionContextMenu to FTL."

• Paste the resulting ./browser_pageAction.xhtml into browser.xhtml in place of the old chunk

• Manually remove the strings from DTD

Thanks, that's much better!

One thing I did in the previous patch was to manually fix up the markup. For instance here we went from:

      <menuitem class="pageActionContextMenuItem extensionPinned"
                label="&pageAction.removeFromUrlbar.label;"
                oncommand="BrowserPageActions.togglePinningForContextAction();"/>

To:

      <menuitem class="pageActionContextMenuItem extensionPinned"
                oncommand="BrowserPageActions.togglePinningForContextAction();" data-l10n-id="page-action-remove-from-urlbar"/>

In some cases the new form is probably fine (if the attributes preceeding it were short enough), but in general we try to put each one on a new line, aligned with the previous one. Does that seem fixable within the current script? Removing the extra manual step that would be great.

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

Does that seem fixable within the current script? Removing the extra manual step that would be great.

Yep! Fixed!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5bb71874a9f3
Fix relative paths in migration script generation. r=bgrins
https://hg.mozilla.org/integration/autoland/rev/86f77b5cc705
Migrate pageActionContextMenu in browser.xhtml to Fluent. r=fluent-reviewers,bgrins,flod

Comment 7

[Andreea Pavel [:apavel]]

Backed out for linting failure at xul.py

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedJob=267276232&resultStatus=testfailed%2Cbusted%2Cexception&revision=86f77b5cc70529fa17047d956c5d5b84d22d912c

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=267276232&repo=autoland&lineNumber=289

Backout: https://hg.mozilla.org/integration/autoland/rev/00e37dda872ba72c8f8b9509690fb071f264df5e

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d336ff588e4b
Fix relative paths in migration script generation. r=bgrins
https://hg.mozilla.org/integration/autoland/rev/f8223f3c02b0
Migrate pageActionContextMenu in browser.xhtml to Fluent. r=fluent-reviewers,bgrins,flod

Comment 9

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/d336ff588e4b
https://hg.mozilla.org/mozilla-central/rev/f8223f3c02b0