53388 2011-01-29 17:35:28 -0800 [Gtk] atk_text_set_caret_offset fails for list items 2011-02-01 01:58:50 -0800 1 1 1 Unclassified WebKit Accessibility 528+ (Nightly build) PC Linux RESOLVED FIXED Gtk P2 Normal --- 53389 25531 53436 0 jdiggs webkit-unassigned apinheiro mario oldest_to_newest 342173 0 80582 jdiggs 2011-01-29 17:35:28 -0800 Created attachment 80582 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. 342516 1 mario 2011-01-31 03:11:17 -0800 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 342521 2 mario 2011-01-31 03:39:00 -0800 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? 342528 3 mario 2011-01-31 04:15:07 -0800 Argh! Just read this comment in bug 53389, which partially answers my doubts about the list item doubt :-) https://bugs.webkit.org/show_bug.cgi?id=53389#c1 ...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! 342657 4 80655 mario 2011-01-31 10:24:45 -0800 Created attachment 80655 Patch proposal + unit test Attached patch for fixing this bug, updating and already present test to check this additional behaviour. 343091 5 mario 2011-02-01 01:58:50 -0800 Committed r77235: <http://trac.webkit.org/changeset/77235> 80582 2011-01-29 17:35:28 -0800 2011-01-29 17:35:28 -0800 test case test.html text/html 142 jdiggs PGh0bWw+CjxoZWFkPgo8dGl0bGU+VGVzdDwvdGl0bGU+CjwvaGVhZD4KPGJvZHk+CjxwPgpIZXJl J3MgYSBsaXN0Ogo8L3A+CjxvbD4KPGxpPkZpcnN0IGl0ZW08L2xpPgo8bGk+U2Vjb25kIGl0ZW08 L2xpPgo8L29sPgo8L2JvZHk+CjwvaHRtbD4KCg== 80655 2011-01-31 10:24:45 -0800 2011-01-31 15:28:41 -0800 Patch proposal + unit test 0001-2011-01-31-Mario-Sanchez-Prada-msanchez-igalia.com.patch text/plain 4748 mario RnJvbSBmMmI2NjUwNzJhM2YxMDc0ZjMyMTQwZWYyMGNjYjQwMjE2YjM3NmQ2IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBNYXJpbyBTYW5jaGV6IFByYWRhIDxtc2FuY2hlekBpZ2FsaWEu Y29tPgpEYXRlOiBNb24sIDMxIEphbiAyMDExIDE5OjE5OjQ1ICswMTAwClN1YmplY3Q6IFtQQVRD SF0gMjAxMS0wMS0zMSAgTWFyaW8gU2FuY2hleiBQcmFkYSAgPG1zYW5jaGV6QGlnYWxpYS5jb20+ CgogICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgoKICAgICAgICBbR3RrXSBhdGtf dGV4dF9zZXRfY2FyZXRfb2Zmc2V0IGZhaWxzIGZvciBsaXN0IGl0ZW1zCiAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTUzMzg4CgogICAgICAgIEFsbG93IHVz aW5nIHRleHQgcmFuZ2VzIGFjcm9zcyBsaXN0IGl0ZW1zLgoKICAgICAgICAqIGFjY2Vzc2liaWxp dHkvZ3RrL0FjY2Vzc2liaWxpdHlPYmplY3RBdGsuY3BwOgogICAgICAgIChXZWJDb3JlOjpBY2Nl c3NpYmlsaXR5T2JqZWN0OjphbGxvd3NUZXh0UmFuZ2VzKTogQWRkIGxpc3QgaXRlbXMKICAgICAg ICB0byB0aGUgbGlzdCBvZiBhY2Nlc3NpYmlsaXR5IG9iamVjdHMgc3VwcG9ydGluZyB0ZXh0IHJh bmdlcy4KCjIwMTEtMDEtMzEgIE1hcmlvIFNhbmNoZXogUHJhZGEgIDxtc2FuY2hlekBpZ2FsaWEu Y29tPgoKICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KCiAgICAgICAgW0d0a10g YXRrX3RleHRfc2V0X2NhcmV0X29mZnNldCBmYWlscyBmb3IgbGlzdCBpdGVtcwogICAgICAgIGh0 dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD01MzM4OAoKICAgICAgICBVcGRh dGUgdW5pdCB0ZXN0IHRvIGNoZWNrIHRoZSBmaXggZm9yIHRoaXMgYnVnLgoKICAgICAgICAqIHRl c3RzL3Rlc3RhdGsuYzoKICAgICAgICAodGVzdFdlYmtpdEF0a0NhcmV0T2Zmc2V0cyk6IEVuc3Vy ZSBzZXR0aW5nIHRoZSBjYXJldCBpbiBhIGxpc3QKICAgICAgICBpdGVtIHRocm91Z2ggdGhlIEF0 a1RleHQgaW50ZXJmYWNlIGlzIHBvc3NpYmxlLgotLS0KIFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxv ZyAgICAgICAgICAgICAgICAgICAgICAgICAgIHwgICAxMyArKysrKysrKysrKysrCiAuLi4vYWNj ZXNzaWJpbGl0eS9ndGsvQWNjZXNzaWJpbGl0eU9iamVjdEF0ay5jcHAgICB8ICAgIDIgKy0KIFNv dXJjZS9XZWJLaXQvZ3RrL0NoYW5nZUxvZyAgICAgICAgICAgICAgICAgICAgICAgIHwgICAxMyAr KysrKysrKysrKysrCiBTb3VyY2UvV2ViS2l0L2d0ay90ZXN0cy90ZXN0YXRrLmMgICAgICAgICAg ICAgICAgICB8ICAgMTAgKysrKysrKy0tLQogNCBmaWxlcyBjaGFuZ2VkLCAzNCBpbnNlcnRpb25z KCspLCA0IGRlbGV0aW9ucygtKQoKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxv ZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCA0Y2YzYjgyLi5iMGFhZmJhIDEwMDY0 NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hh bmdlTG9nCkBAIC0yLDYgKzIsMTkgQEAKIAogICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KIAorICAgICAgICBbR3RrXSBhdGtfdGV4dF9zZXRfY2FyZXRfb2Zmc2V0IGZhaWxzIGZv ciBsaXN0IGl0ZW1zCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD01MzM4OAorCisgICAgICAgIEFsbG93IHVzaW5nIHRleHQgcmFuZ2VzIGFjcm9zcyBsaXN0 IGl0ZW1zLgorCisgICAgICAgICogYWNjZXNzaWJpbGl0eS9ndGsvQWNjZXNzaWJpbGl0eU9iamVj dEF0ay5jcHA6CisgICAgICAgIChXZWJDb3JlOjpBY2Nlc3NpYmlsaXR5T2JqZWN0OjphbGxvd3NU ZXh0UmFuZ2VzKTogQWRkIGxpc3QgaXRlbXMKKyAgICAgICAgdG8gdGhlIGxpc3Qgb2YgYWNjZXNz aWJpbGl0eSBvYmplY3RzIHN1cHBvcnRpbmcgdGV4dCByYW5nZXMuCisKKzIwMTEtMDEtMzEgIE1h cmlvIFNhbmNoZXogUHJhZGEgIDxtc2FuY2hlekBpZ2FsaWEuY29tPgorCisgICAgICAgIFJldmll d2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCiAgICAgICAgIFtHdGtdIGF0a190ZXh0X3NldF9jYXJl dF9vZmZzZXQgcmV0dXJucyBUcnVlIGV2ZW4gd2hlbiBpdCBpcyB1bnN1Y2Nlc3NmdWwKICAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTUzMzg5CiAKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL2FjY2Vzc2liaWxpdHkvZ3RrL0FjY2Vzc2liaWxpdHlPYmpl Y3RBdGsuY3BwIGIvU291cmNlL1dlYkNvcmUvYWNjZXNzaWJpbGl0eS9ndGsvQWNjZXNzaWJpbGl0 eU9iamVjdEF0ay5jcHAKaW5kZXggOTc3MmI0My4uYzhjMTk1MSAxMDA2NDQKLS0tIGEvU291cmNl L1dlYkNvcmUvYWNjZXNzaWJpbGl0eS9ndGsvQWNjZXNzaWJpbGl0eU9iamVjdEF0ay5jcHAKKysr IGIvU291cmNlL1dlYkNvcmUvYWNjZXNzaWJpbGl0eS9ndGsvQWNjZXNzaWJpbGl0eU9iamVjdEF0 ay5jcHAKQEAgLTEwNSw3ICsxMDUsNyBAQCB2b2lkIEFjY2Vzc2liaWxpdHlPYmplY3Q6OnNldFdy YXBwZXIoQWNjZXNzaWJpbGl0eU9iamVjdFdyYXBwZXIqIHdyYXBwZXIpCiAKIGJvb2wgQWNjZXNz aWJpbGl0eU9iamVjdDo6YWxsb3dzVGV4dFJhbmdlcygpIGNvbnN0CiB7Ci0gICAgcmV0dXJuIGlz VGV4dENvbnRyb2woKSB8fCBpc1dlYkFyZWEoKSB8fCBpc0dyb3VwKCkgfHwgaXNMaW5rKCkgfHwg aXNIZWFkaW5nKCk7CisgICAgcmV0dXJuIGlzVGV4dENvbnRyb2woKSB8fCBpc1dlYkFyZWEoKSB8 fCBpc0dyb3VwKCkgfHwgaXNMaW5rKCkgfHwgaXNIZWFkaW5nKCkgfHwgaXNMaXN0SXRlbSgpOwog fQogCiB1bnNpZ25lZCBBY2Nlc3NpYmlsaXR5T2JqZWN0OjpnZXRMZW5ndGhGb3JUZXh0UmFuZ2Uo KSBjb25zdApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdC9ndGsvQ2hhbmdlTG9nIGIvU291cmNl L1dlYktpdC9ndGsvQ2hhbmdlTG9nCmluZGV4IGFlYmFhNmMuLjgwMWE3OTggMTAwNjQ0Ci0tLSBh L1NvdXJjZS9XZWJLaXQvZ3RrL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2ViS2l0L2d0ay9DaGFu Z2VMb2cKQEAgLTIsNiArMiwxOSBAQAogCiAgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09Q UyEpLgogCisgICAgICAgIFtHdGtdIGF0a190ZXh0X3NldF9jYXJldF9vZmZzZXQgZmFpbHMgZm9y IGxpc3QgaXRlbXMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTUzMzg4CisKKyAgICAgICAgVXBkYXRlIHVuaXQgdGVzdCB0byBjaGVjayB0aGUgZml4IGZv ciB0aGlzIGJ1Zy4KKworICAgICAgICAqIHRlc3RzL3Rlc3RhdGsuYzoKKyAgICAgICAgKHRlc3RX ZWJraXRBdGtDYXJldE9mZnNldHMpOiBFbnN1cmUgc2V0dGluZyB0aGUgY2FyZXQgaW4gYSBsaXN0 CisgICAgICAgIGl0ZW0gdGhyb3VnaCB0aGUgQXRrVGV4dCBpbnRlcmZhY2UgaXMgcG9zc2libGUu CisKKzIwMTEtMDEtMzEgIE1hcmlvIFNhbmNoZXogUHJhZGEgIDxtc2FuY2hlekBpZ2FsaWEuY29t PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCiAgICAgICAgIFtHdGtd IGF0a190ZXh0X3NldF9jYXJldF9vZmZzZXQgcmV0dXJucyBUcnVlIGV2ZW4gd2hlbiBpdCBpcyB1 bnN1Y2Nlc3NmdWwKICAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTUzMzg5CiAKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvZ3RrL3Rlc3RzL3Rlc3RhdGsu YyBiL1NvdXJjZS9XZWJLaXQvZ3RrL3Rlc3RzL3Rlc3RhdGsuYwppbmRleCBiNmE0YzhkLi41MDYy YzJiIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0L2d0ay90ZXN0cy90ZXN0YXRrLmMKKysrIGIv U291cmNlL1dlYktpdC9ndGsvdGVzdHMvdGVzdGF0ay5jCkBAIC0yOTIsOSArMjkyLDEzIEBAIHN0 YXRpYyB2b2lkIHRlc3RXZWJraXRBdGtDYXJldE9mZnNldHMoKQogICAgIHJlc3VsdCA9IGF0a190 ZXh0X3NldF9jYXJldF9vZmZzZXQoQVRLX1RFWFQobGlzdEl0ZW0pLCAxKTsKICAgICBnX2Fzc2Vy dF9jbXBpbnQocmVzdWx0LCA9PSwgRkFMU0UpOwogCi0gICAgLyogVE9ETzogQ2hlY2sgaGVyZSB0 aGF0IGl0J3MgcG9zc2libGUgdG8gc2V0IHRoZSBjYXJldCBpbiB0aGUKLSAgICAgICBtaWRkbGUg b2YgdGhlIHRleHQgZm9yIGEgbGlzdCBpdGVtIHdoZW4gZml4aW5nIGJ1ZyA1MzM4OC4KLSAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTMzODggKi8KKyAgICAv KiBJdCBzaG91bGQgYmUgcG9zc2libGUgdG8gcGxhY2UgdGhlIGNhcmV0IGluc2lkZSBhbiBpdGVt J3MgdGV4dC4gKi8KKyAgICByZXN1bHQgPSBhdGtfdGV4dF9zZXRfY2FyZXRfb2Zmc2V0KEFUS19U RVhUKGxpc3RJdGVtKSwgNSk7CisgICAgZ19hc3NlcnRfY21waW50KHJlc3VsdCwgPT0sIFRSVUUp OworCisgICAgLyogVW5jb21tZW50IHRoZSBmb2xsb3dpbmcgdHdvIGxpbmVzIHdoZW4gZml4aW5n IGJ1ZyA1MzQzNi4gKi8KKyAgICAvKiBvZmZzZXQgPSBhdGtfdGV4dF9nZXRfY2FyZXRfb2Zmc2V0 KEFUS19URVhUKGxpc3RJdGVtKSk7ICovCisgICAgLyogZ19hc3NlcnRfY21waW50KG9mZnNldCwg PT0sIDUpOyAqLwogCiAgICAgQXRrT2JqZWN0KiBwYW5lbCA9IGF0a19vYmplY3RfcmVmX2FjY2Vz c2libGVfY2hpbGQob2JqZWN0LCAzKTsKICAgICBnX2Fzc2VydChBVEtfSVNfT0JKRUNUKHBhbmVs KSk7Ci0tIAoxLjcuMy41Cgo=