1162040 - [accessibility][gaia-icons] Make sure icons are properly localized for accessibility.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Components, defect)

Description

[Yura Zenevich [:yzen] (please use "needinfo")]

Currently the only content available to screen reader from an icon is defined by the CSS content property read from data-icon attribute. This is not localized and this not supported well by the screen reader.

We need to ensure that the correct localized aria-label or other meaningful attribute is localized.

Comment 1

[Wilson Page [:wilsonpage]]

This is the job of the app owner, no?

Comment 2

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Wilson Page [:wilsonpage] from comment #1)
> This is the job of the app owner, no?

Which part exactly? If you mean identifying which icons are for decoration only, that right. I am suggesting to either let the app owner apply aria-hidden to icons themselves, or have some other attribute supported within gaia-button available to the author that is more meaningfully named (I am pretty sure aria-hidden is not).

Comment 3

[Wilson Page [:wilsonpage]]

(In reply to Yura Zenevich [:yzen] from comment #2)
> (In reply to Wilson Page [:wilsonpage] from comment #1)
> > This is the job of the app owner, no?
> 
> Which part exactly? If you mean identifying which icons are for decoration
> only, that right. I am suggesting to either let the app owner apply
> aria-hidden to icons themselves, or have some other attribute supported
> within gaia-button available to the author that is more meaningfully named
> (I am pretty sure aria-hidden is not).

Can you give me an example of what you mean?

Comment 4

[Yura Zenevich [:yzen] (please use "needinfo")]

Ah, sorry I confused this bug with the one for buttons (so ignore comment 2).

I agree, in this case it is indeed authors responsibility. That can generally be said about accessibility as well. However, I feel like there's little incentive for an author to add the label. So, I'm worried that we are setting ourselves up for accessibility issues down the road, when icons are used more widely. My hope is that we can provide a toolkit (such as gaia-components) for authors to use that has accessibility in mind from the very beginning.

So I have a couple of suggestions (possible some that are more radical than the other ones):

* I think things could improve already even if we always and only demonstrate usage examples with aria-label (or data-l10n-id in Gaia context) used for icons (currently within both the README and the gh-pages).

* I wonder what you think about some helpful diagnostics when gaia icons are used. For example, have an icon style (or CSS content) that indicates warning/error when the rule is something like: 
[data-icon]:not([aria-label]),
[data-icon]:not([data-l10n-id]) {...}

Comment 5

[Wilson Page [:wilsonpage]]

Might be cool to have:

<gaia-icon name="camera"></gaia-icon>

So then we can handle accessibility inside the component. This may not be as performant as what we have right now though, as each icon would need to run some script.

Comment 6

[[e-mail]]

current gaia-icons is just a .css to include webfont,
For performance concern I prefer do that handling in each component.

suggest rule for related components:

*  If has `data-l10n-id` in <i ..>, add `aria-label` with data-l10n-id, else add `aria-hidden="true"`

So app owner only needs add essential `data-l10n-id` to indicate their explicit purpose, the component do aria treatment for them automatically.

make sense?

Comment 7

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Wilson Page [:wilsonpage] from comment #5)
> Might be cool to have:
> 
> <gaia-icon name="camera"></gaia-icon>
> 
> So then we can handle accessibility inside the component. This may not be as
> performant as what we have right now though, as each icon would need to run
> some script.

Yes, I think since both you and Fred suggest negative affect on performance this approach would be undesirable.

Comment 8

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Fred Lin [:gasolin] from comment #6)
> current gaia-icons is just a .css to include webfont,
> For performance concern I prefer do that handling in each component.
> 
> suggest rule for related components:
> 
> *  If has `data-l10n-id` in <i ..>, add `aria-label` with data-l10n-id, else
> add `aria-hidden="true"`
> 
> So app owner only needs add essential `data-l10n-id` to indicate their
> explicit purpose, the component do aria treatment for them automatically.
> 
> make sense?

I think this approach brings us back to square one, where data-l10n-id's are simply going to be omitted by the author.

I would propose:
* always enforcing data-l10n-id that maps to entry.ariaLabel in the message bundle for *all* icons.
* opt out of the label within the related components by explicitly indicating which icons are for presentation only. 

For example:
* icons will always have data-l10n-id that will resolve aria-label (already supported by l10n):
  <i data-l10n-id="camera" aria-label="Camera" data-icon="camera"></i>

* if related component only uses the icon for presentation, it should be indicated via some sort of attribute:
  <gaia-button>
    <i data-l10n-id="camera" aria-label="Camera" data-icon="camera" {someattribute, maybe even
       role="presentation"}></i>
    Camera
  </gaia-button>

Then gaia-button (and other components) could apply aria-hidden for all presentational icons.

Comment 9

[[e-mail]]

Currently in my survey in bug 1162138 comment 5, the best way to make sure icon with localizable text is put a side <span> with icon tag, and hide the span with style.

If we input

`<i data-icon="camera" data-l10n-id="camera desc"></i>`

It make rendered DOM looks like:

```
<i data-icon="camera" aria-hidden="true"></i>
<span class="icon-aria-note" data-l10n-id="camera desc">Camera</span>
```

If we take this approach to reach localizable icon text, it means add `data-l10n-id` to each icon is costly.



For the issue `data-l10n-id's are simply going to be omitted by the author`, to eliminate the root cause, we may driven the act if UX can include accessibility concern in their future released UX spec (since a11y also need to be well thought in design phase)? And app owner will follow that spec.

Comment 10

[[e-mail]]

I got a new proposal: always declare icon into some component's attribute. 
Refer to existing <gaia-header> element, 

```
```html
<gaia-header action="back" title-start="50" title-end="100">
  <h1>title</h1>
  <button data-icon="add"></button>
  <button data-icon="settings"></button>
</gaia-header>
```
```
Which already define icon in button element

we could redefine gaia-button as:

```
<button data-icon="" data-icon-end="" data-l10n-id="">
```

It means button has icon at start and end.

```
<button data-icon-only="" data-l10n-id="">
```

It means button only shows icon, but provide localized text for ScreenReader.

So we can remove all `<i>` element, embedded icons into that component, and enforce adding `data-l10n-id` attribute in target gaia-element.

Comment 11

[[e-mail]]

Thanks for keeping provide valuable advises for a11y situations.

I got a proposal to remove all `<i>` element, embedded icons into some component's attribute, and enforce adding `data-l10n-id` attribute in target gaia-element.

Refer to existing <gaia-header> element, 

```
```html
<gaia-header action="back" title-start="50" title-end="100">
  <h1>title</h1>
  <button data-icon="add"></button>
  <button data-icon="settings"></button>
</gaia-header>
```
```

Which already define data-icon in button element instead of standalone <i> element.

we could redefine gaia-button as:

```
<button data-icon="" data-icon-end="" data-l10n-id="">
```

It means button has icon at start and end.

```
<button data-icon-only="" data-l10n-id="">
```

It means button only shows icon, but provide localized text for ScreenReader. (I'm not sure if icon-only button can works with data-l10n-id this way now, but its a proposal)

Comment 12

[[e-mail]]

sorry I've some network connection issue, please ignore comment 10

Comment 13

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Fred Lin [:gasolin] from comment #9)
> Currently in my survey in bug 1162138 comment 5, the best way to make sure
> icon with localizable text is put a side <span> with icon tag, and hide the
> span with style.
> 
> If we input
> 
> `<i data-icon="camera" data-l10n-id="camera desc"></i>`
> 
> It make rendered DOM looks like:
> 
> ```
> <i data-icon="camera" aria-hidden="true"></i>
> <span class="icon-aria-note" data-l10n-id="camera desc">Camera</span>
> ```

In case of screen reader specific localization, I think it is not the case, since it depends on what data-l10n-id points to. So for example if data-l10n-id="test" points to test.ariaLabel=Test in the message bundle, then l10n will create an aria-label="Test" attribute on the node instead of touching inner HTML. But I see your comment about removing <i>'s.

> 
> If we take this approach to reach localizable icon text, it means add
> `data-l10n-id` to each icon is costly.
> 
> 
> 
> For the issue `data-l10n-id's are simply going to be omitted by the author`,
> to eliminate the root cause, we may driven the act if UX can include
> accessibility concern in their future released UX spec (since a11y also need
> to be well thought in design phase)? And app owner will follow that spec.

Comment 14

[Yura Zenevich [:yzen] (please use "needinfo")]

I think there are still two separate problems here:

1) Identifying gaia-icons for presentation only.
2) Localizing gaia-icons

The first problem can be solved via aria-hidden="true" attribute. If it is present, localized or not, our screen reader will not be able to reach the icon. It is a good indicator of whether the icon needs localization.

The second problem depends on the context:
* If gaia-icons are used as a standalone, e.g. data-icon attributed element.
  Unless aria-hidden attribute is used on the icon, it will always be accessible to the screen reader, which means it must be localized (if it is not, the screen reader will read the text from the CSS content property which is not localized). It can be achieved by adding data-l10n-id to the element with data-icon that points to {property}.ariaLabel in the properties file (that will add an aria-label attribute to the same element and will not touch inner HTML).
  
* If gaia-icons are used in context of another gaia-component
Pretty much the same rules apply, but we, as a component author, can infer certain things about icon visibility and localization, or have some abstraction where gaia element attributes are propagated to icons, buttons etc.

So to reply to your comment about removing <i>, I think it's mostly orthogonal problem:

(In reply to Fred Lin [:gasolin] from comment #11)
> Thanks for keeping provide valuable advises for a11y situations.
> 
> I got a proposal to remove all `<i>` element, embedded icons into some
> component's attribute, and enforce adding `data-l10n-id` attribute in target
> gaia-element.

This is good, I'm especially in support of enforcing data-l10n-id attribute. As far as I understand, this means that we enforce localization of all elements with data-icon? This would help with (2).

> 
> Refer to existing <gaia-header> element, 
> 
> ```
> ```html
> <gaia-header action="back" title-start="50" title-end="100">
>   <h1>title</h1>
>   <button data-icon="add"></button>
>   <button data-icon="settings"></button>
> </gaia-header>
> ```
> ```
> 
> Which already define data-icon in button element instead of standalone <i>
> element.
> 
> we could redefine gaia-button as:
> 
> ```
> <button data-icon="" data-icon-end="" data-l10n-id="">
> ```
> 
> It means button has icon at start and end.

AFAIK, there're a lot of permutations of possible textual descriptions here:
* {Start icon label} button text {End icon label}
* {Start icon label} {End icon label}
* {Start icon label} button text
* button text {End icon label}
* button text

If data-l10n-id specifies aria-label then it will override everything for the screen reader: inner text and icon content CSS properties. I guess this is author's responsibility to always provide a meaningful label for the screen reader and as long as we have examples showing that it should be reasonable.

> 
> ```
> <button data-icon-only="" data-l10n-id="">
> ```

I think this is simply a case of 
```
<button data-icon="" data-l10n-id="">
```

> 
> It means button only shows icon, but provide localized text for
> ScreenReader. (I'm not sure if icon-only button can works with data-l10n-id
> this way now, but its a proposal)

Yes data-l10n-id supports aria-label see comment 9.

Comment 15

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Yura Zenevich [:yzen] from comment #14)
> Yes data-l10n-id supports aria-label see comment 9.
Sorry comment 13

Comment 16

[Yura Zenevich [:yzen] (please use "needinfo")]

So to move forward in terms of this bug, I think the scope here is simply: 

* updating the documentation that shows only examples with aria-label/data-l10n-id specified for elements that use data-icon (button, i, etc). it fixes both where icon is complementary to text or is standalone (like the button example), aria-label will override both icon content text and inner HTML of the element.

* when icon is present for presentation only, show examples where aria-hidden is used to make it unreachable by the screen reader.

What do you think?

Maybe in cases where icons are used in other components we can have data-icon-presentation="" or something like that that can be interpreted as 'apply aria-hidden="true" here'?

Comment 17

[[e-mail]]

Attachment: pull request redirect to github

Thanks for providing related knowledge about data-l10n-id. I wrote the accessibility section of gaia-icon README, to identify `aria-label`, `aria-hidden`, and `{property}.ariaLabel` usage scenarios.

For icons are used in other components, I think each component should has its own treatment for icons (but may follow some conventions that we could figure out in gaia-button or gaia-header)

Comment 18

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8607949 [details] [review]
pull request redirect to github

Thanks for this, it is super helpful

Comment 19

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Fred Lin [:gasolin] from comment #17)
> For icons are used in other components, I think each component should has
> its own treatment for icons (but may follow some conventions that we could
> figure out in gaia-button or gaia-header)

Perhaps we can even add some sort of logic in gaia-component is all of our components inherit from it?

Comment 20

[[e-mail]]

(In reply to Yura Zenevich [:yzen] from comment #19)
> Perhaps we can even add some sort of logic in gaia-component is all of our
> components inherit from it?

Since not all components use icon in it, we could review if there is any thing we found its useful from the practice with gaia-button or gaia-header.

Comment 21

[[e-mail]]

Comment on attachment 8607949 [details] [review]
pull request redirect to github

According the discussion from comment 13 to comment 16, we come out the icon related usage and design treatment. Please helping check it.

Comment 22

[Wilson Page [:wilsonpage]]

Comment on attachment 8607949 [details] [review]
pull request redirect to github

Some nits inline, but looks good.

Comment 23

[Wilson Page [:wilsonpage]]

Comment on attachment 8607949 [details] [review]
pull request redirect to github

LANDED https://github.com/gaia-components/gaia-icons/commit/aac506e760a343701ac6a84b8fd22371e4c8a915

Comment 24

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: Proposed warning/diagnostics

Wondering what you guys think of this approach in addition to the documentation user guide that's been updated for a11y?

Comment 25

[Wilson Page [:wilsonpage]]

Comment on attachment 8614078 [details] [review]
Proposed warning/diagnostics

I like the approach, but this will require a lot of changes across Gaia as I don't think (m)any apps are using gaia-icons in this way yet.

Comment 26

[Yura Zenevich [:yzen] (please use "needinfo")]

Reopening to carry out some extra improvements based on the proposed warning highlighting.

Comment 27

[[e-mail]]

Comment on attachment 8614078 [details] [review]
Proposed warning/diagnostics

It looks well to me. I took it into button stress test (added related data-l10n-id) and it goes with no obvious performance impact

9647ms vs 9575ms(with patch)

https://github.com/gaia-components/gaia-button/blob/master/test/stress.html
(myth: I've test twice and the result with patch always better than the origin :-/).

Though in buttons the notification text is relatively large and the  description is truncated.

Comment 28

[Wilson Page [:wilsonpage]]

Comment on attachment 8614078 [details] [review]
Proposed warning/diagnostics

LANDED https://github.com/gaia-components/gaia-icons/commit/de969d9a6350535e29b80eb2cc2e7c4b4922b1dd
STAMPED v0.9.0 https://github.com/gaia-components/gaia-icons/releases/tag/v0.9.0

Comment 29

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: System PR

Comment 30

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: Settings PR

Comment 31

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: Calendar PR

Comment 32

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: Dialer PR

Comment 33

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: Costcontrol PR

Comment 34

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: Music PR

Comment 35

[Yura Zenevich [:yzen] (please use "needinfo")]

Calendar: https://github.com/mozilla-b2g/gaia/commit/adeb892c2c0d89db1601e77a2f5a66f9ee026f23

Comment 36

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: Privacy Panel PR

Comment 37

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: Ringtones PR

Comment 38

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: SMS PR

Comment 39

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: Wappush PR

Comment 40

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: new gaia icons in gaia PR

This should lend after all app specific PR's are in.

Comment 41

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8615509 [details] [review]
Dialer PR

I didn't try this, but it looks fine.

Comment 42

[Yura Zenevich [:yzen] (please use "needinfo")]

Dialer: https://github.com/mozilla-b2g/gaia/commit/c311ee027c34c173534c3e6b0ea68ea12829b910

Comment 43

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8615534 [details] [review]
Privacy Panel PR

I am not sure if I could review this. Tim, please help to redirect.

Comment 44

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8615362 [details] [review]
System PR

Cool, Michael, please help to review this. Thanks.

Comment 45

[Marina Rodríguez [:mai]]

Comment on attachment 8615511 [details] [review]
Costcontrol PR

LGTM. Please correct the nit on GitHub.

Comment 46

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8615474 [details] [review]
Settings PR

The patch is looking good. The only question I got would be why it is required adding `aria-describedby` on the icons of the menu items on the root panel. The icons are not functional and the screen reader should be able to read the text on the menu items.

Comment 47

[Julien Wajsberg [:julienw]]

Comment on attachment 8615551 [details] [review]
Wappush PR

Looks good but I'll defer to Gabriele because I seldom know this code.

Comment 48

[Julien Wajsberg [:julienw]]

Hey Matej, can you please have a look at my string proposals in https://github.com/mozilla-b2g/gaia/pull/30431/files#r31806953 ? Thanks a lot !

Comment 49

[Gabriele Svelto [:gsvelto] (PTO)]

Comment on attachment 8615551 [details] [review]
Wappush PR

Yup, that element should serve no purpose besides being a visual hint IIRC.

Comment 50

[Etienne Segonzac (:etienne)]

Comment on attachment 8615534 [details] [review]
Privacy Panel PR

lgtm

Comment 51

[Yura Zenevich [:yzen] (please use "needinfo")]

cost control: https://github.com/mozilla-b2g/gaia/commit/a295b6818650edbabc2d7c4662d23a3fa14b57ea

Comment 52

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8615474 [details] [review]
Settings PR

(In reply to Arthur Chen [:arthurcc] from comment #46)
> Comment on attachment 8615474 [details] [review]
> Settings PR
> 
> The patch is looking good. The only question I got would be why it is
> required adding `aria-describedby` on the icons of the menu items on the
> root panel. The icons are not functional and the screen reader should be
> able to read the text on the menu items.

I left some comments in Github. But to answer your question:
We can't hide the element with data-icon with aria-hidden because is it will hide the element (including its text and sub-tree) from screen reader. I'm actually overriding the label on the link itself which means we replace all of content of the label including the description. To not loose description, I have to use aria-describedby to make sure it is included when the screen reader lands on the menu item.

Comment 53

[Yura Zenevich [:yzen] (please use "needinfo")]

wappush: https://github.com/mozilla-b2g/gaia/commit/0dcec2cdd0b9c8823d67c30adecf858b0cf333c4

Comment 54

[Yura Zenevich [:yzen] (please use "needinfo")]

privacy panel: https://github.com/mozilla-b2g/gaia/commit/9961ea9adb3a9a88ed9ba811791336b67669c62a

Comment 55

[Yura Zenevich [:yzen] (please use "needinfo")]

Replied to your comment for SMS PR, thanks!

Comment 56

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8615362 [details] [review]
System PR

Left one small fix on github, but LGTM. Would be great if we had some tests for this, so that we didn't break these aria labels in the future. But I won't block this bug on it.

Comment 57

[Yura Zenevich [:yzen] (please use "needinfo")]

system: https://github.com/mozilla-b2g/gaia/commit/55858863f320efc73c0bfd9b3eef905e49998e39

Comment 58

[Julien Wajsberg [:julienw]]

Replied on github -- I'd also like to wait for Matej's answer before landing :)

Comment 59

[Arthur Chen [:arthurcc][inactive after 6/30]]

(In reply to Yura Zenevich [:yzen] from comment #52)
> Comment on attachment 8615474 [details] [review]
> Settings PR
> 
> (In reply to Arthur Chen [:arthurcc] from comment #46)
> > Comment on attachment 8615474 [details] [review]
> > Settings PR
> > 
> > The patch is looking good. The only question I got would be why it is
> > required adding `aria-describedby` on the icons of the menu items on the
> > root panel. The icons are not functional and the screen reader should be
> > able to read the text on the menu items.
> 
> I left some comments in Github. But to answer your question:
> We can't hide the element with data-icon with aria-hidden because is it will
> hide the element (including its text and sub-tree) from screen reader. I'm
> actually overriding the label on the link itself which means we replace all
> of content of the label including the description. To not loose description,
> I have to use aria-describedby to make sure it is included when the screen
> reader lands on the menu item.

I see. However, it seems not a good idea adding an l10n id on an element with child elements. Is it possible making screen reader skip the icon if it cannot find an l10n id?

Comment 60

[Dominic Kuo [:dkuo]]

Comment on attachment 8615525 [details] [review]
Music PR

Though I don't know why we didn't add all the shuffle icon ids in bug 1136393, but I am glad we add them now, looks good to me and thanks for working this.

Comment 61

[Yura Zenevich [:yzen] (please use "needinfo")]

music: https://github.com/mozilla-b2g/gaia/commit/6c18f00726d50b1aa0e98bd9bbc285c593e32b1d

Comment 62

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Arthur Chen [:arthurcc] from comment #59)
> (In reply to Yura Zenevich [:yzen] from comment #52)
> > Comment on attachment 8615474 [details] [review]
> > Settings PR
> > 
> > (In reply to Arthur Chen [:arthurcc] from comment #46)
> > > Comment on attachment 8615474 [details] [review]
> > > Settings PR
> > > 
> > > The patch is looking good. The only question I got would be why it is
> > > required adding `aria-describedby` on the icons of the menu items on the
> > > root panel. The icons are not functional and the screen reader should be
> > > able to read the text on the menu items.
> > 
> > I left some comments in Github. But to answer your question:
> > We can't hide the element with data-icon with aria-hidden because is it will
> > hide the element (including its text and sub-tree) from screen reader. I'm
> > actually overriding the label on the link itself which means we replace all
> > of content of the label including the description. To not loose description,
> > I have to use aria-describedby to make sure it is included when the screen
> > reader lands on the menu item.
> 
> I see. However, it seems not a good idea adding an l10n id on an element
> with child elements. Is it possible making screen reader skip the icon if it
> cannot find an l10n id?

Unfortunately it's impossible :(. The css context field can only be hidden with aria-hidden, but if it is applied on the menu item, all of the menu item is going to become invisible to the screen reader. This is unfortunately the problem with Gaia icons atm where if it is not meaningful and needs to be hidden it should have aria-hidden applied to it. But if it has inner content, aria-hidden can not be applied and we should have an aria-label overriding all of the inner content and the icon altogether. Let me know what you think.

Comment 63

[Matej Novak [:matej]]

(In reply to Julien Wajsberg [:julienw] from comment #48)
> Hey Matej, can you please have a look at my string proposals in
> https://github.com/mozilla-b2g/gaia/pull/30431/files#r31806953 ? Thanks a
> lot !

I like the idea of verbs, but I actually think it starts to introduce some complications. Instead of "show options," it could be "view options," which is shift in the user's role in the action.

I also think "compose" sounds a bit technical. I would actually go back to the original set, but use "new message" instead of "compose."

Options
Settings
New message
Call

The one thing that isn't clear to me is the difference between "options" and "settings." Those feel roughly synonymous and I wouldn't know what to expect to find in each. 

Hope that helps.

Comment 64

[Julien Wajsberg [:julienw]]

(In reply to Matej Novak [:matej] from comment #63)
> (In reply to Julien Wajsberg [:julienw] from comment #48)
> 
> Options
> Settings
> New message
> Call
> 
> The one thing that isn't clear to me is the difference between "options" and
> "settings." Those feel roughly synonymous and I wouldn't know what to expect
> to find in each. 
> 

You're totally right, the "options" button is really showing an "additional actions" menu. In the Inbox, the "actions" menu contains "settings", "select conversations" (to be deleted/marked as read/unread). In a Conversation, the "actions" menu contains "select messages" and "add a subject".

I'm not sure what the right label could be. Maybe "actions" ?

Comment 65

[Matej Novak [:matej]]

(In reply to Julien Wajsberg [:julienw] from comment #64)
> (In reply to Matej Novak [:matej] from comment #63)
> > (In reply to Julien Wajsberg [:julienw] from comment #48)
> > 
> > Options
> > Settings
> > New message
> > Call
> > 
> > The one thing that isn't clear to me is the difference between "options" and
> > "settings." Those feel roughly synonymous and I wouldn't know what to expect
> > to find in each. 
> > 
> 
> You're totally right, the "options" button is really showing an "additional
> actions" menu. In the Inbox, the "actions" menu contains "settings", "select
> conversations" (to be deleted/marked as read/unread). In a Conversation, the
> "actions" menu contains "select messages" and "add a subject".
> 
> I'm not sure what the right label could be. Maybe "actions" ?

Yeah, I think actions is a better representation of that and helps distinguish it from "Settings."

Comment 66

[Arthur Chen [:arthurcc][inactive after 6/30]]

(In reply to Yura Zenevich [:yzen] from comment #62)
> Unfortunately it's impossible :(. The css context field can only be hidden
> with aria-hidden, but if it is applied on the menu item, all of the menu
> item is going to become invisible to the screen reader. This is
> unfortunately the problem with Gaia icons atm where if it is not meaningful
> and needs to be hidden it should have aria-hidden applied to it. But if it
> has inner content, aria-hidden can not be applied and we should have an
> aria-label overriding all of the inner content and the icon altogether. Let
> me know what you think.

I'd just like to ensure there won't be problems when adding l10n ids on elements with inner content.

Stas, I remember that l10n throws an exception under this condition but I can't find related code anymore. Could you shed some light on this? Thanks!

Comment 67

[Julien Wajsberg [:julienw]]

Comment on attachment 8615548 [details] [review]
SMS PR

r=me with "Actions" instead of "Options".

Thanks Matej and Yura !

Comment 68

[Yura Zenevich [:yzen] (please use "needinfo")]

sms https://github.com/mozilla-b2g/gaia/commit/ae5630b2103d96a9eae1b39a0ab018b8bfa279e5

Comment 69

[Yura Zenevich [:yzen] (please use "needinfo")]

ringtones https://github.com/mozilla-b2g/gaia/commit/0b5638a17239080b4163cd81656028b4127f02dc

Comment 70

[Staś Małolepszy :stas]

(In reply to Arthur Chen [:arthurcc] from comment #66)

> Stas, I remember that l10n throws an exception under this condition but I
> can't find related code anymore. Could you shed some light on this? Thanks!

We used to warn about this and then we'd even throw in order to prepare for a change in the behavior which already landed.

Currently, the logic is as follows:

 1. if the value of the localization string is null (there's no foo=... where ... might be empty), then l10n.js doesn't touch the subtree of the element and only localizes its attributes,

 2. if the value if the localization string is not null, i.e. is a string (possibly an empty one!), we do one of the following things:

 2.1. if there's no "<" nor "&" in the string value, we replace the elements textContent.  All the subtree will be removed in this case.

 2.2. otherwise, we assume the string value of the translation contains HTML which needs to be sanitized. We then try to match HTML elements from the translation with the element in the subtree.  The elements in the translation are the authoritative ones: if an element is missing from the translation, it will be removed from source.

Here, #1 applies for translations with no value and aria-* attributes, so everything should work as expected.

Comment 71

[Yura Zenevich [:yzen] (please use "needinfo")]

Arthur, let me know if the PR looks good now, thanks!

Comment 72

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8615474 [details] [review]
Settings PR

Thanks for the explanation, stas!

r=me, thanks yura!

Comment 73

[Yura Zenevich [:yzen] (please use "needinfo")]

settings https://github.com/mozilla-b2g/gaia/commit/c6d7b3a8f9ee17221757a2e3c61da6f8f73bde04

Comment 74

[Yura Zenevich [:yzen] (please use "needinfo")]

gaia icons:https://github.com/mozilla-b2g/gaia/commit/20aafde5ef9edf3ff7d8aa0f3d8e05aba224e5c9

Comment 76

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: A Small follow up.

Comment 77

[Wilson Page [:wilsonpage]]

Comment on attachment 8670295 [details] [review]
A Small follow up.

https://github.com/gaia-components/gaia-icons/releases/tag/v0.10.2