:Gijs (he/him) Assignee
	Description • 9 years ago

The binding here: http://hg.mozilla.org/mozilla-central/annotate/c2df1daf74c3/toolkit/content/widgets/textbox.xml#l315

has a <xul:deck> which has a <xul:image>. That's exposed through MSAA as a push button, even when it isn't actually in use as a button (when the searchbutton attribute is not present).

It seems the binding sets aria-autocomplete=list if the searchbutton attribute is not present; Surkov, can you clarify why the deck/image gets exposed as a button in this case? We can hopefully/probably workaround on the toolkit side by setting role=presentation in the constructor and the searchButton setter (see the existing shuffling of attributes in the same) but I'd like to better understand why we even expose this the way we do. Is it just because of the onclick attribute?

	:Gijs (he/him) Assignee
	Comment 1 • 9 years ago

Oh, and I forgot: for a practical example, turn off in-content prefs in about:config (browser.preferences.inContent and browser.preferences.instantApply to false) and open the preferences, then check out the search input at the top of the "Applications" pane.

	alexander :surkov (:asurkov)
	Comment 2 • 9 years ago

(In reply to :Gijs Kruitbosch from comment #0)
> The binding here:
> http://hg.mozilla.org/mozilla-central/annotate/c2df1daf74c3/toolkit/content/
> widgets/textbox.xml#l315
> 
> has a <xul:deck> which has a <xul:image>. That's exposed through MSAA as a
> push button, even when it isn't actually in use as a button (when the
> searchbutton attribute is not present).
> 
> It seems the binding sets aria-autocomplete=list if the searchbutton
> attribute is not present; Surkov, can you clarify why the deck/image gets
> exposed as a button in this case?

not offhand, it's worth to check what role this image has

> We can hopefully/probably workaround on
> the toolkit side by setting role=presentation in the constructor and the
> searchButton setter (see the existing shuffling of attributes in the same)
> but I'd like to better understand why we even expose this the way we do. Is
> it just because of the onclick attribute?

onclick should provide accessible action but it shouldn't affect on role, even when mapped to MSAA. Did you check xp role (via DOMInspector for example)?

	:Gijs (he/him) Assignee
	Comment 3 • 9 years ago

(In reply to alexander :surkov from comment #2)
> (In reply to :Gijs Kruitbosch from comment #0)
> > The binding here:
> > http://hg.mozilla.org/mozilla-central/annotate/c2df1daf74c3/toolkit/content/
> > widgets/textbox.xml#l315
> > 
> > has a <xul:deck> which has a <xul:image>. That's exposed through MSAA as a
> > push button, even when it isn't actually in use as a button (when the
> > searchbutton attribute is not present).
> > 
> > It seems the binding sets aria-autocomplete=list if the searchbutton
> > attribute is not present; Surkov, can you clarify why the deck/image gets
> > exposed as a button in this case?
> 
> not offhand, it's worth to check what role this image has

None, it's in anon content and has no role set, AFAICT.
 
> > We can hopefully/probably workaround on
> > the toolkit side by setting role=presentation in the constructor and the
> > searchButton setter (see the existing shuffling of attributes in the same)
> > but I'd like to better understand why we even expose this the way we do. Is
> > it just because of the onclick attribute?
> 
> onclick should provide accessible action but it shouldn't affect on role,
> even when mapped to MSAA. Did you check xp role (via DOMInspector for
> example)?

It doesn't show up in the a11y tree in DOMI. Is this what you mean? If not, what exactly do you mean? :-)

	alexander :surkov (:asurkov)
	Comment 4 • 9 years ago

(In reply to :Gijs Kruitbosch from comment #3)

> > not offhand, it's worth to check what role this image has
> 
> None, it's in anon content and has no role set, AFAICT.

XBL bindings provides own roles on xbl:content element. So it that's image binding then it has role="xul:image", see http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/general.xml#154

>  
> > > We can hopefully/probably workaround on
> > > the toolkit side by setting role=presentation in the constructor and the
> > > searchButton setter (see the existing shuffling of attributes in the same)
> > > but I'd like to better understand why we even expose this the way we do. Is
> > > it just because of the onclick attribute?
> > 
> > onclick should provide accessible action but it shouldn't affect on role,
> > even when mapped to MSAA. Did you check xp role (via DOMInspector for
> > example)?
> 
> It doesn't show up in the a11y tree in DOMI. Is this what you mean? If not,
> what exactly do you mean? :-)

it should, otherwise you cannot see it in MSAA tree as well. or I miss your point

	:Gijs (he/him) Assignee
	Comment 5 • 9 years ago

(In reply to alexander :surkov from comment #4)
> > > > We can hopefully/probably workaround on
> > > > the toolkit side by setting role=presentation in the constructor and the
> > > > searchButton setter (see the existing shuffling of attributes in the same)
> > > > but I'd like to better understand why we even expose this the way we do. Is
> > > > it just because of the onclick attribute?
> > > 
> > > onclick should provide accessible action but it shouldn't affect on role,
> > > even when mapped to MSAA. Did you check xp role (via DOMInspector for
> > > example)?
> > 
> > It doesn't show up in the a11y tree in DOMI. Is this what you mean? If not,
> > what exactly do you mean? :-)
> 
> it should, otherwise you cannot see it in MSAA tree as well. or I miss your
> point

Sorry, it seems like the a11y tree in DOMI is still different on OS X vs. Windows. It does show up in DOMI on Windows, and has role "pushbutton".

How is that possible if the xul:image bit should give it role "image"? :-\

	alexander :surkov (:asurkov)
	Comment 6 • 9 years ago

(In reply to :Gijs Kruitbosch from comment #5)
> Sorry, it seems like the a11y tree in DOMI is still different on OS X vs.
> Windows. It does show up in DOMI on Windows, and has role "pushbutton".

it should be same on OS X

> How is that possible if the xul:image bit should give it role "image"? :-\

I would check bindings chain to see what role it gets from. I just don't see in a11y code how image can be transformed to button, so I blamed XBL bindings.

	:Gijs (he/him) Assignee
	Comment 7 • 9 years ago

(In reply to alexander :surkov from comment #6)
> (In reply to :Gijs Kruitbosch from comment #5)
> > Sorry, it seems like the a11y tree in DOMI is still different on OS X vs.
> > Windows. It does show up in DOMI on Windows, and has role "pushbutton".
> 
> it should be same on OS X

It isn't. On OS X I see an "entry", with name "Search", node name "html:input, and a menupopup with name "Applications" and node name "xul:menupopup", in a "section" named "Search", with node name "textbox".

On Windows the situation is the same *except* it says that the pushbutton with a blank name and node name "xul:image" is there.

The menupopup, fwiw, seems to be the context menu - I can't see any other menupopup.

> > How is that possible if the xul:image bit should give it role "image"? :-\
> 
> I would check bindings chain to see what role it gets from. I just don't see
> in a11y code how image can be transformed to button, so I blamed XBL
> bindings.

There aren't any other bindings for the image in DOMI besides the image binding from general.xml which you linked to.

I'm really sorry, but can you investigate again? I tried looking through the accessible/xul code with a debugger, and while I can see that it's exposing the button role for the image, it's not clear to me how to figure out when the accessible is being created and how to catch that happening for the image (rather than all the other content in the pref window) and thereby being able to tell *why* it's getting created.

	alexander :surkov (:asurkov)
	Comment 8 • 9 years ago

apparently xul:image having onclick is exposed as a button http://mxr.mozilla.org/mozilla-central/source/accessible/base/nsAccessibilityService.cpp#1238

do you think the search image shouldn't be a button? (the cross button is markup equivalent but it acts as a button).

	:Gijs (he/him) Assignee
	Comment 9 • 9 years ago

(In reply to alexander :surkov from comment #8)
> apparently xul:image having onclick is exposed as a button
> http://mxr.mozilla.org/mozilla-central/source/accessible/base/
> nsAccessibilityService.cpp#1238

Ah, so my guess in comment #0 was actually correct!

> do you think the search image shouldn't be a button? (the cross button is
> markup equivalent but it acts as a button).

AFAICT the iconClick method just no-ops right now in the case of the preferences window.

We might be able to fix this by fixing the binding to not hardcode the onclick attribute but set it dynamically... not sure how hard this is. I'll have a look.

	:Gijs (he/him) Assignee
	Comment 10 • 9 years ago

The only case where we set searchbutton=true outside of tests seems to be the addon manager. For the clear button, I thought that the label should probably always be 'clear', so I've just stuck it in the binding. I've labeled the search button in the add-ons manager using extensions.dtd. Finally, I had to do a little jig with the actual click handler because we don't want it to stop focusing the input, but equally, it doesn't need to be accessible as a button in that case (because tools like that can just focus the input themselves) or have a label. I'm not a great fan of the current solution, but considering we can't override XBL-provided roles, this seems like the most straightforward solution. Try: remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=d3eb9587c15e

	:Gijs (he/him) Assignee
	Comment 11 • 9 years ago

Joyous test failures. Fix included in:

remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=94b1d8ef9b02

Let's see if this goes better.

	:Gijs (he/him) Assignee
	Comment 12 • 9 years ago

remote:   https://hg.mozilla.org/integration/fx-team/rev/db26d58e22b6

	Marco Zehe (:MarcoZ)
	Comment 13 • 9 years ago

(In reply to :Gijs Kruitbosch from comment #11)
> Joyous test failures.

I was going to ask... I distinctly remember that we test for the exposed widget tree for this one. And because the patch changed it, we flag it as such. ;) Thanks for fixing the tests and adding one for the other case!

	Ryan VanderMeulen [:RyanVM]
	Comment 14 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/db26d58e22b6