[Webkit-unassigned] [Bug 112015] [EFL] accessibility/textarea-line-for-index.html is failing

[bugzilla-daemon at webkit.org]

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


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

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #215648|review?                     |review-
               Flag|                            |




--- Comment #3 from Mario Sanchez Prada <mario at webkit.org>  2013-11-01 03:29:21 PST ---
(From update of attachment 215648)
View in context: https://bugs.webkit.org/attachment.cgi?id=215648&action=review

> Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:995
> +    if (!ATK_IS_TEXT(m_element))
> +        return -1;

We normally put this check at the beginning of the function. Could you please move it there?

> Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:998
> +    if (index > atk_text_get_character_count(ATK_TEXT(m_element)))
> +        return -1;

Maybe you could join this check with the index < 0 in a single if condition.

Also, you probably want to check index >= atk_text_get_character_count(ATK_TEXT(m_element)) here (opposite to just >)

> Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:1003
> +    GOwnPtr<gchar> text(atk_text_get_text(ATK_TEXT(m_element), 0, index));
> +
> +    int lineNo = 0;
> +

You don't need these two extra blank lines here

> Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:1006
> +    for (gchar* offset = text.get(); *offset; ++offset)
> +        if (*offset == '\n')
> +            ++lineNo;

Please add braces to this for loop. Even if it only has the if statement inside, it's multiline content and adding braces will make it clearer

> Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:1035
>  int AccessibilityUIElement::lineForIndex(int index)

Same considerations here.

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