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
This one is failing because we need to implement our DRT's AccessibilityUIElement::selectedTextRange() Putting this one on Piñeiro's plate.
(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.
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
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
(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
Chris, would you have time to review this patch?
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 +
(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
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.
(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.
(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 ***