61492 2011-05-25 17:41:19 -0700 Safari's AXFocusedUIElement attribute returns WebArea object with AXSelectedTextMarkerRange of nil 2011-06-01 15:02:15 -0700 1 1 1 Unclassified WebKit Accessibility 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 cfleizach cfleizach oldest_to_newest 410064 0 cfleizach 2011-05-25 17:41:19 -0700 In WK2, the element returned as the AXFocusedUIElement from the WebProcess doesn't know which WKView it's being asked for. So, we iterate the WebPage's to try to find which is focused. This method was only checking m_page->focusController()->isActive(); However, that would return page's that were hidden or not foreground. Changing that to return m_windowIsVisible && m_page->focusController()->isFocused() && m_page->focusController()->isActive(); Gives much better results 410075 1 94893 cfleizach 2011-05-25 17:48:49 -0700 Created attachment 94893 patch 410105 2 94893 webkit-ews 2011-05-25 18:29:13 -0700 Comment on attachment 94893 patch Attachment 94893 did not pass qt-ews (qt): Output: http://queues.webkit.org/results/8735246 410111 3 94899 cfleizach 2011-05-25 18:35:50 -0700 Created attachment 94899 patch 410683 4 94899 darin 2011-05-26 11:52:06 -0700 Comment on attachment 94899 patch View in context: https://bugs.webkit.org/attachment.cgi?id=94899&action=review > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1969 > + bool windowIsVisible = true; > +#if PLATFORM(MAC) > + windowIsVisible = m_windowIsVisible; > +#endif > > + return windowIsVisible && m_page->focusController()->isFocused() && m_page->focusController()->isActive(); This could be done much cleaner: #if PLATFORM(MAC) if (!m_windowIsVisible) return false; #endif return m_page->focusController()->isFocused() && m_page->focusController()->isActive(); I’m also a bit surprised that we need to check isActive. 410692 5 cfleizach 2011-05-26 11:54:02 -0700 (In reply to comment #4) > (From update of attachment 94899 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=94899&action=review > > > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1969 > > + bool windowIsVisible = true; > > +#if PLATFORM(MAC) > > + windowIsVisible = m_windowIsVisible; > > +#endif > > > > + return windowIsVisible && m_page->focusController()->isFocused() && m_page->focusController()->isActive(); > > This could be done much cleaner: > > #if PLATFORM(MAC) > if (!m_windowIsVisible) > return false; > #endif > return m_page->focusController()->isFocused() && m_page->focusController()->isActive(); > > I’m also a bit surprised that we need to check isActive. Will double check if isActive() is necessary. Thanks 410972 6 cfleizach 2011-05-26 16:23:19 -0700 it looks like isActive is needed, otherwise we choose the wrong tab 413696 7 cfleizach 2011-06-01 15:02:15 -0700 http://trac.webkit.org/changeset/87458 94893 2011-05-25 17:48:49 -0700 2011-05-25 18:35:50 -0700 patch patch.txt text/plain 1468 cfleizach SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDg3MzQwKQorKysgU291cmNlL1dlYktpdDIvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTEtMDUtMjUgIENocmlzIEZs ZWl6YWNoICA8Y2ZsZWl6YWNoQGFwcGxlLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JP RFkgKE9PUFMhKS4KKworICAgICAgICBTYWZhcmkncyBBWEZvY3VzZWRVSUVsZW1lbnQgYXR0cmli dXRlIHJldHVybnMgV2ViQXJlYSBvYmplY3Qgd2l0aCBBWFNlbGVjdGVkVGV4dE1hcmtlclJhbmdl IG9mIG5pbAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 NjE0OTIKKworICAgICAgICBUaGUgbWV0aG9kIGZvciBkZXRlcm1pbmluZyB3aGljaCBXZWJQYWdl IGlzIGFjdHVhbGx5IGZvY3VzZWQgYW5kIGZvcmVncm91bmQgd2FzIGZhdWx0eS4gTW9yZQorICAg ICAgICBleHRlbnNpdmUgY2hlY2tzIGFyZSByZXF1aXJlZCB0byBjaGVjayB3aGljaCBXZWJQYWdl IHNob3VsZCBiZSByZXR1cm5lZCBhcyB0aGUgZm9jdXNhYmxlIG9uZS4KKworICAgICAgICAqIFdl YlByb2Nlc3MvV2ViUGFnZS9XZWJQYWdlLmNwcDoKKyAgICAgICAgKFdlYktpdDo6V2ViUGFnZTo6 d2luZG93SXNGb2N1c2VkKToKKwogMjAxMS0wNS0yNSAgTWFyayBSb3dlICA8bXJvd2VAYXBwbGUu Y29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IFNhbSBXZWluaWcuCkluZGV4OiBTb3VyY2UvV2Vi S2l0Mi9XZWJQcm9jZXNzL1dlYlBhZ2UvV2ViUGFnZS5jcHAKPT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNl L1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdlL1dlYlBhZ2UuY3BwCShyZXZpc2lvbiA4NzMwNikK KysrIFNvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvV2ViUGFnZS9XZWJQYWdlLmNwcAkod29ya2lu ZyBjb3B5KQpAQCAtMTk2MSw3ICsxOTYxLDcgQEAKICAgICAKIGJvb2wgV2ViUGFnZTo6d2luZG93 SXNGb2N1c2VkKCkgY29uc3QKIHsKLSAgICByZXR1cm4gbV9wYWdlLT5mb2N1c0NvbnRyb2xsZXIo KS0+aXNBY3RpdmUoKTsKKyAgICByZXR1cm4gbV93aW5kb3dJc1Zpc2libGUgJiYgbV9wYWdlLT5m b2N1c0NvbnRyb2xsZXIoKS0+aXNGb2N1c2VkKCkgJiYgbV9wYWdlLT5mb2N1c0NvbnRyb2xsZXIo KS0+aXNBY3RpdmUoKTsKIH0KICAgICAKIHZvaWQgV2ViUGFnZTo6ZGlkUmVjZWl2ZU1lc3NhZ2Uo Q29yZUlQQzo6Q29ubmVjdGlvbiogY29ubmVjdGlvbiwgQ29yZUlQQzo6TWVzc2FnZUlEIG1lc3Nh Z2VJRCwgQ29yZUlQQzo6QXJndW1lbnREZWNvZGVyKiBhcmd1bWVudHMpCg== 94899 2011-05-25 18:35:50 -0700 2011-05-26 11:52:06 -0700 patch patch.txt text/plain 1673 cfleizach SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDg3MzQwKQorKysgU291cmNlL1dlYktpdDIvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTEtMDUtMjUgIENocmlzIEZs ZWl6YWNoICA8Y2ZsZWl6YWNoQGFwcGxlLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JP RFkgKE9PUFMhKS4KKworICAgICAgICBTYWZhcmkncyBBWEZvY3VzZWRVSUVsZW1lbnQgYXR0cmli dXRlIHJldHVybnMgV2ViQXJlYSBvYmplY3Qgd2l0aCBBWFNlbGVjdGVkVGV4dE1hcmtlclJhbmdl IG9mIG5pbAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 NjE0OTIKKworICAgICAgICBUaGUgbWV0aG9kIGZvciBkZXRlcm1pbmluZyB3aGljaCBXZWJQYWdl IGlzIGFjdHVhbGx5IGZvY3VzZWQgYW5kIGZvcmVncm91bmQgd2FzIGZhdWx0eS4gTW9yZQorICAg ICAgICBleHRlbnNpdmUgY2hlY2tzIGFyZSByZXF1aXJlZCB0byBjaGVjayB3aGljaCBXZWJQYWdl IHNob3VsZCBiZSByZXR1cm5lZCBhcyB0aGUgZm9jdXNhYmxlIG9uZS4KKworICAgICAgICAqIFdl YlByb2Nlc3MvV2ViUGFnZS9XZWJQYWdlLmNwcDoKKyAgICAgICAgKFdlYktpdDo6V2ViUGFnZTo6 d2luZG93SXNGb2N1c2VkKToKKwogMjAxMS0wNS0yNSAgTWFyayBSb3dlICA8bXJvd2VAYXBwbGUu Y29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IFNhbSBXZWluaWcuCkluZGV4OiBTb3VyY2UvV2Vi S2l0Mi9XZWJQcm9jZXNzL1dlYlBhZ2UvV2ViUGFnZS5jcHAKPT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNl L1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdlL1dlYlBhZ2UuY3BwCShyZXZpc2lvbiA4NzMwNikK KysrIFNvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvV2ViUGFnZS9XZWJQYWdlLmNwcAkod29ya2lu ZyBjb3B5KQpAQCAtMTk1OCwxMiArMTk1OCwxOSBAQAogfQogCiAjZW5kaWYKLSAgICAKKwogYm9v bCBXZWJQYWdlOjp3aW5kb3dJc0ZvY3VzZWQoKSBjb25zdAogewotICAgIHJldHVybiBtX3BhZ2Ut PmZvY3VzQ29udHJvbGxlcigpLT5pc0FjdGl2ZSgpOwotfQorICAgIGJvb2wgd2luZG93SXNWaXNp YmxlID0gdHJ1ZTsKKyNpZiBQTEFURk9STShNQUMpCisgICAgd2luZG93SXNWaXNpYmxlID0gbV93 aW5kb3dJc1Zpc2libGU7CisjZW5kaWYKICAgICAKKyAgICByZXR1cm4gd2luZG93SXNWaXNpYmxl ICYmIG1fcGFnZS0+Zm9jdXNDb250cm9sbGVyKCktPmlzRm9jdXNlZCgpICYmIG1fcGFnZS0+Zm9j dXNDb250cm9sbGVyKCktPmlzQWN0aXZlKCk7Cit9ICAgIAorCisgICAgCisKIHZvaWQgV2ViUGFn ZTo6ZGlkUmVjZWl2ZU1lc3NhZ2UoQ29yZUlQQzo6Q29ubmVjdGlvbiogY29ubmVjdGlvbiwgQ29y ZUlQQzo6TWVzc2FnZUlEIG1lc3NhZ2VJRCwgQ29yZUlQQzo6QXJndW1lbnREZWNvZGVyKiBhcmd1 bWVudHMpCiB7CiAgICAgaWYgKG1lc3NhZ2VJRC5pczxDb3JlSVBDOjpNZXNzYWdlQ2xhc3NEcmF3 aW5nQXJlYUxlZ2FjeT4oKSkgewo=