1123760 - Inline urlbar buttons (identity block, go/stop/reload, reading list, ...) and autocomplete dropdown arrow should have proper label

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect)

Description

[:Gijs (he/him)]

They currently all expose "Location" as their name to MSAA consumers. I bet this is fixable if we just give go/stop/reload appropriate aria-label attributes that mirror their tooltip contents, and add new strings for the identity block and the location bar dropdown icon.

Comment 1

[:Gijs (he/him)]

This will also need to cover the focusable notification icons in the icon-box in the location bar...

Comment 2

[:Gijs (he/him)]

Attachment: MozReview Request: Bug 1123760 - fix accessible labels of urlbar items, r?florian,f?MarcoZ

Bug 1123760 - fix accessible labels of urlbar items, r?florian,f?MarcoZ

Comment 3

[:Gijs (he/him)]

Attachment: MozReview Request: Bug 1123760 - make autocomplete dropmarker in the urlbar actually work when activated through a11y APIs, r=surkov

Bug 1123760 - make autocomplete dropmarker in the urlbar actually work when activated through a11y APIs, r?surkov

Comment 4

[:Gijs (he/him)]

Comment on attachment 8673166 [details]
MozReview Request: Bug 1123760 - fix accessible labels of urlbar items, r?florian,f?MarcoZ

Try push:

  https://treeherder.mozilla.org/#/jobs?repo=try&revision=e2515e014e3b

Comment 5

[alexander :surkov (:asurkov)]

Comment on attachment 8673167 [details]
MozReview Request: Bug 1123760 - make autocomplete dropmarker in the urlbar actually work when activated through a11y APIs, r=surkov

https://reviewboard.mozilla.org/r/21851/#review19633

::: accessible/xul/XULFormControlAccessible.cpp:227
(Diff revision 1)
> -  nsCOMPtr<nsIDOMXULButtonElement> parentButtonElement =
> +  nsCOMPtr<nsIContent> parent = mContent->GetFlattenedTreeParent();

nsIContent*

Comment 6

[Florian Quèze [:florian]]

Comment on attachment 8673166 [details]
MozReview Request: Bug 1123760 - fix accessible labels of urlbar items, r?florian,f?MarcoZ

https://reviewboard.mozilla.org/r/21849/#review19691

::: browser/base/content/browser.xul:715
(Diff revision 1)
>                  <image id="identity-notification-icon" class="notification-anchor-icon" role="button"/>

Does the identity notification icon not need an accessible label?

::: browser/locales/en-US/chrome/browser/browser.dtd:206
(Diff revision 1)
> +<!ENTITY urlbar.webRTCShareDevicesNotificationAnchor.label      "Manage sharing your camera with the site">

The notification shows a camera icon, but the reason why it's called "Devices" rather than "Camera" is that when both a camera and a microphone are shared, we show this icon. The label displayed inside the notification is adapted to explain whether only a camera, or a camera + a microphone is currently shared.

I'm not sure if we should bother adapting the accessible label based on the exact situation, or if we can just try to find a better wording implying that a microphone 'may' also be shared.

::: browser/locales/en-US/chrome/browser/browser.dtd:214
(Diff revision 1)
> +<!ENTITY urlbar.servicesNotificationAnchor.label        "View the service install message">

"the service install message" doesn't make much sense to me, so I'm afraid it may also be confusing to users, and may get even more confusing once translated by a localizer that may not understand it better than I do.

Comment 7

[:Gijs (he/him)]

(In reply to Florian Quèze [:florian] [:flo] from comment #6)
> Comment on attachment 8673166 [details]
> MozReview Request: Bug 1123760 - fix accessible labels of urlbar items,
> r?florian,f?MarcoZ
> 
> https://reviewboard.mozilla.org/r/21849/#review19691
> 
> ::: browser/base/content/browser.xul:715
> (Diff revision 1)
> >                  <image id="identity-notification-icon" class="notification-anchor-icon" role="button"/>
> 
> Does the identity notification icon not need an accessible label?

It's only used for persona auth, (hidden) preffed off by default, and considering our current (lack of) commitment to persona I expect it'll disappear from the tree. Doesn't seem worth it.

> ::: browser/locales/en-US/chrome/browser/browser.dtd:206
> (Diff revision 1)
> > +<!ENTITY urlbar.webRTCShareDevicesNotificationAnchor.label      "Manage sharing your camera with the site">
> 
> The notification shows a camera icon, but the reason why it's called
> "Devices" rather than "Camera" is that when both a camera and a microphone
> are shared, we show this icon. The label displayed inside the notification
> is adapted to explain whether only a camera, or a camera + a microphone is
> currently shared.
> 
> I'm not sure if we should bother adapting the accessible label based on the
> exact situation, or if we can just try to find a better wording implying
> that a microphone 'may' also be shared.

Can you suggest a better generic wording?

> ::: browser/locales/en-US/chrome/browser/browser.dtd:214
> (Diff revision 1)
> > +<!ENTITY urlbar.servicesNotificationAnchor.label        "View the service install message">
> 
> "the service install message" doesn't make much sense to me, so I'm afraid
> it may also be confusing to users, and may get even more confusing once
> translated by a localizer that may not understand it better than I do.

The popups themselves use "Service" as a descriptor. I don't think we should use different terminology. Can you suggest better wording?

Comment 8

[Florian Quèze [:florian]]

(In reply to :Gijs Kruitbosch from comment #7)
> (In reply to Florian Quèze [:florian] [:flo] from comment #6)
> > Comment on attachment 8673166 [details]
> > MozReview Request: Bug 1123760 - fix accessible labels of urlbar items,
> > r?florian,f?MarcoZ
> > 
> > https://reviewboard.mozilla.org/r/21849/#review19691
> > 
> > ::: browser/base/content/browser.xul:715
> > (Diff revision 1)
> > >                  <image id="identity-notification-icon" class="notification-anchor-icon" role="button"/>
> > 
> > Does the identity notification icon not need an accessible label?
> 
> It's only used for persona auth, (hidden) preffed off by default, and
> considering our current (lack of) commitment to persona I expect it'll
> disappear from the tree. Doesn't seem worth it.

Fair enough. Maybe we should have a comment saying that it's preff'ed off, so that the next person looking at the code doesn't wonder the same thing?

> > ::: browser/locales/en-US/chrome/browser/browser.dtd:206
> > (Diff revision 1)
> > > +<!ENTITY urlbar.webRTCShareDevicesNotificationAnchor.label      "Manage sharing your camera with the site">
> > 
> > The notification shows a camera icon, but the reason why it's called
> > "Devices" rather than "Camera" is that when both a camera and a microphone
> > are shared, we show this icon. The label displayed inside the notification
> > is adapted to explain whether only a camera, or a camera + a microphone is
> > currently shared.
> > 
> > I'm not sure if we should bother adapting the accessible label based on the
> > exact situation, or if we can just try to find a better wording implying
> > that a microphone 'may' also be shared.
> 
> Can you suggest a better generic wording?

We agreed on IRC for:
Manage sharing your camera and/or microphone with the site

This comment also applied to the 'You are sharing your camera with the site' string that should also say camera and/or microphone.

> > ::: browser/locales/en-US/chrome/browser/browser.dtd:214
> > (Diff revision 1)
> > > +<!ENTITY urlbar.servicesNotificationAnchor.label        "View the service install message">
> > 
> > "the service install message" doesn't make much sense to me, so I'm afraid
> > it may also be confusing to users, and may get even more confusing once
> > translated by a localizer that may not understand it better than I do.
> 
> The popups themselves use "Service" as a descriptor. I don't think we should
> use different terminology. Can you suggest better wording?

Given that this is consistent with the other add-on and app install messages you have, I withdraw this comment.

Comment 9

[Marco Zehe (:MarcoZ)]

Comment on attachment 8673166 [details]
MozReview Request: Bug 1123760 - fix accessible labels of urlbar items, r?florian,f?MarcoZ

This looks good for all the buttons we provide, but if an add-on adds itself into the tool bar items, these buttons get a label of "Location" as they did before.

So I am wondering if, since they provide tooltip text anyway, we could make this easier and try to assign the aria-label the same content even for add-ons?

Comment 10

[:Gijs (he/him)]

(In reply to Marco Zehe (:MarcoZ) from comment #9)
> Comment on attachment 8673166 [details]
> MozReview Request: Bug 1123760 - fix accessible labels of urlbar items,
> r?florian,f?MarcoZ
> 
> This looks good for all the buttons we provide, but if an add-on adds itself
> into the tool bar items, these buttons get a label of "Location" as they did
> before.
> 
> So I am wondering if, since they provide tooltip text anyway, we could make
> this easier and try to assign the aria-label the same content even for
> add-ons?

That sounds like a question for Surkov? Firefox frontend code can't "intercept" the icons add-ons add and modify their attributes, because they just use XUL overlays or manually append stuff - we do not get told when this happens. I'm also not sure how many of these icons have tooltips - our own icons (at least the ones next to the identity box) often don't. If you mean the type of icons at the end of the bar, like stop/go/reload, then I guess those do often have tooltips and so the a11y code could do that. I can file a followup for this.

Comment 11

[Marco Zehe (:MarcoZ)]

Comment on attachment 8673166 [details]
MozReview Request: Bug 1123760 - fix accessible labels of urlbar items, r?florian,f?MarcoZ

Ah OK! Thought if they were using the Jetpack method, we could intercept those. I was using something similar with the Tenon.io extension I wrote and thought this might be straight-forward. Yeah a follow-up bug might be nice, although with all this being in flux anyway, we might want to wait on fixing it until we have a new proper mechanism in place that no longer relies on XUL overlays. :)

Changing to f+ given your reply.

Comment 12

[:Gijs (he/him)]

Comment on attachment 8673166 [details]
MozReview Request: Bug 1123760 - fix accessible labels of urlbar items, r?florian,f?MarcoZ

Bug 1123760 - fix accessible labels of urlbar items, r?florian,f?MarcoZ

Comment 13

[:Gijs (he/him)]

Comment on attachment 8673167 [details]
MozReview Request: Bug 1123760 - make autocomplete dropmarker in the urlbar actually work when activated through a11y APIs, r=surkov

Bug 1123760 - make autocomplete dropmarker in the urlbar actually work when activated through a11y APIs, r=surkov

Comment 14

[Florian Quèze [:florian]]

Comment on attachment 8673166 [details]
MozReview Request: Bug 1123760 - fix accessible labels of urlbar items, r?florian,f?MarcoZ

https://reviewboard.mozilla.org/r/21849/#review19755

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/4bd18dfe291a
https://hg.mozilla.org/integration/fx-team/rev/d91e585c5a37

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/4643b70405d9

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4bd18dfe291a
https://hg.mozilla.org/mozilla-central/rev/d91e585c5a37
https://hg.mozilla.org/mozilla-central/rev/4643b70405d9

Comment 18

[Dão Gottwald [:dao]]

(In reply to Marco Zehe (:MarcoZ) from comment #9)
> So I am wondering if, since they provide tooltip text anyway, we could make
> this easier and try to assign the aria-label the same content even for
> add-ons?

I remembered / would have expected that it was already the case that a11y could would fall back on using the tooltip as the accessible label when there's no (aria-)label set. Is this not the case?

(In reply to :Gijs Kruitbosch from comment #10)
> I'm also not sure how many of these icons have tooltips - our
> own icons (at least the ones next to the identity box) often don't.

We should fix that by setting tooltips where they're missing. They're useful for sighted users too.

> If you
> mean the type of icons at the end of the bar, like stop/go/reload, then I
> guess those do often have tooltips and so the a11y code could do that. I can
> file a followup for this.

Have you filed that bug yet?

Comment 19

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #18)
> (In reply to Marco Zehe (:MarcoZ) from comment #9)
> > So I am wondering if, since they provide tooltip text anyway, we could make
> > this easier and try to assign the aria-label the same content even for
> > add-ons?
> 
> I remembered / would have expected that it was already the case that a11y
> could would fall back on using the tooltip as the accessible label when
> there's no (aria-)label set. Is this not the case?

There is a label on the location bar, so that is used instead.

> (In reply to :Gijs Kruitbosch from comment #10)
> > I'm also not sure how many of these icons have tooltips - our
> > own icons (at least the ones next to the identity box) often don't.
> 
> We should fix that by setting tooltips where they're missing. They're useful
> for sighted users too.

I'm not sure I agree - the interaction between the panel and the tooltip seems like it would be likely to be poor.

> > If you
> > mean the type of icons at the end of the bar, like stop/go/reload, then I
> > guess those do often have tooltips and so the a11y code could do that. I can
> > file a followup for this.
> 
> Have you filed that bug yet?

Thanks for reminding me. I filed bug 1216478.

Comment 20

[Dão Gottwald [:dao]]

(In reply to :Gijs Kruitbosch from comment #19)
> (In reply to Dão Gottwald [:dao] from comment #18)
> > (In reply to Marco Zehe (:MarcoZ) from comment #9)
> > > So I am wondering if, since they provide tooltip text anyway, we could make
> > > this easier and try to assign the aria-label the same content even for
> > > add-ons?
> > 
> > I remembered / would have expected that it was already the case that a11y
> > could would fall back on using the tooltip as the accessible label when
> > there's no (aria-)label set. Is this not the case?
> 
> There is a label on the location bar, so that is used instead.

This doesn't really make sense, though (as demonstrated by this bug).

> > (In reply to :Gijs Kruitbosch from comment #10)
> > > I'm also not sure how many of these icons have tooltips - our
> > > own icons (at least the ones next to the identity box) often don't.
> > 
> > We should fix that by setting tooltips where they're missing. They're useful
> > for sighted users too.
> 
> I'm not sure I agree - the interaction between the panel and the tooltip
> seems like it would be likely to be poor.

Then we should get that fixed too. We already set tooltips on various other popup anchors (e.g. toolbar buttons on the navigation toolbar).

Comment 21

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #20)
> (In reply to :Gijs Kruitbosch from comment #19)
> > (In reply to Dão Gottwald [:dao] from comment #18)
> > > (In reply to Marco Zehe (:MarcoZ) from comment #9)
> > > > So I am wondering if, since they provide tooltip text anyway, we could make
> > > > this easier and try to assign the aria-label the same content even for
> > > > add-ons?
> > > 
> > > I remembered / would have expected that it was already the case that a11y
> > > could would fall back on using the tooltip as the accessible label when
> > > there's no (aria-)label set. Is this not the case?
> > 
> > There is a label on the location bar, so that is used instead.
> 
> This doesn't really make sense, though (as demonstrated by this bug).

Sure, I wasn't saying it was working *well*, but simply explaining what happens right now. :-)