Open Bug 1576067 Opened 5 years ago Updated 2 years ago

Investigate getting rid of dynamic-shortcut-tooltip/UpdateDynamicShortcutTooltipText

Categories

(Firefox :: Toolbars and Customization, task, P3)

task

Tracking

()

People

(Reporter: Jamie, Unassigned)

References

Details

(Keywords: access)

We have a bunch of controls with tooltip="dynamic-shortcut-tooltip". The text for the tooltip is generated in the UpdateDynamicShortcutTooltipText function when the user hovers the mouse over the control and thus triggers the tooltip. This is an accessibility problem because:

  1. The tooltip text isn't available until the user hovers the mouse, and some users (screen reader users, keyboard only users) don't use the mouse.
  2. Even if they do hover the mouse, the information only appears in a separate tooltip node. That is, the information isn't exposed on the accessible for the control itself. The association is purely visual: the tooltip visually appears near the control.

In bug 1480415, this resulted in a button without an a11y label because the button had no label. A11y would normally fall back to tooltiptext, but it can't do that for the reasons described above.

For the remaining cases, it's not so critical because the buttons have labels. The user still misses out on learning about the shortcut (and any other detail in the tooltip), but at least the button isn't unlabelled. It's still something we should fix.

I can think of two possible solutions:

  1. Bug 1480415 comment 11 suggested it might be possible to get rid of this with Fluent:

    With Fluent we should be able to remove the UpdateDynamicShortcutTooltipText function since we can build out those tooltips inside of the .ftl files and can most likely remove usage of the dynamic-shortcut-tooltip altogether.
    Looking at UpdateDynamicShortcutTooltipText and the related strings, there are none that are context-specific and would need to be changed depending on external factors such as the target of the item or how many

  2. Failing that, we could keep this "dynamic". Rather than using the tooltip attribute, we would update tooltiptext on mouseover or focus. This isn't perfect because the accessible still wouldn't have a description if it wasn't focused or hovered, but it's not horribly unreasonable to have a non-mouse user focus something in order to access the tooltip.

I think option 1 is likely our best bet, but I don't think anyone from the frontend team has cycles to work on this atm, so this'll go into backlog...

Priority: -- → P3

So in bug 1592719 I'm not sure if I realized when reviewing at the time (even so, perfect is the enemy of good, so it's good that it landed), but we could potentially have fixed that bug by fixing this bug.

Fluent should definitely support this, though working out exactly how may be a bit hairy. For context, this is the code at

https://searchfox.org/mozilla-central/rev/ce02064d8afc8673cef83c92896ee873bd35e7ae/browser/base/content/browser.js#7003-7075

We can perhaps prototype this with the new tab button, just to have an example to work with...

We'd want to end up in a situation where we:

and then FTL that looked more or less like this (untested, probably wants splitting up to have an ftl equivalent of platformKeys.properties and one that's actually about browser keys):

shortcut-vk-meta =
    { PLATFORM() ->
        [macos] ⌘
       *[other] Meta
    }

shortcut-vk-ctrl = 
    { PLATFORM() ->
        [macos] ⌃
       *[other] Ctrl
    }

shortcut-mod-accel-key =
    { PLATFORM() ->
        [macos] { shortcut-vk-meta }
       *[other] { shortcut-vk-ctrl }
    }
shortcut-mod-separator = 
    { PLATFORM() ->
        [macos] { "" }
       *[other] { "-" }
    }
shortcut-new-tab =
  .commandkey = T

toolbarbutton-new-tab =
  .label = New Tab
  .tooltiptext = Open a new tab ({ shortcut-mod-accel-key }{ shortcut-mod-separator }{ shortcut-new-tab.commandkey })

:flod, does that sound right? Obviously the above is quite a bit of scaffolding, but AFAICT we can then do the next shortcut as, for instance,

shortcut-new-window =
  .commandkey = N

toolbarbutton-new-window =
  .label = New Window
  .tooltiptext = Open a new window ({ shortcut-mod-accel-key }{ shortcut-mod-separator }{ shortcut-new-window.commandkey })

etc.

Flags: needinfo?(francesco.lodolo)

CCing other Fluent folks in case they have ideas.

A couple of notes on the scaffolding part:

  • Separate messages for shortcut-vk-meta and shortcut-vk-ctrl would only be useful if we plan to use them outside of shortcut-mod-accel-key.
  • No need for curly parentheses around the - in the separator, only for the empty string.
toolbarbutton-new-window =
  .label = New Window
  .tooltiptext = Open a new window ({ shortcut-mod-accel-key }{ shortcut-mod-separator }{ shortcut-new-window.commandkey })

From a localization point of view, this would be extremely painful to translate: 80% of the text is references to other messages, and most locales (if not all) will just copy the English references as-is. On top of that, this pattern will be repeated in hundreds of messages.

Exclusively from an FTL point of view, the following approach would be more understandable, but I have no clue what the consequences would be on the code side, since you need to read the shortcut from another string, and pass it in the $key argument.

shortcut-template = { shortcut-mod-accel-key }{ shortcut-mod-separator }{ $key }
toolbarbutton-new-window =
  .label = New Window
  .tooltiptext = Open a new window ({ shortcut-template })

Potentially we could also hardcode the template, and pass it via $shortcut. Some locales might not be happy though (thinking of Japanese and the strange things they're used to do with accesskeys…)

toolbarbutton-new-window =
  .label = New Window
  .tooltiptext = Open a new window ({ $shortcut })
Flags: needinfo?(francesco.lodolo)

(In reply to Francesco Lodolo [:flod] from comment #3)

Exclusively from an FTL point of view, the following approach would be more understandable, but I have no clue what the consequences would be on the code side, since you need to read the shortcut from another string, and pass it in the $key argument.

I'd like a solution that is declarative, ie that gets rid of the requirement for frontend JS, and ensures we cache the localized content into startupcache now that we do that for fluent. As far as I can tell, that's not possible if we use a variable $key here that will need to be read and then passed in via data-l10n-attrs. I don't think there's a way to use the value of the variable as another fluent key, is there? Then you could at least do something like:

<toolbarbutton data-l10n-attrs='{"key":"shortcut-new-tab"}' data-l10n-id="toolbarbutton-new-tab">

or similar, again not requiring JS.

Flags: needinfo?(francesco.lodolo)

Ad-hoc, I'd think that using setAttributes would be the least horrible solution, where we'd continue to compute the command keys from data we infer on the code side, but then set that as data-l10n-atttrs.

That's effectively keeping all the code in https://searchfox.org/mozilla-central/rev/ce02064d8afc8673cef83c92896ee873bd35e7ae/toolkit/modules/ShortcutUtils.jsm#55-149 and not trying to port that to Fluent. I've toyed around with the idea of message references, but in this case, I think it's a red herring. I also entertained the idea of adding a global function for this, but that's also weird?

While I understand the benefits of being able to cache these, I'm not sure that's worth a ton of complexity in each of those tooltip strings.

(In reply to :Gijs (he/him) from comment #4)

(In reply to Francesco Lodolo [:flod] from comment #3)

Exclusively from an FTL point of view, the following approach would be more understandable, but I have no clue what the consequences would be on the code side, since you need to read the shortcut from another string, and pass it in the $key argument.

I'd like a solution that is declarative, ie that gets rid of the requirement for frontend JS, and ensures we cache the localized content into startupcache now that we do that for fluent. As far as I can tell, that's not possible if we use a variable $key here that will need to be read and then passed in via data-l10n-attrs. I don't think there's a way to use the value of the variable as another fluent key, is there?

That's definitely a good question for the technical side (Axel and Zibi).

I think we need to find a balance (compromise) between "clean code" and "localizability". In that sense, I'm OK with having one empty attribute for the separator, although I'm not a fan of empty strings. But carrying that pattern over in hundreds of messages is going to make the localization of this part of Firefox a huge pain, and incredibly error prone.

Flags: needinfo?(francesco.lodolo)

(In reply to :Gijs (he/him) from comment #4)

I'd like a solution that is declarative, ie that gets rid of the requirement for frontend JS, and ensures we cache the localized content into startupcache now that we do that for fluent. As far as I can tell, that's not possible if we use a variable $key here that will need to be read and then passed in via data-l10n-attrs.

I think I wasn't very clear.

The original problem here is that the tooltips aren't exposed to a11y software. To fix that, we need to actually set tooltiptext on the elements in question, one way or another.

The simplest solution is to have the attribute be always present. Solution (2) in comment #0 would add it dynamically, but I'm worried that won't even work with fluent + AT (and perhaps our own DOM code!) because fluent is async. So if we respond to the hover or focus event by telling fluent to translate things, we might not have the translations by the time we check if we need to show a popup and/or by the time AT asks gecko about an accessible name / description for the element.

So if we require JS usage here with a custom attribute, that means we have to run JS every time a window opens in order to get the translations onto the element, which would be a perf drain and wouldn't be very clean code.

(In reply to Francesco Lodolo [:flod] from comment #6)

But carrying that pattern over in hundreds of messages is going to make the localization of this part of Firefox a huge pain, and incredibly error prone.

I'm not sure I follow. We can abstract the localization of "accel-" away further if that's easier, and I used pretty long message identifiers because of namespacing/prefixing, which we could also consider changing. Perhaps we could encapsulate the shortcut key on toolbarbutton-new-tab itself if that makes it simpler for localizers...

I'm also not clear as to why it would be bad for localizers to copy the shortcut templates - most locales would then end up with what they want, right? As for "error-prone" - I mean, I would hope that we have syntax highlighting and checking for our locale repos...

We're also talking about just shy of 20 tooltips, not "hundreds"...

(In reply to Axel Hecht [:Pike] from comment #5)

Ad-hoc, I'd think that using setAttributes would be the least horrible solution, where we'd continue to compute the command keys from data we infer on the code side, but then set that as data-l10n-atttrs.

Note that of necessity this would require 2 l10n passes (one to get the shortcut keys, another to get the tooltips), and would require more manual work on the consumer side to re-localize on locale changes (as the shortcut key could/would change).

That's effectively keeping all the code in https://searchfox.org/mozilla-central/rev/ce02064d8afc8673cef83c92896ee873bd35e7ae/toolkit/modules/ShortcutUtils.jsm#55-149 and not trying to port that to Fluent.

If we've got .properties code that we feel we don't want to port to Fluent then I think we have a problem - along the lines of https://xkcd.com/927/ ... which would be very sad.

While I understand the benefits of being able to cache these, I'm not sure that's worth a ton of complexity in each of those tooltip strings.

Hopefully my description earlier in this comment is clearer about the reasons I strongly prefer having this in markup.

Would it be better for localizers to do something like:

shortcut-new-tab =
  .commandkey = T
  .description =
    { PLATFORM() ->
        [macos] ⌘T
       *[other] Ctrl-T
    }

and source only that 1 string instead of 3? That's less modular and more copy-paste/duplication, and is fragile in that it requires localizers to match modifiers - but they kind of have to do that anyway and copying these bits from English is perhaps not actually worse than copying { shortcut-accel-modifier } ?

(or we could go whole-hog and put the PLATFORM() check into toolbarbutton-new-tab.tooltiptext itself...)

(In reply to :Gijs (he/him) from comment #7)

I'm not sure I follow. We can abstract the localization of "accel-" away further if that's easier

What do you mean with "abstract further"?

I'm also not clear as to why it would be bad for localizers to copy the shortcut templates - most locales would then end up with what they want, right? As for "error-prone" - I mean, I would hope that we have syntax highlighting and checking for our locale repos...

We have syntax highlighting in Pontoon. As far as I remember, missing message references are not a warning, because not using them in a locale is a legitimate choice.

We're also talking about just shy of 20 tooltips, not "hundreds"...

I was more thinking long term. The approach we choose here is likely going to be used as a reference in other places, and we have a lot of legacy strings where the accesskey is displayed between parenthesis (I counted almost 70 between browser and devtools, and we might need to add more to solve accessibility issues).

(In reply to :Gijs (he/him) from comment #8)

Would it be better for localizers to do something like:

shortcut-new-tab =
  .commandkey = T
  .description =
    { PLATFORM() ->
        [macos] ⌘T
       *[other] Ctrl-T
    }

and source only that 1 string instead of 3? That's less modular and more copy-paste/duplication, and is fragile in that it requires localizers to match modifiers - but they kind of have to do that anyway and copying these bits from English is perhaps not actually worse than copying { shortcut-accel-modifier } ?

That would be worse. Ignoring the amount of duplication, there would be no way, beyond custom checks, to verify that the description matches the actual key.

Dumb question: is there no way to use the same fluent ID for the label and the shortcut?

shortcut-modifiers = { shortcut-mod-accel-key }{ shortcut-mod-separator }

toolbarbutton-new-tab =
  .label = New Tab
  .tooltiptext = Open a new tab ({ shortcut-modifiers }{ toolbarbutton-new-tab.key })
  .key = T

(In reply to Francesco Lodolo [:flod] from comment #11)

Dumb question: is there no way to use the same fluent ID for the label and the shortcut?

shortcut-modifiers = { shortcut-mod-accel-key }{ shortcut-mod-separator }

toolbarbutton-new-tab =
  .label = New Tab
  .tooltiptext = Open a new tab ({ shortcut-modifiers }{ toolbarbutton-new-tab.key })
  .key = T

Yes, we could do this, if that's acceptable from localizers' perspective.

I'm not in love with the ({ shortcut-modifiers }{ toolbarbutton-new-tab.key }), but at least it reduces the amount of duplicated content, and it's a bit more readable.

@pike, zibi
Thoughts?

<mumble>It'd be nice to have a self. or this. type of reference for attributes within the same message…</mumble>

Trying to get a better idea how big the problem space here actually is.

We're mostly talking about different elements for the actual key and the button, right?

Also, I notice that the commandkeys are lower-case, but we show them in Uppercase. Is either of that important? Also, the modifiers aren't accessible to localizers, and we do that some with and some w/out SHIFT.

Which of all these things change when we rip out XUL elements?

(In reply to Axel Hecht [:Pike] from comment #14)

Trying to get a better idea how big the problem space here actually is.

We're mostly talking about different elements for the actual key and the button, right?

Yes, I think for all of these (not even just "most").

Also, I notice that the commandkeys are lower-case, but we show them in Uppercase. Is either of that important?

I think the casing for commandkey is not important - it seems inconsistent, looking at https://searchfox.org/mozilla-central/search?q=commandkey&case=false&regexp=false&path=dtd . For the shortcut, I think convention on all platforms is upper-case.

Also, the modifiers aren't accessible to localizers, and we do that some with and some w/out SHIFT.

Yes, we'd need different identifiers for shift/non-shift, if we use a shared bit for those, and potentially a third for ctrl+alt+shift / cmd+opt+shift (e.g. browser toolbox - though I don't think any of those have tooltips atm).

Which of all these things change when we rip out XUL elements?

I don't know. The actual keys will still need to be localizable, and we want to move them from DTD to fluent, so I expect more or less nothing, wrt this bug?

<mumble>It'd be nice to have a self. or this. type of reference for attributes within the same message…</mumble>

While designing the syntax we did circulate an idea of using a sigil to refer to "itself", sth like:

shortcut-modifiers = { shortcut-mod-accel-key }{ shortcut-mod-separator }

toolbarbutton-new-tab =
  .label = New Tab
  .tooltiptext = Open a new tab ({ shortcut-modifiers }{ .key })
  .key = T

Also, I'm wondering, would dynamic references offer us another way to solve this - https://github.com/projectfluent/fluent/issues/80 ?

(In reply to :Gijs (he/him) from comment #12)

(In reply to Francesco Lodolo [:flod] from comment #11)

Dumb question: is there no way to use the same fluent ID for the label and the shortcut?

shortcut-modifiers = { shortcut-mod-accel-key }{ shortcut-mod-separator }

toolbarbutton-new-tab =
  .label = New Tab
  .tooltiptext = Open a new tab ({ shortcut-modifiers }{ toolbarbutton-new-tab.key })
  .key = T

Yes, we could do this, if that's acceptable from localizers' perspective.

Based on discussion elsewhere, this may be less possible than I thought, because all the other attributes will then also end up on the nodes that don't want them...

What's the benefit of putting all this on a single identifier?

Flags: needinfo?(francesco.lodolo)

From my point of view:

  • It's clearer for localizers that this is a shortcut and should be kept in English unless local keyboard layouts make it unreachable. It's also clear for what command it's used.
  • It's possible to automate checks on the key attribute.
Flags: needinfo?(francesco.lodolo)

Changing to task because this itself isn't a defect that directly impacts users and can have a severity, though it is potentially a fix for defects that do impact users.

Type: defect → task
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.