Bug 87806 – Add mouse support to pointer and hover CSS media features

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: CSS

[Rick Byers]

In bug 87403 I added basic support for (pointer) and (hover) with respect to a touch screen.  To make the support more complete we should allow ports to supply information about whether or not a mouse is known to be available.  In particular, in   I have left this comment:

 // FIXME: We should also try to determine if we know we have a mouse.
 // When we do this, we'll also need to differentiate between known not to
 // have mouse or touch screen (NoPointer) and unknown (UnknownPointer).
 // We could also take into account other preferences like accessibility
 // settings to decide which of the available pointers should be considered
 // "primary".

[Rick Byers]

(In reply to comment #0)
> In bug 87403 I added basic support for (pointer) and (hover) with respect to a touch screen.  To make the support more complete we should allow ports to supply information about whether or not a mouse is known to be available.  In particular, in   I have left this comment:
> 
>  // FIXME: We should also try to determine if we know we have a mouse.
>  // When we do this, we'll also need to differentiate between known not to
>  // have mouse or touch screen (NoPointer) and unknown (UnknownPointer).
>  // We could also take into account other preferences like accessibility
>  // settings to decide which of the available pointers should be considered
>  // "primary".

That's in leastCapablePrimaryPointerDeviceType in MediaQueryEvaluator.cpp

[Rick Byers]

*** Bug 112841 has been marked as a duplicate of this bug. ***

[David Bokan]

Created attachment 194294 [details]
Patch for BUG 87806

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Adam Barth]

Comment on attachment 194294 [details]
Patch for BUG 87806

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

> Source/WebCore/page/Settings.h:60
> +    // Enum for primary pointer device to tell webkit what kind of input the

webkit -> WebKit

> Source/WebCore/page/Settings.h:68
> +        PointerDeviceMouseAndTouch,

How can the primary pointer be PointerDeviceMouseAndTouch?

> Source/WebKit/chromium/public/WebSettings.h:61
> +    enum PointerDevice {
> +        PointerDeviceMouse,
> +        PointerDeviceTouch,
> +        PointerDeviceMouseAndTouch,
> +        PointerDeviceNone,
> +        PointerDeviceUnknown
> +    };

We need some COMPILE_ASSERTs that these enums match the WebCore versions.

[David Bokan]

Comment on attachment 194294 [details]
Patch for BUG 87806

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

>> Source/WebCore/page/Settings.h:60
>> +    // Enum for primary pointer device to tell webkit what kind of input the
> 
> webkit -> WebKit

Fixed.

>> Source/WebCore/page/Settings.h:68
>> +        PointerDeviceMouseAndTouch,
> 
> How can the primary pointer be PointerDeviceMouseAndTouch?

The intention was to make sure clients with Mouse+Touch behave like Touch. Though thinking about it again, I'm not sure this should generally be the case. Plus this does seem even more confusing. I've removed the PointerDeviceMouseAndTouch enum.

>> Source/WebKit/chromium/public/WebSettings.h:61
>> +    };
> 
> We need some COMPILE_ASSERTs that these enums match the WebCore versions.

Added

[David Bokan]

Created attachment 194596 [details]
Review Fixes

[Adam Barth]

Comment on attachment 194596 [details]
Review Fixes

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

This seems like a reaonable improvement over what we've got currently, but we seem to be missing a notification pathway for when the primary device changes.

> Source/WebCore/ChangeLog:6
> +        Reviewed by Adam Barth

You should leave this line as the default (i.e., saying "nobody").  The tools will fill in the correct value automatically based on who actually sets the review flag to +.

> Source/WebCore/page/Settings.h:64
> +    // Enum for primary pointer device to tell WebKit what kind of input the
> +    // user is primarily using. Note that it's not an indication of what's 
> +    // available on the user's system.
> +    // e.g. Primary pointer may be set to PointerDeviceMouse but device has
> +    // a mouse and a touch screen.

How do we know what the primary input device is?  Is there a notification pathway if that changes (e.g., a laptop that transforms into a tablet)?

[WebKit Review Bot]

Comment on attachment 194596 [details]
Review Fixes

Rejecting attachment 194596 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=gce-cq-04', 'apply-attachment', '--no-update', '--non-interactive', 194596, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
4 succeeded at 753 (offset 5 lines).
patching file Source/WebKit/chromium/src/WebSettingsImpl.h
patching file Source/WebKit/chromium/src/WebViewImpl.cpp
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/fast/media/mq-pointer-expected.txt
patching file LayoutTests/fast/media/mq-pointer.html

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'Adam Barth']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://webkit-commit-queue.appspot.com/results/17303475

[David Bokan]

Comment on attachment 194596 [details]
Review Fixes

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

>> Source/WebCore/page/Settings.h:64
>> +    // a mouse and a touch screen.
> 
> How do we know what the primary input device is?  Is there a notification pathway if that changes (e.g., a laptop that transforms into a tablet)?

The browser will set this. This means that it will have to monitor some platform-specific events and change this setting appropriately. Is this what you mean by notification pathway or is there notifications in WebKit I need to set to say a CSS media query has changed?

[David Bokan]

Created attachment 196359 [details]
Resolving merge conflicts

[Rick Byers]

Comment on attachment 194294 [details]
Patch for BUG 87806

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

>>> Source/WebCore/page/Settings.h:68
>>> +        PointerDeviceMouseAndTouch,
>> 
>> How can the primary pointer be PointerDeviceMouseAndTouch?
> 
> The intention was to make sure clients with Mouse+Touch behave like Touch. Though thinking about it again, I'm not sure this should generally be the case. Plus this does seem even more confusing. I've removed the PointerDeviceMouseAndTouch enum.

There isn't a single "primary" pointer, there is a set of primary pointers.  The media queries here rely on the 'least capable primary pointer device' to tell the page what the lowest common denominator is (i.e. if one of the primary pointers doesn't support hover, then don't rely on hover).  But the notion of 'least capable' is context specific (for granularity and hover, touch is less capable than mouse, but for a multi-pointer or radius API of some sort, mouse would be less capable than touch).

So ideally Chrome shouldn't be the one coercing touch+mouse into 'primary is touch' here.  That should be left to the MQ processing code.  So either the API should take/return a list of values, or we should have one special value for 'touch and mouse' (since that's the only additional combination - though this would get messy if, for example, we add stylus support in the future).  Regardless, the name of the API should be plural (eg. 'getPrimaryPointerDevices') not singular.

[Rick Byers]

(In reply to comment #10)
> (From update of attachment 194596 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=194596&action=review
> 
> >> Source/WebCore/page/Settings.h:64
> >> +    // a mouse and a touch screen.
> > 
> > How do we know what the primary input device is?  Is there a notification pathway if that changes (e.g., a laptop that transforms into a tablet)?
> 
> The browser will set this. This means that it will have to monitor some platform-specific events and change this setting appropriately. Is this what you mean by notification pathway or is there notifications in WebKit I need to set to say a CSS media query has changed?

Yes we are missing the notification pathway - this is bug 88339.  But it sounds like most people agree this is pretty low priority, still a bug and something we should be designing for, but no particular urgency in implementing it.

[Rick Byers]

*** This bug has been marked as a duplicate of bug 134822 ***