1469339 - Expose edit menu strings through Fluent

Status: RESOLVED FIXED

(Firefox :: Menus, enhancement, P3)

Description

[Brian Grinstead [:bgrins]]

We'd like to work on opening an 'edit' context menu from an HTML document, and since the menu will be created from JS it'd be better to access the strings from Fluent than from the dtd: https://searchfox.org/mozilla-central/rev/285da1fd7dcf67448b9175741fa330158edcff73/browser/locales/en-US/chrome/browser/browser.dtd#317-339.

Comment 1

[Brian Grinstead [:bgrins]]

Alternatively, this file might be be a target for conversion: https://searchfox.org/mozilla-central/source/toolkit/locales/en-US/chrome/global/textcontext.dtd. The drawback is there's some XBL anon content usage for input-box, although we are going to convert that to a Custom Element in Bug 1470910.

We'd want to expose it in toolkit/ which doesn't yet have any fluent files so see Bug 1411707 for how that needs to get registered in browser glue / build system.

Comment 2

[Brian Grinstead [:bgrins]]

Attachment: Bug 1469339 - Put strings needed for the edit context menu into ftl at toolkit/locales;

This also registers the locale through the build system and browser glue since
it's the first ftl file in toolkit/.

Review commit: https://reviewboard.mozilla.org/r/255848/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/255848/

Comment 3

[Brian Grinstead [:bgrins]]

:flod, please see the patch in Bug 1456852 for an example of how this would be used (specifically https://reviewboard.mozilla.org/r/255676/diff/3#index_header). I'd prefer to not do migration of existing consumers here (see Comment 1 and that we'd be blocked on Bug 1455649) - but should be able to ultimately migrate the various dtd and properties consumers to use the ftl version: https://searchfox.org/mozilla-central/search?q=select+all&path=%7Bproperties%2Cdtd%7D.

Comment 4

[Brian Grinstead [:bgrins]]

Comment on attachment 8990803 [details]
Bug 1469339 - Put strings needed for the edit context menu into ftl at toolkit/locales;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/255848/diff/1-2/

Comment 5

[Axel Hecht [:Pike]]

Migration of strings can happen without the migration of the calling code.

I'm not sure if we should include accesskeys in generic strings, or if we should do terms generically, and do per-usecase accesskeys (by using term references in the target ftl files).

As food for thought for flod.

Comment 6

[Brian Grinstead [:bgrins]]

Updated the patch showing how this would be used: https://reviewboard.mozilla.org/r/255674/diff/3#index_header

Comment 7

[Francesco Lodolo [:flod]]

Comment on attachment 8990803 [details]
Bug 1469339 - Put strings needed for the edit context menu into ftl at toolkit/locales;

https://reviewboard.mozilla.org/r/255848/#review263166

Interesting, I did comment last week in MozReview, but looks like it failed to publish in the bug :-\

Please make sure to add all missing equal sign after the message IDs., that's the only issue I spotted.

Also chatted with Axel about his comment, and while it might be interesting to have accesskeys changing based on context, it also feel like over-engineering. The current patch uses the same approach we currently use with DTD, and it's been working so far.

No need to write a migration for these strings, easier and safer to just retranslate them, given we have a few places in browser and toolkit with these commands.

::: toolkit/locales/en-US/toolkit/main-window/editmenu.ftl:4
(Diff revision 2)
> +### This file contains the entities needed for the 'edit' menu
> +### It's currently only used for the Browser Console
> +
> +editmenu-undo

Note: you need an equal sign even if the string doesn't have a value, see for example
https://hg.mozilla.org/l10n/gecko-strings/file/default/browser/browser/preferences/applicationManager.ftl#l9

Comment 8

[Brian Grinstead [:bgrins]]

Comment on attachment 8990803 [details]
Bug 1469339 - Put strings needed for the edit context menu into ftl at toolkit/locales;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/255848/diff/2-3/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d104946698b2
Put strings needed for the edit context menu into ftl at toolkit/locales;r=flod

Comment 10

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/d104946698b2