Closed Bug 1453540 Opened 6 years ago Closed 6 years ago

Migrate the remaining DTD entries to FTL

Categories

(Firefox :: Settings UI, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 61
Tracking Status
firefox61 --- fixed

People

(Reporter: zbraniecki, Assigned: zbraniecki)

References

Details

Attachments

(3 files, 1 obsolete file)

With bug 1451992 landed we have now 13 DTD entities in browser/locales/en-US/chrome/browser/preferences.

Most of them stayed waiting for DOM Overlays 2 so with bug 1453480 we can migrate them as well!
Blocks: 1415730
Depends on: 1453480
Priority: -- → P3
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
This cleans up leftovers from all the work on Preferences.

With this patch there is just one DTD entity in Preferences left, in privacy.dtd this block[0]:

```
  <hbox align="center">
    <label id="historyModeLabel"
           control="historyMode"
           accesskey="&historyHeader2.pre.accesskey;">&historyHeader2.pre.label;
    </label>
    <!-- Please don't remove the wrapping hbox/vbox/box for these elements. It's used to properly compute the search tooltip position. -->
    <hbox>
      <menulist id="historyMode">
        <menupopup>
          <menuitem label="&historyHeader.remember.label;"
                    value="remember"
                    search-l10n-ids="history-remember-description"/>
          <menuitem label="&historyHeader.dontremember.label;"
                    value="dontremember"
                    search-l10n-ids="history-dontremember-description"/>
          <menuitem label="&historyHeader.custom.label;"
                    value="custom"
                    search-l10n-ids="
                      history-private-browsing-permanent.label,
                      history-remember-option.label,
                      history-remember-search-option.label,
                      history-clear-on-close-option.label,
                      history-clear-on-close-settings.label"/>
        </menupopup>
      </menulist>
    </hbox>
```

I've raised this issue in fluent.js repo - https://github.com/projectfluent/fluent.js/issues/171 

I'm going to verify with UX if this is the intended way, but we'll likely have to find a way to make it work in Fluent.

Except of that there's one more dtd string in `browser/locales/en-US/chrome/browser/preferences/security.dtd` but it's only used in aboutRights.xhtml outside of Preferences.

Finally, there's a bunch of DTD entries in Preferences that come from outside of Prefs. We'll have to migrate them to get rid of those strings in Preferences. I'll tackle that part in 62.
Stas, Pike, Flod - assuming we'll have to migrate this block. Do you have any recommendations on how to do it?
Flags: needinfo?(stas)
Flags: needinfo?(l10n)
Flags: needinfo?(francesco.lodolo)
Comment on attachment 8967907 [details]
Bug 1453540 - Migrate the remaining DTD entries to FTL.

https://reviewboard.mozilla.org/r/236604/#review242404

::: browser/locales/en-US/browser/preferences/preferences.ftl:489
(Diff revision 1)
>  
>  sync-signedout-account-signin =
>      .label = Sign In…
>      .accesskey = I
>  
> +sync-mobile-promo = Download Firefox for <img data-l10n-name="android-icon"/> <a data-l10n-name="android-link">Android</a> or <img data-l10n-name="ios-icon"/> <a data-l10n-name="ios-link">iOS</a> to sync with your mobile device.

Given the amount of data-l10n-name attributes in this string, it's worth a comment to tell localizers to not translate them.

We should also document it somewhere in our localizer-documentation.
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #3)
> Stas, Pike, Flod - assuming we'll have to migrate this block. Do you have
> any recommendations on how to do it?

I might be missing something, but this should be localized mapping strings to messages 1:1, without doing any transformation.

#171 is slightly incorrect, because English doesn't have any text in historyHeader.post.label (on GitHub there's a period).

All locales are empty in post, with the exception of vi and zam, which makes me think they're doing something really wrong, and we should just keep historyHeader2.pre.label as a stand-alone message.
https://transvision.mozfr.org/string/?entity=browser/chrome/browser/preferences/privacy.dtd:historyHeader.post.label&repo=gecko_strings

From Google Translate
vi: "Use custom settings for history"
zam: N/A (also, not shipping in Firefox)

At this point I'm just going to unapprove those two strings in Pontoon :-\
Flags: needinfo?(francesco.lodolo)
It's interesting that rtl locales don't use the post. I'll check how it looks in rtl but could you also check with Hebrew or Arabic pls?

Because if the layout reverses without post, then we're golden (or, lucky, because I'm afraid we'll have UI cases like this which we should develop an answer for)
Attached image RTL (ar, he)
I can't spot anything wrong with these locales.

We should add localization comments explaining how the string is used, and alternative ways to translate it that don't require having text after the select.
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #2)

Let's do that without postfix, if we can't drop the UX completely (which I'd prefer).

The alternative is that we do this with a deep inline xul element, and that'd probably require the refactor I suggested in https://github.com/projectfluent/fluent.js/pull/168#pullrequestreview-109650068, plus a strongly defined support for dom overlays in compare-locales (which is now bug 1454249).

Sounds like two hardly scoped blockers.

One can also argue that the no-postfix could be the desired UX, keeping the drop-down to share post-fix text even if it's the same. I guess that makes the drop down easier to read, though still hard with the leading text not being aligned with the dropped-down content.
Flags: needinfo?(l10n)
Attached image en-US History UI
Emanuela - can you verify if it's ok if this UX only uses pre, without post sentence?

It's a pretty unique UI in that it has a menulist in the middle of the sentence, except that functionally it seems that it's not in the middle.

All locales seems to be treating it basically as:

"Nightly will: <menulist>"

Is it ok if we remove the ability to add part of the sentence after <menulist>? It doesn't seem like any locale uses it.
Flags: needinfo?(emanuela)
Sorry for jumping in, but I think there are two different issues at hand:

1) Should we keep the "post" string? IMO that's not a question for UX, it's an internationalization problem, since English clearly doesn't need it. From the evidence, we can drop the post, and add a comment offering alternatives if they really need one. BTW, some locales went for a completely different route (e.g. "Firefox will:" -> "History settings:").

2) Is the current UI good? With a structure that:
- Uses a select to generate 3 full sentences from "Firefox will XXX", potentially with text after the select.
- Makes a set of options appear magically when "Use custom settings for history" is selected.

That's definitely a question for UX.

#2 sounds like a major change, beyond the scope of migrations. It would also invalidate existing strings, with the exception of checkbox labels used for custom settings.

Can someone confirm that the current UX, dropping the post, can be migrated without any update to Fluent?
> Can someone confirm that the current UX, dropping the post, can be migrated without any update to Fluent?

Yep, without "post" it's easy.
Forwarding Emanuelas needinfo to Jaqueline, as she last worked on that piece of UI.
Flags: needinfo?(emanuela) → needinfo?(jsavory)
For #1 This makes sense to me, currently the English version doesn't need anything after the dropdown so I would feel ok with removing the capability of adding anything afterwards. (Let me know if I'm misunderstanding the question). I also think it is good practice to not have a section of text after the dropdown if we can help it. 

For #2, I agree here as well, there is certainly some exploration that could happen here but I believe it should be apart of a bigger effort to improve the history section. There has been some discussion about working on this part of preferences, but for the short term I would leave it as is. When we do explore this area further, however, I will make sure that we look into this piece of it and see if users have any struggles here.
Flags: needinfo?(jsavory)
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #6)
> It's interesting that rtl locales don't use the post. I'll check how it
> looks in rtl but could you also check with Hebrew or Arabic pls?

This is the expected behavior in RTL. In the source, the pre label is defined before the menulist. In the rendered layout, the pre label is displayed first. In RTL, things that come first are displayed on the RHS.
We won't be able to migrate this UX to Fluent as-is. Nested elements are currently not supported in translations. Nested elements with data-l10n-ids (i.e. elements which require localization themselves) are not supported either. We can certainly add more features to Fluent but it would delay our ability to migrate this string. Perhaps it's OK to keep it in DTD for now? Especially given that the whole history section might see some improvements in the future.

If we don't want to wait, we could migrate this UX as separate strings. It's not ideal and it's not something I'd call canonical Fluent, but I think it could work here.

    ## History Settings
    history-settings-label = Firefox will
        .accesskey = w
    history-settings-remember = Remember history
    history-settings-never-remember = Never remember history
    history-settings-custom = Use custom settings for history

That said, the fact that the <menuitems> are capitalized in English and that some locales chose to put "History settings:" in the pre label (rather than "Firefox will") suggests to me that this shouldn't be a free-flowing sentence with a select box inside of it. Looking at about:preferences#search, there's the Default Search Engine section which looks like the following:

    Default Search Engine
    ---------------------
    Choose the default search engine to use in the address bar and search bar.

    [Google      ↓ ]

We could do something similar for the History settings:

    History
    -------
    Choose what Firefox should do with your browsing history.

    [Remember history     ↓ ]

This would require new strings altogether. We wouldn't need to migrate anything. Which, I guess, is also a valid answer to your question from comment 3 :)
Flags: needinfo?(stas)
Attachment #8967907 - Attachment is obsolete: true
Comment on attachment 8968668 [details]
Bug 1453540 - Migrate the remaining DTD entries to FTL.

https://reviewboard.mozilla.org/r/237342/#review243224

The r- is mostly because I want to double check comments and the migration fix. 

It would be great to test the recipe against en-US before the review though.

::: browser/locales/en-US/browser/preferences/preferences.ftl:223
(Diff revision 1)
>  
>  translate-web-pages =
>      .label = Translate web content
>      .accesskey = T
>  
> +translate-attribution = Translations by <img data-l10n-name="logo"/>

Let's add a comment, e.g.:

the <img> element is replaced by the logo of the provider used to provide machine translations for web pages.

::: browser/locales/en-US/browser/preferences/preferences.ftl:503
(Diff revision 1)
>  
>  sync-signedout-account-signin =
>      .label = Sign In…
>      .accesskey = I
>  
> +# This message contains two links and two icon images.

Let's add for safety: "They can be moved within the sentence as needed to adapt to your language, but should not be changed or translated."

::: browser/locales/en-US/browser/preferences/preferences.ftl:631
(Diff revision 1)
>  
>  ## Privacy Section - History
>  
>  history-header = History
>  
> +history-remember-label = { -brand-short-name } will

This one needs a long comment…

This label is followed, on the same line, by a dropdown list of options (Remember history, etc.). In English it visually creates a full sentence, e.g. "Firefox will Remember History". 

If this doesn't work for your language, you can translate this message:
* Moving the verb into each option.
* As a stand-alone message, for example "Firefox history settings:".

::: python/l10n/fluent_migrations/bug_1453540_preferences_remaining_dtd.py:83
(Diff revision 1)
> +                )
> +            ),
> +            FTL.Message(
> +                id=FTL.Identifier('history-remember-label'),
> +                value=COPY(
> +                    'browser/chrome/browser/preferences/privacy.dtd',

Missing a replace for &brandShortName;
Attachment #8968668 - Flags: review?(francesco.lodolo) → review-
Comment on attachment 8968668 [details]
Bug 1453540 - Migrate the remaining DTD entries to FTL.

https://reviewboard.mozilla.org/r/237342/#review243232

::: browser/locales/en-US/browser/preferences/preferences.ftl:636
(Diff revision 1)
> +history-remember-label = { -brand-short-name } will
> +    .accesskey = w
> +
> +history-remember-option-all =
> +    .label = Remember history
> +history-remember-option-none =

history-remember-option-never is probably a better ID
Comment on attachment 8968668 [details]
Bug 1453540 - Migrate the remaining DTD entries to FTL.

https://reviewboard.mozilla.org/r/237342/#review243294

Code looks fine to me.
Attachment #8968668 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8968668 [details]
Bug 1453540 - Migrate the remaining DTD entries to FTL.

https://reviewboard.mozilla.org/r/237342/#review243420

With the MESSAGE_REFERENCE import fixed in the recipe.
Attachment #8968668 - Flags: review?(francesco.lodolo) → review+
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c7c5fd50d175
Migrate the remaining DTD entries to FTL. r=flod,Gijs
https://hg.mozilla.org/mozilla-central/rev/c7c5fd50d175
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
You need to log in before you can comment on or make changes to this bug.