262929 2023-10-09 15:40:00 -0700 AX: Attribute cache is not enabled in traverseToOffsetInRange 2023-10-10 09:45:54 -0700 1 1 1 Unclassified WebKit Accessibility WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 jhoffman23 jhoffman23 aboxhall andresg_22 apinheiro cfleizach dmazzoni ews-watchlist jcraig jdiggs samuel_white webkit-bug-importer oldest_to_newest 1983851 0 jhoffman23 2023-10-09 15:40:00 -0700 The attribute cache is not enabled in traverseToOffsetInRange, causing more accessibilityIsIgnored calls than necessary. 1983852 1 webkit-bug-importer 2023-10-09 15:40:09 -0700 <rdar://problem/116703599> 1983860 2 468137 jhoffman23 2023-10-09 16:29:37 -0700 Created attachment 468137 Patch 1983979 3 andresg_22 2023-10-10 06:39:38 -0700 (In reply to Joshua Hoffman from comment #2) > Created attachment 468137 [details] > Patch diff --git a/Source/WebCore/accessibility/AXObjectCache.cpp b/Source/WebCore/accessibility/AXObjectCache.cpp index 05688086b278..bea7eaf1ef97 100644 --- a/Source/WebCore/accessibility/AXObjectCache.cpp +++ b/Source/WebCore/accessibility/AXObjectCache.cpp @@ -2679,6 +2679,9 @@ CharacterOffset AXObjectCache::traverseToOffsetInRange(const SimpleRange& range, behaviors.add(TextIteratorBehavior::EntersTextControls); TextIterator iterator(range, behaviors); + // Enable the cache here for accessibilityIsIgnored calls in replacedNodeNeedsCharacter + AXAttributeCacheEnabler enableCache(this); AG: if the problem is in replacedNodeNeedsCharacter, shouldn't we enable the caching there instead? + // When the range has zero length, there might be replaced node or brTag that we need to increment the characterOffset. if (iterator.atEnd()) { currentNode = range.start.container.ptr(); 1984000 4 jhoffman23 2023-10-10 08:48:40 -0700 (In reply to Andres Gonzalez from comment #3) > (In reply to Joshua Hoffman from comment #2) > > Created attachment 468137 [details] > > Patch > + // Enable the cache here for accessibilityIsIgnored calls in > replacedNodeNeedsCharacter > + AXAttributeCacheEnabler enableCache(this); > > AG: if the problem is in replacedNodeNeedsCharacter, shouldn't we enable the > caching there instead? replacedNodeNeedsCharacter, is called in a loop inside traverseToOffsetInRange, so we want to make sure the cache stays enabled across all of those calls to prevent duplicate work. 1984023 5 ews-feeder 2023-10-10 09:45:51 -0700 Committed 269149@main (7d4e7afc92b3): <https://commits.webkit.org/269149@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 468137. 468137 2023-10-09 16:29:37 -0700 2023-10-10 09:45:52 -0700 Patch bug-262929-20231009162936.patch text/plain 1541 jhoffman23 RnJvbSAzNzAyNGExMzM2ZTEwNTM5NGYyZDc3ZjBhMzgyNjVmNjE0ZDg4Y2M4IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBKb3NodWEgSG9mZm1hbiA8amhvZmZtYW4yM0BhcHBsZS5jb20+ CkRhdGU6IE1vbiwgOSBPY3QgMjAyMyAxNjoyNjoyNyAtMDcwMApTdWJqZWN0OiBbUEFUQ0hdIEFY OiBBdHRyaWJ1dGUgY2FjaGUgaXMgbm90IGVuYWJsZWQgaW4gdHJhdmVyc2VUb09mZnNldEluUmFu Z2UKIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yNjI5MjkgcmRhcjov L3Byb2JsZW0vMTE2NzAzNTk5CgpSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KClRoaXMgcGF0 Y2ggZW5hYmxlcyB0aGUgQVhPYmplY3RBdHRyaWJ1dGVDYWNoZSBpbiB0aGUgc2NvcGUgb2YKQVhP YmplY3RDYWNoZTo6dHJhdmVyc2VUb09mZnNldEluUmFuZ2UuIEJ5IGRvaW5nIHRoaXMsIHdlIHNh dmUKdW5uZWNlc3NhcnkgYWNjZXNzaWJpbGl0eUlzSWdub3JlZCBjYWxscyBvbiB0aGUgbWFpbiB0 aHJlYWQuCgoqIFNvdXJjZS9XZWJDb3JlL2FjY2Vzc2liaWxpdHkvQVhPYmplY3RDYWNoZS5jcHA6 CihXZWJDb3JlOjpBWE9iamVjdENhY2hlOjp0cmF2ZXJzZVRvT2Zmc2V0SW5SYW5nZSk6Ci0tLQog U291cmNlL1dlYkNvcmUvYWNjZXNzaWJpbGl0eS9BWE9iamVjdENhY2hlLmNwcCB8IDMgKysrCiAx IGZpbGUgY2hhbmdlZCwgMyBpbnNlcnRpb25zKCspCgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNv cmUvYWNjZXNzaWJpbGl0eS9BWE9iamVjdENhY2hlLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2FjY2Vz c2liaWxpdHkvQVhPYmplY3RDYWNoZS5jcHAKaW5kZXggMDU2ODgwODZiMjc4Li5iZWE3ZWFmMWVm OTcgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2FjY2Vzc2liaWxpdHkvQVhPYmplY3RDYWNo ZS5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvYWNjZXNzaWJpbGl0eS9BWE9iamVjdENhY2hlLmNw cApAQCAtMjY3OSw2ICsyNjc5LDkgQEAgQ2hhcmFjdGVyT2Zmc2V0IEFYT2JqZWN0Q2FjaGU6OnRy YXZlcnNlVG9PZmZzZXRJblJhbmdlKGNvbnN0IFNpbXBsZVJhbmdlJiByYW5nZSwKICAgICAgICAg YmVoYXZpb3JzLmFkZChUZXh0SXRlcmF0b3JCZWhhdmlvcjo6RW50ZXJzVGV4dENvbnRyb2xzKTsK ICAgICBUZXh0SXRlcmF0b3IgaXRlcmF0b3IocmFuZ2UsIGJlaGF2aW9ycyk7CiAgICAgCisgICAg Ly8gRW5hYmxlIHRoZSBjYWNoZSBoZXJlIGZvciBhY2Nlc3NpYmlsaXR5SXNJZ25vcmVkIGNhbGxz IGluIHJlcGxhY2VkTm9kZU5lZWRzQ2hhcmFjdGVyCisgICAgQVhBdHRyaWJ1dGVDYWNoZUVuYWJs ZXIgZW5hYmxlQ2FjaGUodGhpcyk7CisKICAgICAvLyBXaGVuIHRoZSByYW5nZSBoYXMgemVybyBs ZW5ndGgsIHRoZXJlIG1pZ2h0IGJlIHJlcGxhY2VkIG5vZGUgb3IgYnJUYWcgdGhhdCB3ZSBuZWVk IHRvIGluY3JlbWVudCB0aGUgY2hhcmFjdGVyT2Zmc2V0LgogICAgIGlmIChpdGVyYXRvci5hdEVu ZCgpKSB7CiAgICAgICAgIGN1cnJlbnROb2RlID0gcmFuZ2Uuc3RhcnQuY29udGFpbmVyLnB0cigp Owo=