Safari's AXFocusedUIElement attribute returns WebArea object with AXSelectedTextMarkerRange of nil

[chris fleizach]

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

[chris fleizach]

Created attachment 94893 [details]
patch

[Early Warning System Bot]

Comment on attachment 94893 [details]
patch

Attachment 94893 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/8735246

[chris fleizach]

Created attachment 94899 [details]
patch

[Darin Adler]

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.

[chris fleizach]

(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

[chris fleizach]

it looks like isActive is needed, otherwise we choose the wrong tab

[chris fleizach]

http://trac.webkit.org/changeset/87458