333833 - Image buttons in Firefox Pref dialog can't be grabbed by GOK

Status: RESOLVED FIXED

(Firefox :: Disability Access, defect)

Description

[Ginn Chen]

1) Open Firefox pref dialog.
2) Use GOK to UI grab.

Result:
Image buttons (General, Privacy, Content, Tabs, Downloads, Advanced) are not grabbed.

Comment 1

[Ginn Chen]

Attachment: patch

I don't know if it regresses Bug 302359.
To use XULRadioButton is more reasonable.

Comment 2

[Aaron Leventhal]

Comment on attachment 218393 [details] [diff] [review]
patch

Hmm we should never override GetFinalRole() -- we can override GetRole() but not GetFinalRole(). The idea of GetFinalRole() is that someoen can override something's role with a dhtml role in markup.

Hmm we should never override GetFinalRole() -- we can override GetRole() but not GetFinalRole(). The idea of GetFinalRole() is that someoen can override something's role with a dhtml role in markup.

People were confused by getting radio buttons there but list items made sense. Can't GOK grab list items?

Comment 3

[Ginn Chen]

OK, I found Bug 286145.

GOK can grab list items.
We're faking xulradiobutton/xulradiogroup to listitem/list here.
It has a problem.
I'll try to identify it.

Comment 4

[Ginn Chen]

Aaron,

1) The problem I found is nsXULRadioGroupAccessible doesn't implement nsXULSelecctableAccessible

2) After the fix, the pane is grabbed as a list, user has to click "list", and then click a pane button, then click "back", next click "UI Grab" again.
It introduces more clicks than the radiogroup/radiobutton solution.

Do you think it's OK?

Comment 5

[Aaron Leventhal]

Ginn, it's really a flaw in how we implement DHTML accessibility. If something has xhtml2:role="wairole:list", it should support the selection API. We need to think about how to do that.

It would be strange to call them radio buttons because radio buttons don't usually change what the content pane holds. I have seen lists do that. We used to call them radio buttons but it confused users.

The right way to do this might be to make it a real XUL listbox with listitems or whatever, instead of using XHTML2:role.

Comment 6

[Ginn Chen]

If more clicks are not a problem, I agree we can use a real XUL list box instead of XHTML2:role.
I think we should also give "prefpane" an accessible name instead of "list".

Comment 7

[Aaron Leventhal]

(In reply to comment #6)
> If more clicks are not a problem, I agree we can use a real XUL list box
> instead of XHTML2:role.
> I think we should also give "prefpane" an accessible name instead of "list".
> 

I would go for that solution if it doesn't look too difficult to get the same looks.

Another benefit would be that home/end would work as well as typing the first letter of each item to get there fast. After all, a list should work like a list as far as the keyboard goes.

Comment 8

[Ginn Chen]

I don't want to go so far, and it's not very possible to be done in Firefox 2.
Home/end benefits little, because we only have 6 buttons.

Comment 9

[Ginn Chen]

Attachment: patch v2

Comment 10

[Aaron Leventhal]

Comment on attachment 219134 [details] [diff] [review]
patch v2

I can go with that. It doesn't solve every similar problem we might have but it takes care of what you need for Firefox 2.

Comment 11

[[e-mail]]

I don't really know this panel; doesn't it work more like a set of tabs?

Comment 12

[Aaron Leventhal]

Neil, we're treating it like a list because one of the panels has its own set of real tabs. We don't want 2 rows of tabs active/visible at the same time.

Comment 13

[Aaron Leventhal]

One more comment -- 

XUL radio group should inherit from nsXULSelectableAccessible so that it supports nsIAccessibleSelection. So this fix should happen anyway, and just happens to help in this case.

We're dealing with Ben's ugly hack for these icons where he used styled radio buttons by using the DHTML accessibility namespaced attributes to say they're actually list items. However the impl underneath is still for the XUL Radio buttons because that's what the elements are. You don't want to know too much more about how we're doing this hack. Just trust me on this one. We'll have to make some real deep changes to do it in a better way, but that's in the future.

Comment 14

[Ginn Chen]

Checking in src/xul/nsXULFormControlAccessible.cpp;
/cvsroot/mozilla/accessible/src/xul/nsXULFormControlAccessible.cpp,v  <--  nsXULFormControlAccessible.cpp
new revision: 1.56; previous revision: 1.55
done
Checking in src/xul/nsXULFormControlAccessible.h;
/cvsroot/mozilla/accessible/src/xul/nsXULFormControlAccessible.h,v  <--  nsXULFormControlAccessible.h
new revision: 1.27; previous revision: 1.26
done
Checking in src/xul/nsXULSelectAccessible.cpp;
/cvsroot/mozilla/accessible/src/xul/nsXULSelectAccessible.cpp,v  <--  nsXULSelectAccessible.cpp
new revision: 1.31; previous revision: 1.30
done

Checking in src/xul/nsXULFormControlAccessible.cpp;
/cvsroot/mozilla/accessible/src/xul/nsXULFormControlAccessible.cpp,v  <--  nsXULFormControlAccessible.cpp
new revision: 1.52.2.4; previous revision: 1.52.2.3
done
Checking in src/xul/nsXULFormControlAccessible.h;
/cvsroot/mozilla/accessible/src/xul/nsXULFormControlAccessible.h,v  <--  nsXULFormControlAccessible.h
new revision: 1.25.4.2; previous revision: 1.25.4.1
done
Checking in src/xul/nsXULSelectAccessible.cpp;
/cvsroot/mozilla/accessible/src/xul/nsXULSelectAccessible.cpp,v  <--  nsXULSelectAccessible.cpp
new revision: 1.27.2.2; previous revision: 1.27.2.1
done