Bug 98379 – [GTK] accessibility/textarea-selected-text-range.html is failing

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: WebKitGTK

[Zan Dobersek]

accessibility/textarea-selected-text-range.html is failing on all GTK platforms.
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=accessibility%2Ftextarea-selected-text-range.html

[Joanmarie Diggs]

This one is failing because we need to implement our DRT's AccessibilityUIElement::selectedTextRange()

Putting this one on Piñeiro's plate.

[Alejandro Piñeiro]

(In reply to comment #1)
> This one is failing because we need to implement our DRT's AccessibilityUIElement::selectedTextRange()

After a quick look, this test is also using AccessibilityUIElement::setSelectedTextRange(), so I will implement both.

> 
> Putting this one on Piñeiro's plate.

Using WebkitGTK hackfest to catch it.

[Alejandro Piñeiro]

Created attachment 179091 [details]
Fixes the bug

In the end it was not only implement those DRT methods. Some of the ATK implementation methods were failing for the case of text areas (so making remove this test from the failure list more important). So in a short this bug:
  * Fixes ATK implementation case for text area
  * Update the atk unit test, in order to include text areas
  * Implement the DRT methods pointed by Joanie at comment 1
  * Remove the test from the failure list at TestExpectations

[Build Bot]

Comment on attachment 179091 [details]
Fixes the bug

Attachment 179091 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15304147

New failing tests:
fast/frames/sandboxed-iframe-attribute-parsing.html

[Alejandro Piñeiro]

(In reply to comment #4)
> (From update of attachment 179091 [details])
> Attachment 179091 [details] did not pass mac-ews (mac):
> Output: http://queues.webkit.org/results/15304147
> 
> New failing tests:
> fast/frames/sandboxed-iframe-attribute-parsing.html

This is strange, because on my patch I didn't change anything related to WebCore accessibility. All my changes were related to the gtk port, and in the same way, the test failing is not related at all with selection.

Chris what do you think about this? Can be a false positive? Meanwhile I will set the commit-queue again to '?'.

Btw, Chris if you finally come here to give your opinion, could you review the patch? Thanks

[Philippe Normand]

Chris, would you have time to review this patch?

[chris fleizach]

Comment on attachment 179091 [details]
Fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=179091&action=review

> LayoutTests/ChangeLog:8
> +        * platform/gtk/TestExpectations: Removed textare-selected-text-range.html failure expectation.

missing "a" at the end of text area in
"textare-selected"

> Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:510
> +    // siblings (e.g. multiline text) or text markers (e.g. list item's bullet)

I would rewrite this as

"The text content for TextAreas is already flattened so the startOffset is the same as the selectionStart.
Other elements must handle other sibling nodes and text markers."

> Tools/DumpRenderTree/gtk/AccessibilityUIElementGtk.cpp:806
> +    // WebKit atk implementation only supports one selection

use a period at the end of comments.
ATK should probably be capitalized

does anything support more than one selection? maybe the comment is not necessary

> Tools/DumpRenderTree/gtk/AccessibilityUIElementGtk.cpp:821
> +    atk_text_set_selection(ATK_TEXT(m_element), 0, location, location+length);

space before and after +

[chris fleizach]

(In reply to comment #7)
> (From update of attachment 179091 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=179091&action=review
> 
> > LayoutTests/ChangeLog:8
> > +        * platform/gtk/TestExpectations: Removed textare-selected-text-range.html failure expectation.
> 
> missing "a" at the end of text area in
> "textare-selected"
> 
> > Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:510
> > +    // siblings (e.g. multiline text) or text markers (e.g. list item's bullet)
> 
> I would rewrite this as
> 
> "The text content for TextAreas is already flattened so the startOffset is the same as the selectionStart.
> Other elements must handle other sibling nodes and text markers."
> 
> > Tools/DumpRenderTree/gtk/AccessibilityUIElementGtk.cpp:806
> > +    // WebKit atk implementation only supports one selection
> 
> use a period at the end of comments.
> ATK should probably be capitalized
> 
> does anything support more than one selection? maybe the comment is not necessary
> 
> > Tools/DumpRenderTree/gtk/AccessibilityUIElementGtk.cpp:821
> > +    atk_text_set_selection(ATK_TEXT(m_element), 0, location, location+length);
> 
> space before and after +
otherwise things seem ok

[Mario Sanchez Prada]

Comment on attachment 179091 [details]
Fixes the bug

Clearing r? so it does not appear in webkit.org/pending-review.

API, do you want to give it another try? If that's the case, bear in mind that getSelectionOffsetsForObject() has changed recently, so this original patch won't apply cleanly.

[Alejandro Piñeiro]

(In reply to comment #9)
> (From update of attachment 179091 [details])
> Clearing r? so it does not appear in webkit.org/pending-review.
> 
> API, do you want to give it another try? If that's the case, bear in mind that getSelectionOffsetsForObject() has changed recently, so this original patch won't apply cleanly.

Well, I also need to update the patch due Chris's review. In any case thanks for the update. FWIW, probably I would not be able to work on this until next webkitgtk hackfest, and seems a good bug to start with.

[Mario Sanchez Prada]

(In reply to comment #10)
> (In reply to comment #9)
> > (From update of attachment 179091 [details] [details])
> > Clearing r? so it does not appear in webkit.org/pending-review.
> > 
> > API, do you want to give it another try? If that's the case, bear in mind that getSelectionOffsetsForObject() has changed recently, so this original patch won't apply cleanly.
> 
> Well, I also need to update the patch due Chris's review. In any case thanks 
> for the update. FWIW, probably I would not be able to work on this until next 
> webkitgtk hackfest, and seems a good bug to start with.

It seems that in the end all we needed to fix this at this point was to provide the implementation for DRT and WKTR, which has been done as part of bug 112016.

So resolving.

*** This bug has been marked as a duplicate of bug 112016 ***