Skip to content

Decouple message attributes from DOM attributes #437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
stasm opened this issue Dec 3, 2019 · 6 comments
Open

Decouple message attributes from DOM attributes #437

stasm opened this issue Dec 3, 2019 · 6 comments

Comments

@stasm
Copy link
Contributor

stasm commented Dec 3, 2019

The current design of DOM and React bindings tends to create a tight coupling in the long term between Fluent messages and the UI widgets they localize. The attributes found on compound messages are used to localize DOM attributes and React props with the same names.

The tight coupling contributes to the accruing of technical debt. It discourages from making changes to the source element to avoid breaking its translations or the need to migrate them to the new structure.

To remedy this situation, I wonder if it would be a good idea to change the bindings to expose in the source a way to map DOM attributes to Fluent message attributes (or the other way around).

For example, the following message could then be used to localize a number of different elements, either at the same time, or as a function of time.

toolbar-back =
    .label = Back
    .accesskey = B
    .tooltip = Go back one page

Then, using @fluent/dom:

<menuitem data-l10n-id="navbar-menu-back"
    data-l10n-attrs="aria-label:label accesskey tooltiptext:tooltip">
<description data-l10n-id="navbar-tooltip-back"
    data-l10n-attrs="value:tooltip">
<toolbarbutton data-l10n-id="toolbar-button-back"
    data-l10n-attrs="label">

Or using @fluent/react:

<Localized id="navbar-menu-back" attrs={{
    ariaLabel: "label",
    accesskey: "accesskey", // or true, since the names match and because
                            // we already use true in attrs right now
    tooltiptext: "tooltip"
}}>...</Localized>

Compound messages adapted to this usage pattern would likely only use attributes because the message value would not be "mappable"

# Can't map the value to a source DOM attribute
toolbar-back = Back
    .accesskey = B
# Can map "label" to a source DOM attribute
toolbar-back =
    .label = Back
    .accesskey = B
@gijsk
Copy link

gijsk commented Dec 3, 2019

It'd seem simpler to add support for (1) self-attribute-references (so I can express that foo.label should just be the same as foo.aria-label) and (2) having the consuming element be able to either safelist or blocklist attributes. Right now data-l10n-attrs is additive on top of a default; we'd probably need data-l10n-blockattrs or similar (or syntax in data-l10n-attrs, e.g. data-l10n-attrs="-label" meaning don't give me a label attribute). Attribute mapping becomes... pretty messy.

All that said, I'm confused why we're having this conversation at all, for a few reasons:

  1. if @zbraniecki 's patch works already, with a shared thing from which we source values for 2 or more elements, I think that's sufficient? We've definitely already used this approach in Firefox (e.g. duplicated pane titles), and the DRY-er approach helps with ensuring messaging is consistent and updating 1 value implies updating all of them. We've not been super aggressive about it, see e.g. translations mentioning preferences but if anything I'd be more aggressive than we have been about using terms or other "shared" strings for common concepts, to make things like https://bugzilla.mozilla.org/show_bug.cgi?id=1399502 less nightmarish.
  2. access keys needing to be context-unique means that in some cases, the same label may be shared without necessarily using the same accesskey. The approach outlined here does not allow for that; we'd have to fall back to duplicating the label or using zibi's extant approach anyway.
  3. localizers may wish to use different strings even where the English uses the same, for reasons of grammar, context, or style. The suggestions here push control more towards the DOM and away from localizers, because the DOM will use one ID in multiple places. I don't think that's the right direction.
  4. we already have more FTL strings than DTD ones at this point, and this would be a pretty wide-ranging change that'd then prompt a lot of rewriting / re-educating. I don't think all of fluent needs to be set in stone at this point, but the choice for having the messages be DOM-shaped was made ages ago, people pointed out this problem at the time and people decided not to use more DTD-type approaches where the DOM explicitly indicates which things go in which attributes. I think it's too late to go back on that. This is the bed that we made. It's not a particularly bad bed, but if we keep trying to find the perfect bed we will never sleep (to stretch the idiom a bit...).

@flodolo
Copy link

flodolo commented Dec 3, 2019

1. to make things like https://bugzilla.mozilla.org/show_bug.cgi?id=1399502 less nightmarish.

OT, but I feel it's necessary to clear this up. That will be always a nightmare. If you have one term and 100 messages referencing that term, and change the term from "Preferences" to "Settings", you have two options:
a) Change the term ID, and change all 100 IDs referencing it, because you just broke all of them.
b) Change the term without a new ID, and hope that locales will catch up at some point (they won't notice it). Also, the new term will ship in all versions because of cross-channel.

Option b) is probably the one I'd go with, although not ideal. And we might have to deal with that, if they ever decide to change the brand name from Firefox.

@stasm
Copy link
Contributor Author

stasm commented Dec 3, 2019

Thanks for sharing your thoughts, @gijsk. I agree that this solution might put too much control in the hands of developers and lead to a single message being reused in too many places. The point that you made about accesskeys and their being context-unique is also a very good one. Another reason to discourage for reusing translations in more than one place.

I filed this issue prompted by the discussion in bug 1600528 and I realize that I didn't make it clear enough that I had been thinking about this problem for much longer than just this morning, and also not in the context of translations shared by more than one UI widget, but instead in the context of change happening over time.

Unfortunately, I think change over time will always be messy to some extent:

  1. If you make the change in the source without any changes to localizations, the mess happens on the side of localizers. They now need to update their translations to match the new message shapes.
  2. If the bindings expose a way to map attributes to attributes, then the mess happens on the side of developers, as you pointed out.
  3. Or, you do 1. but also have someone write a Fluent-to-Fluent migration recipe, someone else reviews and tests it, and someone else temporarily disables Pontoon's sync so that some yet another someone can run the migration and push it. That's also mess :)

I don't mean to revisit the entire design on Fluent's DOM bindings here, but I do think it valuable to recognize its limitations, try to find remedies to them, or maybe document good practices which somehow alleviate them. And I also think beyond Firefox, where it might be harder to run migrations as we do.

@gijsk
Copy link

gijsk commented Dec 3, 2019

Or, you do 1. but also have someone write a Fluent-to-Fluent migration recipe, someone else reviews and tests it, and someone else temporarily disables Pontoon's sync so that some yet another someone can run the migration and push it. That's also mess :)

I think we should investigate making this less of a mess, like (and I spent maybe 2 minutes thinking about this, so it's probably a terrible idea, but you get my drift): having a staging for a migration where someone checks that it produces reasonable results but doesn't commit, and then queueing the migration with some system (I'm not super familiar with our l10n infra, so maybe this should be pontoon but maybe something else) and having that system run all queued migrations automatically at a "quiet" time, automatically turning pontoon sync off and back on. :-)

@stasm
Copy link
Contributor Author

stasm commented Dec 3, 2019

Yeah, totally! And I think mach fluent-migration-test is a great start, thanks to @Pike and @flodolo. Migrations are in fact a very good solution to the tight coupling problem :)

@zbraniecki
Copy link
Collaborator

zbraniecki commented Dec 3, 2019

OT, but I feel it's necessary to clear this up. That will be always a nightmare.

@flodolo - I disagree with you. We do have a solution to that - projectfluent/fluent#141

It's an example of a scenario where the old string does work (and should be preferred over fallback locale), but we need to notify the localizers that they may want to update the translations (some will, some will just mark their translation as up to date).
The semantic comments with string versioning is addressing precisely this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants