Closed Bug 1162040 Opened 10 years ago Closed 9 years ago

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

Categories

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

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: yzen, Assigned: yzen)

References

Details

(Keywords: access)

Attachments

(14 files)

53 bytes, text/x-github-pull-request
wilsonpage
: review+
yzen
: feedback+
Details | Review
53 bytes, text/x-github-pull-request
wilsonpage
: review+
wilsonpage
: feedback+
gasolin
: feedback+
Details | Review
46 bytes, text/x-github-pull-request
mikehenrty
: review+
Details | Review
46 bytes, text/x-github-pull-request
arthurcc
: review+
Details | Review
46 bytes, text/x-github-pull-request
mmedeiros
: review+
Details | Review
46 bytes, text/x-github-pull-request
drs
: review+
Details | Review
46 bytes, text/x-github-pull-request
mai
: review+
Details | Review
46 bytes, text/x-github-pull-request
dkuo
: review+
Details | Review
46 bytes, text/x-github-pull-request
etienne
: review+
Details | Review
46 bytes, text/x-github-pull-request
squib
: review+
Details | Review
46 bytes, text/x-github-pull-request
julienw
: review+
Details | Review
46 bytes, text/x-github-pull-request
gsvelto
: review+
Details | Review
46 bytes, text/x-github-pull-request
wilsonpage
: review+
Details | Review
53 bytes, text/x-github-pull-request
wilsonpage
: review+
Details | Review
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.
This is the job of the app owner, no?
(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).
(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?
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]) {...}
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.
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?
(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.
(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.
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.
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.
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)
sorry I've some network connection issue, please ignore comment 10
(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.
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.
(In reply to Yura Zenevich [:yzen] from comment #14) > Yes data-l10n-id supports aria-label see comment 9. Sorry comment 13
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'?
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)
Attachment #8607949 - Flags: feedback?(yzenevich)
Comment on attachment 8607949 [details] [review] pull request redirect to github Thanks for this, it is super helpful
Attachment #8607949 - Flags: feedback?(yzenevich) → feedback+
(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?
(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 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.
Attachment #8607949 - Flags: review?(wilsonpage)
Comment on attachment 8607949 [details] [review] pull request redirect to github Some nits inline, but looks good.
Attachment #8607949 - Flags: review?(wilsonpage) → review+
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Wondering what you guys think of this approach in addition to the documentation user guide that's been updated for a11y?
Attachment #8614078 - Flags: feedback?(wilsonpage)
Attachment #8614078 - Flags: feedback?(gasolin)
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.
Attachment #8614078 - Flags: feedback?(wilsonpage) → feedback+
Reopening to carry out some extra improvements based on the proposed warning highlighting.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #8614078 - Flags: review?(wilsonpage)
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.
Attachment #8614078 - Flags: feedback?(gasolin) → feedback+
Attached file System PR
Attachment #8615362 - Flags: review?(alive)
Assignee: nobody → yzenevich
Attached file Settings PR
Attachment #8615474 - Flags: review?(arthur.chen)
Attached file Calendar PR
Attachment #8615490 - Flags: review?(mmedeiros)
Attached file Dialer PR
Attachment #8615509 - Flags: review?(drs)
Attachment #8615490 - Flags: review?(mmedeiros) → review+
Attached file Costcontrol PR
Attachment #8615511 - Flags: review?(marina.rodrigueziglesias)
Attached file Music PR
Attachment #8615525 - Flags: review?(dkuo)
Attached file Privacy Panel PR
Attachment #8615534 - Flags: review?(alive)
Attached file Ringtones PR
Attachment #8615536 - Flags: review?(squibblyflabbetydoo)
Attached file SMS PR
Attachment #8615548 - Flags: review?(felash)
Attached file Wappush PR
Attachment #8615551 - Flags: review?(felash)
This should lend after all app specific PR's are in.
Attachment #8615561 - Flags: review?(wilsonpage)
Comment on attachment 8615509 [details] [review] Dialer PR I didn't try this, but it looks fine.
Attachment #8615509 - Flags: review?(drs) → review+
Comment on attachment 8615534 [details] [review] Privacy Panel PR I am not sure if I could review this. Tim, please help to redirect.
Attachment #8615534 - Flags: review?(alive) → review?(timdream)
Comment on attachment 8615362 [details] [review] System PR Cool, Michael, please help to review this. Thanks.
Attachment #8615362 - Flags: review?(alive) → review?(mhenretty)
Comment on attachment 8615511 [details] [review] Costcontrol PR LGTM. Please correct the nit on GitHub.
Attachment #8615511 - Flags: review?(marina.rodrigueziglesias) → review+
Attachment #8615534 - Flags: review?(timdream) → review?(etienne)
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.
Attachment #8615474 - Flags: review?(arthur.chen)
Comment on attachment 8615551 [details] [review] Wappush PR Looks good but I'll defer to Gabriele because I seldom know this code.
Attachment #8615551 - Flags: review?(felash) → review?(gsvelto)
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 !
Flags: needinfo?(matej)
Comment on attachment 8615551 [details] [review] Wappush PR Yup, that element should serve no purpose besides being a visual hint IIRC.
Attachment #8615551 - Flags: review?(gsvelto) → review+
Comment on attachment 8615534 [details] [review] Privacy Panel PR lgtm
Attachment #8615534 - Flags: review?(etienne) → review+
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.
Attachment #8615474 - Flags: review?(arthur.chen)
Replied to your comment for SMS PR, thanks!
Flags: needinfo?(felash)
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.
Attachment #8615362 - Flags: review?(mhenretty) → review+
Replied on github -- I'd also like to wait for Matej's answer before landing :)
Flags: needinfo?(felash)
(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?
Flags: needinfo?(yzenevich)
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.
Attachment #8615525 - Flags: review?(dkuo) → review+
Attachment #8615561 - Flags: review?(wilsonpage) → review+
(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.
Flags: needinfo?(yzenevich) → needinfo?(arthur.chen)
(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.
Flags: needinfo?(matej)
(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" ?
Flags: needinfo?(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."
Flags: needinfo?(matej)
(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!
Flags: needinfo?(arthur.chen) → needinfo?(stas)
Attachment #8615536 - Flags: review?(squibblyflabbetydoo) → review+
Comment on attachment 8615548 [details] [review] SMS PR r=me with "Actions" instead of "Options". Thanks Matej and Yura !
Attachment #8615548 - Flags: review?(felash) → review+
(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.
Flags: needinfo?(stas)
Arthur, let me know if the PR looks good now, thanks!
Flags: needinfo?(arthur.chen)
Comment on attachment 8615474 [details] [review] Settings PR Thanks for the explanation, stas! r=me, thanks yura!
Flags: needinfo?(arthur.chen)
Attachment #8615474 - Flags: review?(arthur.chen) → review+
Status: REOPENED → RESOLVED
Closed: 10 years ago9 years ago
Resolution: --- → FIXED
Attached file A Small follow up.
Attachment #8670295 - Flags: review?(wilsonpage)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: