[Webkit-unassigned] [Bug 118969] [ATK] Implement tables-related attributesOf*() functions for AccessibilityUIElement

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Nov 5 07:47:03 PST 2013


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





--- Comment #7 from Krzysztof Czech <k.czech at samsung.com>  2013-11-05 07:45:49 PST ---
> > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:382
> > +static void getRowHeaders(AtkObject* accessible, Vector<AccessibilityUIElement>& rowHeaders)
> 
> This function only works for AtkTable, so I think you can already expect that type as the first parameter and avoid the casts below. Just make sure you check and cast properly to AtkTable at the caller points
Good point. Done.
> 
> Also, I think this is one of those cases where you can use the "move semantics" feature of C++11, which is already supported by Vector through its move contructor Vector(Vector&&). See https://www.webkit.org/blog/3172/webkit-and-cxx11/

Very good idea. Done.
> 
> > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:384
> > +    for (int row = 0; row < atk_table_get_n_rows(ATK_TABLE(accessible)); ++row)
> 
> I think it's better to put the result of get_n_rows() in a variable and use it in the limit condition in the loop, to avoid too many calls to this function.
> 
You are right. Done.
> > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:388
> > +static void getColumnHeaders(AtkObject* accessible, Vector<AccessibilityUIElement>& columnHeaders)
> 
> Same considerations than before here (and in the rest of the patch)
> 
> > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:400
> > +    for (int row = 0; row < atk_table_get_n_rows(ATK_TABLE(accessibilityObject)); ++row)
> > +        for (int column = 0; column < atk_table_get_n_columns(ATK_TABLE(accessibilityObject)); ++column)
> > +            visibleCells.append(uiElement->cellForColumnAndRow(column, row));
> 
> Please use braces for the outermost loop. Also, you can store the get_n_rows() and get_n_columns() in variables too here

Right, Done.
> 
> > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:968
> > +    Vector<AccessibilityUIElement> columnHeaders;
> > +    getColumnHeaders(m_element, columnHeaders);
> 
> If the C++11 thing works, this will become one line :)
Done.
> 
> > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:979
> > +    Vector<AccessibilityUIElement> rowHeaders;
> > +    getRowHeaders(m_element, rowHeaders);
> 
> Ditto.
Done.
> 
> > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:1002
> > +    Vector<AccessibilityUIElement> visibleCells;
> > +    getVisibleCells(this, visibleCells);
> 
> Ditto.
Done.

Thanks.

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



More information about the webkit-unassigned mailing list