[Webkit-unassigned] [Bug 118179] [ATK] Refactor code for translating offsets between WebCore a11y and ATK

[bugzilla-daemon at webkit.org]

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


Mario Sanchez Prada <mario at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #205700|0                           |1
        is obsolete|                            |
 Attachment #205700|review?                     |
               Flag|                            |
 Attachment #205897|                            |review?
               Flag|                            |




--- Comment #5 from Mario Sanchez Prada <mario at webkit.org>  2013-07-02 05:54:34 PST ---
Created an attachment (id=205897)
 --> (https://bugs.webkit.org/attachment.cgi?id=205897&action=review)
Patch proposal

Thanks for the review, Chris. Now attaching a new patch addressing the issues you pointed out, see my comments below...

(In reply to comment #2)
> (From update of attachment 205700 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=205700&action=review
> 
> > Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:473
> > +static int listItemMarkerLengthForObject(const AccessibilityObject* object)
> 
> this only applies to LTR, maybe the method should be named to indicate that?

I already thought of that, but in the end I found it better, or at least more clear, not to do it since it's an internal matter from the point of view of that function. From the caller you just care about "normalizing" the offset for list items since it's an special element from the point of view of ATK (as we flatten it out together with the marker).

> > Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:475
> > +    if (!object->isAccessibilityRenderObject())
> 
> you shouldn't have to check isAXRenderObject() -- renderer() is on every AXObject, so you just need to check if the renderer is 0 or not (which you're already doing)

Ok.

> > Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:482
> > +        String markerText = toRenderListItem(renderer)->markerTextWithSuffix();
> 
> this can be made into one line of code

Done.

> > Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:491
> > +    if (!object->isAccessibilityRenderObject() || !object->isListItem())
> 
> AXrenderObject check is not necessary

Ok.

> > Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:500
> > +    if (!coreObject || !coreObject->isAccessibilityRenderObject() || !coreObject->isListItem())
> 
> AXRenderObject check not necessary

Ok.

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