Bug 53436

Summary: [Gtk] atk_text_get_caret_offset fails for list items
Product: WebKit Reporter: Mario Sanchez Prada <mario>
Component: AccessibilityAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: apinheiro, jdiggs
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on: 53388    
Bug Blocks: 25531    
Attachments:
Description Flags
Patch proposal + unit test
mrobinson: review-
Patch proposal + unit tests xan.lopez: review+

Mario Sanchez Prada
Reported 2011-01-31 10:11:47 PST
Steps to reproduce: 1. Open the test case attached to bug 53388 (attachment 80582 [details]) in Epiphany. 2. Enable caret mode by pressing F7 and manually (with the mouse) place the caret in the first item, right after the 'First' word. 3. Using Accerciser, attempt to get the caret position within that list item, by: A. Selecting the corresponding object in Accerciser's tree of accessible objects B. Typing 'acc.queryText().caretOffset() Expected results: You should get 8 as the offset value Actual results: You get 5 as the offset value, basically because it's plainly ignoring the fact that a 3-characted marker ('1. ') is present for that list item.
Attachments
Patch proposal + unit test (5.15 KB, patch)
2011-01-31 10:44 PST, Mario Sanchez Prada 		mrobinson: review-
DetailsFormatted DiffDiff
Patch proposal + unit tests (5.81 KB, patch)
2011-02-01 09:15 PST, Mario Sanchez Prada 		xan.lopez: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mario Sanchez Prada
Comment 1 2011-01-31 10:44:10 PST
Created attachment 80659 [details] Patch proposal + unit test Attaching patch proposal + unit test (actually an update of another one)
Mario Sanchez Prada
Comment 2 2011-01-31 10:45:04 PST
Depending on bug 53388 as the unit test is an update of the code previously modified with the patch for that bug, so they should be applied in specific order
Martin Robinson
Comment 3 2011-01-31 15:32:01 PST
Comment on attachment 80659 [details] Patch proposal + unit test View in context: https://bugs.webkit.org/attachment.cgi?id=80659&action=review > Source/WebCore/ChangeLog:11 > + (webkit_accessible_text_get_caret_offset): Ajust the offset with Should be "Adjust" here. > Source/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1231 > + offset += g_utf8_strlen(markerText.utf8().data(), -1); Unless I'm mistaken, if you want the offset in bytes, you should use strlen(markerText.utf8().data()) if you want the offset in characters you can just use markerText.length().
Mario Sanchez Prada
Comment 4 2011-02-01 09:15:29 PST
Created attachment 80766 [details] Patch proposal + unit tests (In reply to comment #3) > (From update of attachment 80659 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=80659&action=review > > > Source/WebCore/ChangeLog:11 > > + (webkit_accessible_text_get_caret_offset): Ajust the offset with > > Should be "Adjust" here. Fixed > > Source/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1231 > > + offset += g_utf8_strlen(markerText.utf8().data(), -1); > > Unless I'm mistaken, if you want the offset in bytes, you should use strlen(markerText.utf8().data()) if you want the offset in characters you can just use markerText.length(). You're right. Thanks for noticing. Fixed either there and in webkit_accessible_text_set_caret_offset (and updated the ChangeLog)
Xan Lopez
Comment 5 2011-02-07 08:12:27 PST
Comment on attachment 80766 [details] Patch proposal + unit tests Looks good.
Martin Robinson
Comment 6 2011-02-07 08:14:56 PST
Comment on attachment 80766 [details] Patch proposal + unit tests View in context: https://bugs.webkit.org/attachment.cgi?id=80766&action=review > Source/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1231 > + // We need to adjust the offset for list items. Might want to clarify this a little bit to say "We need to adjust the offset for the list item marker."
Mario Sanchez Prada
Comment 7 2011-02-07 08:17:59 PST
(In reply to comment #6) > (From update of attachment 80766 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=80766&action=review > > > Source/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1231 > > + // We need to adjust the offset for list items. > > Might want to clarify this a little bit to say "We need to adjust the offset for the list item marker." :-) Yes, you're right, sorry. Specifying that the thing is related to the item marker adding some extra length to the exposed text will, definitely, be clearer. I'll change that when committing then. Thanks
Mario Sanchez Prada
Comment 8 2011-02-07 08:23:31 PST
Committed r77817: <http://trac.webkit.org/changeset/77817>
Note You need to log in before you can comment on or make changes to this bug.