Bug 64615 - REGRESSION: Mouse cursor doesn’t hide when full screen video HUD hides
Summary: REGRESSION: Mouse cursor doesn’t hide when full screen video HUD hides
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: mitz
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-07-15 12:12 PDT by mitz
Modified: 2011-07-20 10:02 PDT (History)
6 users (show)

See Also:


Attachments
Add Chrome::setCursorHiddenUntilMouseMoved() and call it from the media controls (29.90 KB, patch)
2011-07-15 12:20 PDT, mitz
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2011-07-15 12:12:20 PDT
REGRESSION: Mouse cursor doesn’t hide when full screen video HUD hides
Comment 1 mitz 2011-07-15 12:12:50 PDT
<rdar://problem/9525626>
Comment 2 mitz 2011-07-15 12:20:02 PDT
Created attachment 101023 [details]
Add Chrome::setCursorHiddenUntilMouseMoved() and call it from the media controls
Comment 3 Anders Carlsson 2011-07-15 12:31:55 PDT
Comment on attachment 101023 [details]
Add Chrome::setCursorHiddenUntilMouseMoved() and call it from the media controls

Looks good, but please fix the build failures before committing.
Comment 4 mitz 2011-07-15 13:08:03 PDT
Fixed in r91097. <http://trac.webkit.org/r91097>
Comment 5 Darin Adler 2011-07-15 16:59:44 PDT
Comment on attachment 101023 [details]
Add Chrome::setCursorHiddenUntilMouseMoved() and call it from the media controls

Do we really need the boolean? Would we ever call this with false? What would it mean if we did?
Comment 6 Darin Adler 2011-07-15 17:00:08 PDT
8>c:\cygwin\home\buildbot\webkit\source\webkit2\uiprocess\win\WebView.h(70) : error C2259: 'WebKit::WebView' : cannot instantiate abstract class
8>        due to following members:
8>        'void WebKit::PageClient::setCursorHiddenUntilMouseMoves(bool)' : is abstract
8>        c:\cygwin\home\buildbot\WebKit\Source\WebKit2\UIProcess\PageClient.h(113) : see declaration of 'WebKit::PageClient::setCursorHiddenUntilMouseMoves'
Comment 7 mitz 2011-07-15 17:57:34 PDT
(In reply to comment #5)
> (From update of attachment 101023 [details])
> Do we really need the boolean?

No.

> Would we ever call this with false?

Maybe if we wanted to fix a hypothetical bug where when the page ends fullscreen video mode the mouse cursor doesn’t show up.

> What would it mean if we did?

It would show the cursor if it were still hidden as a result of calling this with true.
Comment 8 Daniel Bates 2011-07-16 17:52:20 PDT
This patch broke the Qt Linux Release build:

<http://build.webkit.org/builders/Qt%20Linux%20Release/builds/35466/steps/compile-webkit/logs/stdio>

It looks like an implementation of setCursorHiddenUntilMouseMoves() is needed for the Qt WebKit2 port from looking at the stdio bot output.

Committed build fix in changeset 91159 <http://trac.webkit.org/changeset/91159>.
Comment 9 mitz 2011-07-16 17:59:20 PDT
Thanks!
Comment 10 mitz 2011-07-16 18:02:29 PDT
Is there a reason why the Qt PageClient is called QtWebPageProxy? I think that’s why I didn’t notice that it needed this change.
Comment 11 Daniel Bates 2011-07-16 19:53:49 PDT
(In reply to comment #10)
> Is there a reason why the Qt PageClient is called QtWebPageProxy? I think that’s why I didn’t notice that it needed this change.

I don't know. CC'ing Andreas Kling, Benjamin Poulain and Kenneth Rohde Christiansen who may have some insight about the name.
Comment 12 Benjamin Poulain 2011-07-17 04:14:20 PDT
(In reply to comment #10)
> Is there a reason why the Qt PageClient is called QtWebPageProxy? I think that’s why I didn’t notice that it needed this change.

It is a work-in-progress class from the old design (where we had QWKPagePrivate being both a wrapper around WebPageProxy and an implementation of PageClient.

That should get cleaner as we are splitting the old classes.
Comment 13 Jer Noble 2011-07-17 19:57:42 PDT
Could this have been fixed by adding a "cursor:none" CSS style to the full screen stylesheets?
Comment 14 mitz 2011-07-20 10:02:58 PDT
(In reply to comment #13)
> Could this have been fixed by adding a "cursor:none" CSS style to the full screen stylesheets?

It couldn’t have just been added to the style sheets, as the cursor needs to be hidden and shown dynamically, but you are right that it may have been possible to control this using a style declaration and the existing CSS cursor mechanism.