Bug 53388 - [Gtk] atk_text_set_caret_offset fails for list items
Summary: [Gtk] atk_text_set_caret_offset fails for list items
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
Keywords: Gtk
Depends on: 53389
Blocks: 25531 53436
  Show dependency treegraph
Reported: 2011-01-29 17:35 PST by Joanmarie Diggs
Modified: 2011-02-01 01:58 PST (History)
2 users (show)

See Also:

test case (142 bytes, text/html)
2011-01-29 17:35 PST, Joanmarie Diggs
no flags Details
Patch proposal + unit test (4.64 KB, patch)
2011-01-31 10:24 PST, Mario Sanchez Prada
mrobinson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joanmarie Diggs 2011-01-29 17:35:28 PST
Created attachment 80582 [details]
test case

Steps to reproduce:

1. Open the attached test case in Epiphany.

2. Using Accerciser, attempt to set the caret position within the paragraph and within the first list item, by:

A. Selecting the corresponding object in Accerciser's tree of accessible objects

B. Typing 'acc.queryText().setCaretOffset(n) - where 'n' is the desired offset.

Expected results: You'd be able to set the caret offset for both objects.

Actual results: You can set the caret offset for only the paragraph; not the list item.

Related aside: In both cases, True is returned suggesting that the offset was successfully positioned. I shall open a separate bug requesting a more accurate return value.

Impact: Orca has 'structural navigation' commands which include the ability to navigate amongst HTML elements ('p' for paragraphs, 't' for tables, 'l' for lists, etc.). A typical use case is to locate a desired section of the page via structural navigation and then use caret navigation to read or select the text in that section. Because we cannot set the caret within a list item, this navigation/selection technique fails for lists and their items.
Comment 1 Mario Sanchez Prada 2011-01-31 03:11:17 PST
Hmmm... I think we're gonna have an extra problem in here, sorry :-(

As you probably remember, we in the past did a change to properly expose list items markers ('1. ', '* ' ...) as part of the list item text through the AtkText interface.

However, that exposure was made in a very artificial way, letting the Atk wrapper dynamically add that 'prefix' (or suffix, for Right-To-Left styles) to the item's text when requested through atk_text_get_text(). Also some additional tweaks were done to properly report the size of the text, the position of the caret and so on.

This hack was needed to workaround that WebKit doesn't represent internally item's markers as part of the items, but as child objects for them, and that is the expected behavior in other ports (e.g. mac) so we couldn't change that in WebCore.

And now that became an issue again because there's another difference: in WebKit list item markers do not allow to put the caret inside of them, so you never get the cursor, for instance, in the middle of '1' and '.' for an item with text '1. This is an item'.

What does this have to do with this bug? Well, actually the bug itself is about to being able to place the cursor in the item's text, not in the marker, so it's not a 1-1 relationship, but related anyway because I wonder what to do in this case:

Suppose the following list item:

  1. A first item

If I position accerciser in the item's accessible object and want to set the caret right before 'first', which one of these would be the right one?:

  (A) atk_text_set_caret_offset(obj, 5)
  (B) atk_text_set_caret_offset(obj, 2)

I guess the right one would be (A), since the text exposed includes the marker, but then I wonder what should be the expected behaviour if I did atk_text_set_caret_offset(obj, i) with 0 <= i < 3, since the cursor can't be placed in the marker.

Probably the best solution would include making it possible to place the caret in the list item's marker, but not sure how feasible that is at this point, so sharing these thoughts now to see if we can think of something better.

As I said, this is not directly related to the bug reported, but highly related IMHO (and probably needing another bug), but I think it's worth telling anyway. Sorry for the long comment
Comment 2 Mario Sanchez Prada 2011-01-31 03:39:00 PST
Btw, the patch to 'fix' the issue reported in this bug would be as simple as this:

--- a/Source/WebCore/accessibility/gtk/AccessibilityObjectAtk.cpp
+++ b/Source/WebCore/accessibility/gtk/AccessibilityObjectAtk.cpp
@@ -105,7 +105,7 @@ void AccessibilityObject::setWrapper(AccessibilityObjectWrapper* wrapper)
 bool AccessibilityObject::allowsTextRanges() const
-    return isTextControl() || isWebArea() || isGroup() || isLink() || isHeading();
+    return isTextControl() || isWebArea() || isGroup() || isLink() || isHeading() || isListItem();

But I said 'fix' because that would mean that atk_text_set_caret_offset(obj, 0), where obj is the a11y object for the list item, would place the caret at the beginning of the item's text, right after the marker, and I doubt whether that's the expected behaviour.

Guess I could workaround it this so atk_text_set_caret_offset() would place the caret at the beginning of the items text for 0 <= offset <= markerText.length, but not sure whether that would be correct.

Perhaps the best thing to do would be to file a new bug for the issue with the marker and get this one fixed either by applying the diff above or even by using the workaround suggested to 'fix' the issue with the offsets.

What do you think?
Comment 3 Mario Sanchez Prada 2011-01-31 04:15:07 PST
Argh! Just read this comment in bug 53389, which partially answers my doubts about the list item doubt :-)


...and at the same time you answered my doubt about what to do if we try to set the caret in the marker: return FALSE!

Awesome, Joanie... thanks!
Comment 4 Mario Sanchez Prada 2011-01-31 10:24:45 PST
Created attachment 80655 [details]
Patch proposal + unit test

Attached patch for fixing this bug, updating and already present test to check this additional behaviour.
Comment 5 Mario Sanchez Prada 2011-02-01 01:58:50 PST
Committed r77235: <http://trac.webkit.org/changeset/77235>