Bug 87806 - Add mouse support to pointer and hover CSS media features
Summary: Add mouse support to pointer and hover CSS media features
Status: RESOLVED DUPLICATE of bug 134822
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Bokan
URL:
Keywords:
Depends on: 87403
Blocks: 112847
  Show dependency treegraph
 
Reported: 2012-05-29 18:40 PDT by Rick Byers
Modified: 2014-07-21 18:04 PDT (History)
16 users (show)

See Also:


Attachments
Patch for BUG 87806 (24.56 KB, patch)
2013-03-21 11:18 PDT, David Bokan
no flags Details | Formatted Diff | Diff
Review Fixes (24.14 KB, patch)
2013-03-22 10:50 PDT, David Bokan
no flags Details | Formatted Diff | Diff
Resolving merge conflicts (24.23 KB, patch)
2013-04-03 08:47 PDT, David Bokan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rick Byers 2012-05-29 18:40:46 PDT
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".
Comment 1 Rick Byers 2012-05-29 18:41:50 PDT
(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
Comment 2 Rick Byers 2013-03-20 14:00:33 PDT
*** Bug 112841 has been marked as a duplicate of this bug. ***
Comment 3 David Bokan 2013-03-21 11:18:31 PDT
Created attachment 194294 [details]
Patch for BUG 87806
Comment 4 WebKit Review Bot 2013-03-21 11:19:36 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 5 Adam Barth 2013-03-21 14:47:27 PDT
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.
Comment 6 David Bokan 2013-03-22 10:49:34 PDT
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
Comment 7 David Bokan 2013-03-22 10:50:20 PDT
Created attachment 194596 [details]
Review Fixes
Comment 8 Adam Barth 2013-03-31 18:00:48 PDT
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)?
Comment 9 WebKit Review Bot 2013-03-31 18:02:20 PDT
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
Comment 10 David Bokan 2013-04-03 08:40:25 PDT
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?
Comment 11 David Bokan 2013-04-03 08:47:27 PDT
Created attachment 196359 [details]
Resolving merge conflicts
Comment 12 Rick Byers 2013-04-03 09:20:37 PDT
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.
Comment 13 Rick Byers 2013-04-03 09:22:08 PDT
(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.
Comment 14 Rick Byers 2014-07-21 18:04:47 PDT

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