Bug 61492

Summary: Safari's AXFocusedUIElement attribute returns WebArea object with AXSelectedTextMarkerRange of nil
Product: WebKit Reporter: chris fleizach <cfleizach>
Component: AccessibilityAssignee: chris fleizach <cfleizach>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
webkit-ews: commit-queue-
patch darin: review+

Description chris fleizach 2011-05-25 17:41:19 PDT
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
Comment 1 chris fleizach 2011-05-25 17:48:49 PDT
Created attachment 94893 [details]
patch
Comment 2 Early Warning System Bot 2011-05-25 18:29:13 PDT
Comment on attachment 94893 [details]
patch

Attachment 94893 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/8735246
Comment 3 chris fleizach 2011-05-25 18:35:50 PDT
Created attachment 94899 [details]
patch
Comment 4 Darin Adler 2011-05-26 11:52:06 PDT
Comment on attachment 94899 [details]
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.
Comment 5 chris fleizach 2011-05-26 11:54:02 PDT
(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
Comment 6 chris fleizach 2011-05-26 16:23:19 PDT
it looks like isActive is needed, otherwise we choose the wrong tab
Comment 7 chris fleizach 2011-06-01 15:02:15 PDT
http://trac.webkit.org/changeset/87458