Bug 153263 – [Mac] Tooltips do not honor some types of obscuring windows

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Brent Fulgham]

When a WebView is obscured by the Notification Center or Spotlight windows, tooltips continue to be displayed even though the mouse pointer is no longer in the WebView.

This is happening because these these types of applications (and perhaps others) do not become the foreground application, confusing WebKit.

[Brent Fulgham]

<rdar://problem/21423972>

[Brent Fulgham]

Created attachment 269323 [details]
Patch

[WebKit Commit Bot]

Attachment 269323 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Cocoa/WebPageProxyCocoa.mm:29:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tim Horton]

Does WebKit1 have the same bug?

[Tim Horton]

Comment on attachment 269323 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=269323&action=review

> Source/WebKit2/UIProcess/Cocoa/WebPageProxyCocoa.mm:67
> +    NSInteger eventWindowNumber = [NSWindow windowNumberAtPoint:[NSEvent mouseLocation] belowWindowWithWindowNumber:0];

This is not Cocoa code, it's either AppKit or Mac code.

[Tim Horton]

Also, we have this big ViewState mechanism. Does it make any sense to make this a part of that?

[Build Bot]

Comment on attachment 269323 [details]
Patch

Attachment 269323 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/714926

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 269325 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Simon Fraser (smfr)]

Comment on attachment 269323 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=269323&action=review

>> Source/WebKit2/UIProcess/Cocoa/WebPageProxyCocoa.mm:67
>> +    NSInteger eventWindowNumber = [NSWindow windowNumberAtPoint:[NSEvent mouseLocation] belowWindowWithWindowNumber:0];
> 
> This is not Cocoa code, it's either AppKit or Mac code.

This also may not be cheap. It's probably a round trip with the WindowServer. I suspect that there are also legitimate times when other windows are on top but we still need to show tooltips.

Why isn't this just an AppKit bug?

[Brent Fulgham]

Created attachment 269360 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 269360 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=269360&action=review

> Source/WebKit2/UIProcess/mac/WebPageProxyMac.mm:599
> +bool WebPageProxy::viewIsObscured()

The name of this function...

> Source/WebKit2/UIProcess/mac/WebPageProxyMac.mm:602
> +    NSInteger eventWindowNumber = [NSWindow windowNumberAtPoint:[NSEvent mouseLocation] belowWindowWithWindowNumber:0];

...doesn't agree with the implementation, which is looking at the global mouse location. I don't think we should have a function whose behavior depends implicitly on the mouse location.

[Brent Fulgham]

Created attachment 269399 [details]
Patch

[Brent Fulgham]

Comment on attachment 269399 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=269399&action=review

> Source/WebKit/mac/WebView/WebHTMLView.mm:2138
> +

Whoops! Extra space.

[Simon Fraser (smfr)]

Comment on attachment 269399 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=269399&action=review

> Source/WebKit2/UIProcess/mac/WebPageProxyMac.mm:608
> +    NSPoint eventLocalWindowPosition = event.nativeEvent().locationInWindow;
> +    NSRect eventScreenPosition = [m_pageClient.platformWindow() convertRectToScreen:NSMakeRect(eventLocalWindowPosition.x, eventLocalWindowPosition.y, 0, 0)];
> +
> +    NSInteger currentWindowNumber = [m_pageClient.platformWindow() windowNumber];
> +    NSInteger eventWindowNumber = [NSWindow windowNumberAtPoint:eventScreenPosition.origin belowWindowWithWindowNumber:0];
> +    
> +    return currentWindowNumber != eventWindowNumber;

Feels like this code should live up in WKWebView or somewhere.

The name could also be improved; something like windowIsFrontWindowUnderMouse:

[Tim Horton]

(In reply to comment #14)
> Comment on attachment 269399 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=269399&action=review
> 
> > Source/WebKit2/UIProcess/mac/WebPageProxyMac.mm:608
> > +    NSPoint eventLocalWindowPosition = event.nativeEvent().locationInWindow;
> > +    NSRect eventScreenPosition = [m_pageClient.platformWindow() convertRectToScreen:NSMakeRect(eventLocalWindowPosition.x, eventLocalWindowPosition.y, 0, 0)];
> > +
> > +    NSInteger currentWindowNumber = [m_pageClient.platformWindow() windowNumber];
> > +    NSInteger eventWindowNumber = [NSWindow windowNumberAtPoint:eventScreenPosition.origin belowWindowWithWindowNumber:0];
> > +    
> > +    return currentWindowNumber != eventWindowNumber;
> 
> Feels like this code should live up in WKWebView or somewhere.

WebViewImpl

> The name could also be improved; something like
> windowIsFrontWindowUnderMouse:

[Brent Fulgham]

Created attachment 269525 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 269525 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=269525&action=review

> Source/WebKit2/UIProcess/WebPageProxy.cpp:1671
> +        setToolTip("");

I think we prefer String() to "".

[Brent Fulgham]

Committed r195451: <http://trac.webkit.org/changeset/195451>