If an ARIA textbox has aaa:secret="true" then it's an nsIAccessibleRole::ROLE_PASSWORD If an ARIA button has aaa:pressed at all, it's an nsIAccessibleRole::ROLE_TOGGLE_BUTTON If an ARIA button has aaa:haspopup it's an nsIAccesibleRole::ROLE_BUTTONMENU

I think (State(xpAccessible) & nsIAccessibleStates::STATE_HASPOPUP)\ and content->AttrValueIs(kNameSpaceID_WAIProperties, nsAccessibilityAtoms::haspopup, nsAccessibilityAtoms::_true, eCaseMatters) are not the same

I mean probably we should check states instead of attributes?

Also, please add a bug number about reorg of aria table and probably it makes sense to mention these code in that bug.

Surkov, it's not exactly the same but it works just as well and is a lot less expensive. GetState() is fairly expensive because of all the visibility checks etc.

Comment on attachment 274611 [details] [diff] [review] Support role dependent on role + ARIA properties in GetFinalRole() >+ if (content) { >+ if (content->HasAttr(kNameSpaceID_WAIProperties, nsAccessibilityAtoms::pressed)) { >+ // For aaa:pressed="false" or aaa:pressed="true" >+ // For simplicity, any pressed attribute indicates it's a toggle button >+ *aRole = nsIAccessibleRole::ROLE_TOGGLE_BUTTON; >+ } >+ if (content->AttrValueIs(kNameSpaceID_WAIProperties, nsAccessibilityAtoms::haspopup, >+ nsAccessibilityAtoms::_true, eCaseMatters)) { nit: I think it's better to use "else if"

Comment on attachment 274611 [details] [diff] [review] Support role dependent on role + ARIA properties in GetFinalRole() A polish bug for ARIA support. Very low risk.

I'll add the else to else if at checkin time.