Closed Bug 1761893 Opened 3 years ago Closed 2 years ago

Don't treat aria-valuetext as a localisable attribute

Categories

(Core :: Internationalization, defect)

defect

Tracking

()

RESOLVED FIXED
105 Branch
Tracking Status
firefox105 --- fixed

People

(Reporter: Jamie, Assigned: ayeddi)

References

Details

Attachments

(1 file)

While aria-valuetext is indeed localised, it is much more dynamic than other localisable attributes like title, aria-label, etc. For example, consider the month spinner for the date picker (bug 1676068), which is what inspired the filing of this bug. Currently, to do this properly with Fluent, we'd have to add a bunch of new duplicate strings, resulting in something like this:

month-jan =
  .aria-label = Month
  .aria-valuetext = January
month-feb =
  .aria-label = Month
  .aria-valuetext = February
...

We'd then switch data-l10n-id every time the value of the spinner changes. This seems really wasteful and ugly.

Having aria-valuetext localisable like this would be akin to having the value attribute on <input type="text"> treated as localisable, which isn't the case (value is only considered localisable on button inputs where the value is really more like a label). Finally, there is no use of aria-valuetext in Fluent files anywhere in our code.

I think both instances of aria-valuetext should be removed from IsAttrNameLocalizable in dom/l10n/L10nOverlays.cpp.

Erik, what do you think?

Severity: -- → S3
Flags: needinfo?(enordin)

I'm generally not opposed to the repetition.

According to the Fluent Good Practices For Developers Guide we should prefer WET (write everything twice) over DRY (don't repeat yourself) when writing localization strings, even if it feels counterintuitive compared to typical best practice for writing code in general.

That being said, if there is a reasonable argument to not localize this content, then that's another matter entirely.

I don't know the right answer to the latter question. Just because something hasn't been localized historically doesn't necessarily mean that we shouldn't start to localize it now if we can.

What do you all think?

Flags: needinfo?(enordin)

Aria-valuetext attribute is expected to be mutated with each change of user's input, thus it does not have to be listed as localizable attribute.

Also, this approach removes pre-selected value from the DOM on init of a component like a spinbutton, preventing users of assistive technology from accessing human-readable value that was pre-selected and is present on-screen.

Assignee: nobody → ayeddi
Status: NEW → ASSIGNED

(In reply to Erik Nordin [:nordzilla] from comment #2)

What do you all think?

Per the WAI ARIA specification:

A few properties, such as aria-activedescendant, aria-valuenow, and aria-valuetext are expected to change often.

And aria-valuetext is specifically created for a fixed range of values, for example, on a range widget such as a slider or progress bar which gives a very limited set of use cases. Those use cases likely will have pre-selected measurements, like %. While there could be examples of custom set of values that would need l10n, like small, medium, and large for sizing, this still could be manually localized by Fluent's attributes.

The implementation right now silently removes aria-valuetext from the DOM, no errors thrown. At the same time, the example that I am working on now (which could be the lucky 13th instance), has a set of 12 pre-localized values (months) that is just being reused across the Datepicker and MonthYear Spinner. Fluent also does allow a non-localized string to be assigned to the aria-valuetext when the widget is mutated, so the rule here is not strictly enforced.

I submitted a patch, just in case :)

I'm not necessarily suggesting that aria-valuetext isn't localised from the user perspective. IN the case of the month chooser, it certainly is. However, I don't think it makes sense for it to be localised using the same l10n id as the label, etc. Things like label/description/title are associated with "what" the control is. In contrast, aria-valuetext is part of the "state" of the control.

Imagine a <select> with 20 <option>s. With Fluent, the <select> would have one l10n id and the <option>s would all have separate l10n ids. That's effectively how we should see aria-valuetext, except it just doesn't have separate DOM nodes for each possible value. Unfortunately, AFAIK, there's no way with Fluent to specify that a given attribute should use a different l10n id to the rest of the element.

The other issue here is that the month names come from Services.intl.DisplayNames. If we duplicate them in our own Fluent strings, they're likely to not work well if there are locales that use a different calendar.

That said, if you feel strongly about this, another alternative here could be to remove data-l10n-id and instead use document.l10n.setAttributes, passing monthName as an argument. So, in Fluent, we'd do something like this:

month-chooser =
  .aria-label = Month
  .aria-valuetext = { $monthName }

In JS, every time we update the value, we'd do:

document.l10n.setAttributes(monthChooser, "month-chooser", { monthName: monthStrings[monthNum] });

That kinda feels uglier to me, but I could live with it.

Thoughts?

Flags: needinfo?(enordin)

Unfortunately, AFAIK, there's no way with Fluent to specify that a given attribute should use a different l10n id to the rest of the element.

You are correct, this is one part of functionality that was proposed for Fluent Revision 3.

In JS, every time we update the value, we'd do:

document.l10n.setAttributes(monthChooser, "month-chooser", { monthName: monthStrings[monthNum] });

That kinda feels uglier to me, but I could live with it.

Thoughts?

Would the names contained within monthStrings be localized? If so, then that solution seems workable to me, given the constraints.

Flags: needinfo?(enordin) → needinfo?(jteh)

(In reply to Erik Nordin [:nordzilla] from comment #6)

Would the names contained within monthStrings be localized?

Yes.

If so, then that solution seems workable to me, given the constraints.

The question is: is it better than the solution of not treating aria-valuetext as localisable; i.e. the attached patch? We're likely to end up having to do this every time aria-valuetext is used. In other usage, aria-valuetext might be a formatted version of some digits, which again will have already been formatted for visual display, so we just want to copy that. If we always have to have a Fluent string with something like:
.aria-valuetext = {$someLocalizedArg}
and then:
document.l10n.setAttributes(elem, "someFluentId", { someFormattedArg: formattedVal });
is that really cleaner than just setting aria-valuetext manually? I think the chances of us having a Fluent string for .aria-valuetext that isn't just {$someFormattedArg} are pretty much 0.

If you still think it's cleaner after all that, we will follow that pattern and close this. I'm not convinced it is, but there's a reason you're responsible for i18n things and I'm not. :)

Flags: needinfo?(jteh) → needinfo?(enordin)

I do see your point. Having a value that is only an argument doesn't feel great.

I would be curious if eemeli has any thoughts here, particularly with regard to which style would be easiest to transition to a future message format standard.

Flags: needinfo?(enordin) → needinfo?(earo)

With the proviso that I was not previously familiar with aria-valuetext specifically and have only briefly reviewed its suggested usage, I think it does make sense as a localisable attribute.

Specifically, as mentioned on MDN,

If the numeric value is meaningful, such as a spinner with aria-valuenow="3" for how many pizza slices you want to order, aria-valuetext is not needed.

So this is meant for usage where the value is a message, or a specifically formatted number. These are some examples that I could find online:

  • MDN
    • aria-valuetext="8% (34 minutes) remaining"
    • aria-valuetext="Sunday"
  • w3.org wai-aria-practices
    • A string value that provides a user-friendly name for the current value of the slider.
    • For the temperature slider, is the value of the slider appended with the text " degrees".
    • For fan and heat/cool, is the text representation of the current state, e.g., "Off" or "Heat".

With the month example, this is how I'd imagine this to work "properly" in Fluent:

month-chooser =
  .aria-label = Month
  .aria-valuetext = { DATETIME($date, month:"long") }

For locales that may need to change that due to a different calendrical preference or for cases where the input range value has some other custom meaning (like "Off"/"Cool"/"Heat"), a Fluent selector on the input value should provide for the appropriate result.

Flags: needinfo?(earo)

After some more review and thought triggered by hitting this issue myself when working on bug 1734221, I'm starting to think that the root issue here is not whether aria-valuetext is localisable or not, it's that the original solution for bug 922577 has resulted in our current "feature" of resetting all localisable attributes when a node is localised. A particularly code-smell-y aspect of this is that the node's value is not reset even when it's void (though there are good reasons for this current behaviour). Changing these aspects of DOM Localization in general would be breaking changes, so I'd very much prefer to figure out a more specific solution.

In general, DOM Localization does not really play nice when anyone else wants to also control the value of a localisable attribute. Furthermore, DOM Localization is fully async, and guides usage away from directly relying on any localised value. This counts as a feature, though it does make certain usage patterns (somewhat intentionally) more difficult.

In some cases it might be possible to circumvent issues with generally localisable attributes by adding some explicit or implicit way for DOM Localization to ignore them, but this would only make the corner-case smaller and more surprising. With our current API, it would be better to acknowledge that DOM Localization is not suitable for any such mixed use, in particular when the string value of a possibly-localised attribute or contents of an element may be used by other code.

However, aria-valuetext is indeed a bit special, as it's meant to directly reflect the node's value, and as mentioned we're not treating the value and attributes the same when resetting them. So given its very limited usage, the quirky DOM Localization-specific results of considering an attribute as "localisable", and the nature of aria-valuetext, I'd like to update my position above to its opposite from what I stated previously, and recommend that despite aria-valuetext being in theory "localisable", we should not consider it so.

This would still allow for the attribute to be localised by including an explicit data-l10n-attrs="aria-valuetext". While this bug has been open, one such usage has been added to m-c, via bug 1779625. That'll need to be patched before landing the patch already attached to this bug.

Jamie, Erik, do you concur?

Flags: needinfo?(jteh)
Flags: needinfo?(enordin)

Thank you for giving so much thought and a detailed write-up to this. I certainly don't want to introduce breaking changes, but everything you described seems reasonable to me. That being said, I still don't have a lot of hands-on experience with aria-valuetext so I am still curious to hear Jamie's opinion here.

Flags: needinfo?(enordin)

I totally concur with aria-valuetext not being treated as localisable (by default) as outlined in comment 7. I do understand some of the arguments made in comment 9. However, ultimately, I think puristic correctness here gets in the way of pragmatism and how aria-valuetext will really be used. If we have to jump through hoops almost every time to use aria-valuetext correctly, that's not ideal. Also, the great thing about this solution (making it not localisable by default) is that it's incredibly simple for anyone who does want it to be localisable to make it so.

(In reply to Eemeli Aro [:eemeli] from comment #10)

one such usage has been added to m-c, via bug 1779625. That'll need to be patched before landing the patch already attached to this bug.

Anna, would you mind updating this patch here to set data-l10n-attrs="aria-valuetext" on the Fluent aria-valuetext usage introduced in bug 1779625? Let me know if you need any further details.

Flags: needinfo?(jteh) → needinfo?(ayeddi)
Attachment #9269916 - Attachment description: Bug 1761893 - Remove aria-valuetext from a list of localisable attributes. r=Gijs,smaug,Jamie!,nordzilla → Bug 1761893 - Remove aria-valuetext from a list of localisable attributes. r=eemeli,smaug,Jamie!,nordzilla
Blocks: 1734221
Attachment #9269916 - Attachment description: Bug 1761893 - Remove aria-valuetext from a list of localisable attributes. r=eemeli,smaug,Jamie!,nordzilla → Bug 1761893 - Remove aria-valuetext from a list of localisable attributes. r=eemeli!,smaug,Jamie!,nordzilla
Pushed by ayeddi@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4d337fb37888 Remove aria-valuetext from a list of localisable attributes. r=smaug,nordzilla,Jamie,eemeli

The patch is tested, approved and landed.

Thank you all for your guidance!

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Flags: needinfo?(ayeddi)
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 105 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: