373532 - Move roles and states out of nsIAccessible

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

Move roles form nsIAccessible into nsIAccessibleStates like it do IA2. This will allow to keep nsIAccessible more cleaner. Also, I'd like to keep roles documented. Now the unique way to define, why this gecko role is, is to look through gecko code or platform role maps.

Comment 1

[alexander :surkov (:asurkov)]

Aaron, will you approve this?

Comment 2

[Aaron Leventhal]

> Move roles form nsIAccessible into nsIAccessibleStates
Do you mean move roles from nsIAccesible to nsIAccessibleRoles?
And then maybe states into nsIAccessibleStates?

Comment 3

[alexander :surkov (:asurkov)]

(In reply to comment #2)
> > Move roles form nsIAccessible into nsIAccessibleStates
> Do you mean move roles from nsIAccesible to nsIAccessibleRoles?
> And then maybe states into nsIAccessibleStates?
> 

Exactly :)

Comment 4

[Aaron Leventhal]

That's great. Go ahead.

Comment 5

[alexander :surkov (:asurkov)]

Attachment: role patch

Comment 6

[Aaron Leventhal]

What about having nsIAccessible inherit from nsIAccessibleRole, just to avoid all the CVS changes?

Comment 7

[Aaron Leventhal]

Comment on attachment 258576 [details] [diff] [review]
role patch

Get it in soon so we can avoid conflicts with existing work. You can decide about whether to inherit or not, in order to avoid so many changes (nsIAccessible to nsIAccessibleRole).

Comment 8

[alexander :surkov (:asurkov)]

(In reply to comment #6)
> What about having nsIAccessible inherit from nsIAccessibleRole, just to avoid
> all the CVS changes?
> 

It looks not total correct. Accessible object is not accessible role. Also I'd like to be closer to IA2. I checked in patch as is with following up linux's bustage fixes.

Comment 9

[alexander :surkov (:asurkov)]

(In reply to comment #8)
> (In reply to comment #6)
> > What about having nsIAccessible inherit from nsIAccessibleRole, just to avoid
> > all the CVS changes?
> > 
> 
> It looks not total correct. Accessible object is not accessible role. Also I'd
> like to be closer to IA2. I checked in patch as is with following up linux's
> bustage fixes.
> 

I hope our successors will estimates at their true worth :)

Comment 10

[alexander :surkov (:asurkov)]

Attachment: state patch

Aaron, is states are used outside of accessible module?

Comment 11

[Aaron Leventhal]

They can be useful for Javascript extensions like Fire Vox, a screen reading extension.

Comment 12

[alexander :surkov (:asurkov)]

Are they in mozilla tree?

Comment 13

[Aaron Leventhal]

No they aren't.

Comment 14

[alexander :surkov (:asurkov)]

Ok, I just afraid to fire the tree like I did with role patch :)

Comment 15

[Aaron Leventhal]

I wish there was a way to import the namespace so we didn't need to be so verbose.

Also, why nsIAccessibleState_s_ plural when nsIAccessibleRole is singular?

Comment 16

[alexander :surkov (:asurkov)]

(In reply to comment #15)
> I wish there was a way to import the namespace so we didn't need to be so
> verbose.

Is there a way?

> Also, why nsIAccessibleState_s_ plural when nsIAccessibleRole is singular?
> 

That's how in IA2.

Comment 17

[Aaron Leventhal]

Comment on attachment 258653 [details] [diff] [review]
state patch

Is there any reason for me to check this patch over carefully? It's just moving stuff into a new file and doing a global replace, correct?

Comment 18

[Aaron Leventhal]

I don't remember, is there anything special we have to do to make the new IDL available to scripts? Probably not.

Comment 19

[alexander :surkov (:asurkov)]

(In reply to comment #17)
> (From update of attachment 258653 [details] [diff] [review])
> Is there any reason for me to check this patch over carefully? It's just moving
> stuff into a new file and doing a global replace, correct?
> 

Not sure the throughout check is needed. You're right it's moving stuff and replacing. The changes are compiled on Windows.

Comment 20

[alexander :surkov (:asurkov)]

(In reply to comment #18)
> I don't remember, is there anything special we have to do to make the new IDL
> available to scripts? Probably not.
> 

I think we should do nothing I guess since it is scriptable.

Comment 21

[Aaron Leventhal]

Comment on attachment 258653 [details] [diff] [review]
state patch

Okay, eventually we'll get you set up with a Linux machine too so you can test patches on there as well.

Comment 22

[alexander :surkov (:asurkov)]

(In reply to comment #21)
> (From update of attachment 258653 [details] [diff] [review])
> Okay, eventually we'll get you set up with a Linux machine too so you can test
> patches on there as well.
> 

Okay. I hope there is no mac in the list? :)

checked in.

Comment 23

[Stuart Morgan]

This broke accessible/src/mac/mozAccessible.mm; that file at least needs the
new states header and fixes to the references to nsIAccessible::STATE_*

Comment 24

[Aaron Leventhal]

Unfortunately Surkov is in Irkutsk, so he won't be awake for a while.

Comment 25

[alexander :surkov (:asurkov)]

Attachment: state fix patch

Unfortunally I can't test this patch. Stuart, can you ensure that all is fine?

Comment 26

[Joel Craig]

Stuart, this works for me. Successful build, and the limited functionality we had with voiceover is working. Two thumbs up! Thanks Alexander.

Comment 27

[alexander :surkov (:asurkov)]

Comment on attachment 258750 [details] [diff] [review]
state fix patch

checked in