Closed Bug 1304904 Opened 6 years ago Closed 5 years ago

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

Categories

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

50 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: git, Assigned: stone)

Details

Attachments

(1 file, 4 obsolete files)

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.
Retriage to Widget Cocoa if Dom Events is wrong.
Component: Untriaged → DOM: Events
Product: Firefox → Core
:stone, is this bug in your area? I saw you fix some pointer events bugs recently.
Flags: needinfo?(sshih)
Yes, I can add this in my to-do list
Flags: needinfo?(sshih)
Assignee: nobody → sshih
Priority: -- → P3
Attachment #8826060 - Attachment is obsolete: true
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 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.
Attachment #8826062 - Flags: review?(bugs)
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.
Attachment #8826062 - Flags: review?(bugs)
Re-upload patch with modifications that override tabletProximity when OS X version is equal or larger than 10.10
Attachment #8826062 - Attachment is obsolete: true
Attachment #8826097 - Flags: review?(bugs)
Attachment #8826097 - Flags: review?(mstange)
Attachment #8826097 - Flags: review?(bugs)
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?
Attachment #8826097 - Flags: review?(mstange)
Attachment #8826097 - Attachment is obsolete: true
Attachment #8826097 - Flags: review?(bugs)
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.
Attachment #8828232 - Flags: review?(mstange)
Attachment #8828232 - Flags: review?(mstange) → review+
Attachment #8828232 - Flags: review?(bugs)
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8828232 - Flags: review?(bugs) → review+
That was mostly rs+, given that this is pretty much all Cocoa
Updated the patch summary
Attachment #8828232 - Attachment is obsolete: true
Attachment #8829301 - Flags: review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2fe1d9200e32
Handle tabletProximity to detect stylus and support tilt for OS X. r=mstange, r=smaug
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/2fe1d9200e32
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.