1304904 - pointerEvent.pointerType == "mouse" for pen input on MacOS 10.11.6

Status: RESOLVED FIXED

(Core :: DOM: Events, defect, P3)

Description

[git]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.116 Safari/537.36

Steps to reproduce:

Clicked on http://codepen.io/appsforartists/full/NRdLrv with various pointer types


Actual results:

| Input (pointerdown)        | event.pointerType | event.buttons | event.pointerId | event.pressure |
| -------------------------- | ----------------- | ------------- | --------------- | -------------- |
| Wacom tap                  | mouse             | 1             | 0               | .5             |
| Wacom stylus tip           | mouse             | 1             | 0               | 0.352941185235 |
| Wacom stylus lower button  | mouse             | 2             | 0               | .5             |
| Wacom stylus upper button  | mouse             | 4             | 0               | .5             |
| Wacom stylus eraser        | mouse             | 1             | 0               | 0.572549045085 |
| MacBook one-finger click   | mouse             | 1             | 0               | .5             |
| MacBook two-finger click   | mouse             | 2             | 0               | .5             |



Expected results:

pointerType should be "pen" for the Wacom stylus events.

Comment 1

[Loic]

Retriage to Widget Cocoa if Dom Events is wrong.

Comment 2

[Loic]

:stone, is this bug in your area? I saw you fix some pointer events bugs recently.

Comment 3

[Ming-Chou Shih [:stone]]

Yes, I can add this in my to-do list

Comment 4

[Ming-Chou Shih [:stone]]

Attachment: Handle tabletProximity to detect stylus and support tilt for OS X

Comment 5

[Ming-Chou Shih [:stone]]

Attachment: Handle tabletProximity to detect stylus and support tilt for OS X

Comment 6

[Ming-Chou Shih [:stone]]

The concept is overriding tabletProximity to detect whether a pen is entering or leaving the range of the pointing device. Then set tilt attribute and input source = pen when a pen is detected.

Comment 7

[Ming-Chou Shih [:stone]]

Comment on attachment 8826062 [details] [diff] [review]
Handle tabletProximity to detect stylus and support tilt for OS X

Hi Smaug,
Could you please take a look at my patch? Please advise me if other reviewers might want to review it. Thanks.

Comment 8

[Ming-Chou Shih [:stone]]

Comment on attachment 8826062 [details] [diff] [review]
Handle tabletProximity to detect stylus and support tilt for OS X

Oh, I should wrap tabletProximity with OS version check. Withdraw the review request.

Comment 9

[Ming-Chou Shih [:stone]]

Attachment: Handle tabletProximity to detect stylus and support tilt for OS X

Re-upload patch with modifications that override tabletProximity when OS X version is equal or larger than 10.10

Comment 10

[Markus Stange [:mstange]]

Comment on attachment 8826097 [details] [diff] [review]
Handle tabletProximity to detect stylus and support tilt for OS X

Review of attachment 8826097 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/cocoa/nsChildView.mm
@@ +5180,2 @@
>        if ([aMouseEvent subtype] == NSTabletPointEventSubtype) {
> +        if ([aPointerEvent type] != NSMouseMoved) {

aPointerEvent? This looks like a mistake.

@@ +5210,5 @@
> +  NS_OBJC_END_TRY_ABORT_BLOCK;
> +}
> +
> +#if defined(MAC_OS_X_VERSION_10_10) && \
> +    MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_10

What's the purpose of this check?
https://developer.apple.com/reference/appkit/nsresponder/1527018-tabletproximity?language=objc says that this method has been available since 10.4. And overriding methods that aren't called would not be harmful anyway.

Also, this is a compile time check, and our builders currently build with the 10.9 SDK, so this code would not be compiled in our official builds.

@@ +5214,5 @@
> +    MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_10
> +- (void)tabletProximity:(NSEvent *)theEvent
> +{
> +  NS_OBJC_BEGIN_TRY_ABORT_BLOCK_RETURN
> +  sIsTabletPointerActivated = [theEvent isEnteringProximity];

Is the isEnteringProximity field only valid on events that are passed to tabletProximity:? Or could you check this field on aMouseEvent in convertCocoaMouseEvent instead?

Comment 11

[Ming-Chou Shih [:stone]]

Attachment: Handle tabletProximity to detect stylus and support tilt for OS X

Comment 12

[Ming-Chou Shih [:stone]]

Comment on attachment 8828232 [details] [diff] [review]
Handle tabletProximity to detect stylus and support tilt for OS X

> Is the isEnteringProximity field only valid on events that are passed to
> tabletProximity:? Or could you check this field on aMouseEvent in
> convertCocoaMouseEvent instead?
Yes, it's only valid in tabletProximity. This method is invoked when a pen is entering or leaving the range which OS can detect.

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

That was mostly rs+, given that this is pretty much all Cocoa

Comment 14

[Ming-Chou Shih [:stone]]

Attachment: Bug 1304904: Handle tabletProximity to detect stylus and support tilt for OS X

Updated the patch summary

Comment 15

[Ming-Chou Shih [:stone]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=211bf846aca6a04b7b6621a6d6b1ec92954bc7c2

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2fe1d9200e32
Handle tabletProximity to detect stylus and support tilt for OS X. r=mstange, r=smaug

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2fe1d9200e32