Bug 87806

(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

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

Created attachment 194294 [details] Patch for BUG 87806

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 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 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

Created attachment 194596 [details] Review Fixes

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 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 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?

Created attachment 196359 [details] Resolving merge conflicts

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.

(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.

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