Summary: | Safari's AXFocusedUIElement attribute returns WebArea object with AXSelectedTextMarkerRange of nil | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | chris fleizach <cfleizach> | ||||||
Component: | Accessibility | Assignee: | chris fleizach <cfleizach> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | ||||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
chris fleizach
2011-05-25 17:41:19 PDT
Created attachment 94893 [details]
patch
Comment on attachment 94893 [details] patch Attachment 94893 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/8735246 Created attachment 94899 [details]
patch
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. (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 it looks like isActive is needed, otherwise we choose the wrong tab |