[Webkit-unassigned] [Bug 73819] [Gtk] Dojo toggle buttons should expose ROLE_TOGGLE_BUTTON not ROLE_PUSH_BUTTON

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=73819





--- Comment #6 from Mario Sanchez Prada <msanchez at igalia.com>  2012-08-13 14:32:21 PST ---
(In reply to comment #5)
> > [...]
> > About the aria check, is there any real possibility of a button not being an aria button? There are some extra 
> > types of button, but looking at the code, if the aria button role is not a "plain" ButtonRole, it is also the 
> > case for the button role. In fact determineAccessibilityRole already checks aria stuff to determine the role. 
> > IMHO, having coreObject->roleValue as ButtonRole but the aria role as something different should not 
> > happen. Taking that into account, is it still required to add that check there?
> 
> Sorry I have just realized that you are talking about the new method created at AccessibilityRenderObject. 
> It is true that in this case it was already checked that the role is button, but if in the future someone wants 
> to add that method, that method should be safe by his own. I will also add those checks.

Yes, exactly: as you're implementing a new method isToggleButton() in AccessibilityObject / AccessibilityRenderObject, that should be agnostic to any other check you might do in the callee, and therefore able to determine whether it's a toggle button by itself.

Maybe you could do, to avoid checking things twice, change the code in the wrapper to somthing like this:

@@ -538,7 +543,7 @@ static AtkRole webkitAccessibleGetRole(AtkObject* object)
     if (coreObject->isPasswordField())
         return ATK_ROLE_PASSWORD_TEXT;
+ 
+   if (coreObject->isToggleButton())
+        return ATK_ROLE_PUSH_BUTTON;

     return atkRole(coreObject);
 }

... and you leave atkRole(AccessibilityRole role) untouched (thus, receiving an AccessibilityRole as it used to do). Perhaps it would make sense to rename atkRole() to atkRoleForCoreRole(), or something like that, to make clear the purpose of the method.

I personally think this way is cleaner. What do you think?

> Sorry for the noise.

No problem. Happy to help

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.