[Pointer Event] Investigate failure of pointerevent_attributes_mouse-manual.html

RESOLVED FIXED in Firefox 51

Status

()

P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: stone, Assigned: stone)

Tracking

unspecified
mozilla51
Points:
---

Firefox Tracking Flags

(firefox51 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Comment hidden (empty)
(Assignee)

Updated

2 years ago
Depends on: 1296185

Updated

2 years ago
Priority: -- → P2
(Assignee)

Comment 1

2 years ago
failed | mouse pointermove.detail value is 0. - got 1, expected +0
failed | mouse pointerdown.detail value is 0. - got 1, expected +0
failed | mouse pointerup.detail value is 0. - got 1, expected +0
Assignee: nobody → sshih
(Assignee)

Comment 2

2 years ago
[1] defines the attribute 'detail' of all pointer events should be 0.

[1]https://w3c.github.io/pointerevents/#pointer-event-types
(Assignee)

Comment 3

2 years ago
Created attachment 8787496 [details] [diff] [review]
Bug 1296491 - [Pointer Event] Investigate failure of pointerevent_attributes_mouse-manual.html (V1)

Followed spec to set detail attribute of pointer event to 0.
Attachment #8787496 - Flags: feedback?(btseng)
Comment on attachment 8787496 [details] [diff] [review]
Bug 1296491 - [Pointer Event] Investigate failure of pointerevent_attributes_mouse-manual.html (V1)

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

I'd like to revisit this again after the following concern is clarified.

Thanks!

::: dom/events/PointerEvent.cpp
@@ +31,5 @@
>      mEvent->mTime = PR_Now();
>      mEvent->mRefPoint = LayoutDeviceIntPoint(0, 0);
>      mouseEvent->inputSource = nsIDOMMouseEvent::MOZ_SOURCE_UNKNOWN;
>    }
> +  mDetail = 0;

It's pretty easy to override the protected value in subclass but I'd like to know the following concern before approving this approach: 
1. why this will be non-zero for a newly created PointerEvent instance in current implementation?
2. and why should we overwrite it here instead of having expected value be initiated in parent class(UIEvent)?
Attachment #8787496 - Flags: feedback?(btseng)
(Assignee)

Comment 5

2 years ago
Per UI Event spec, detail of mousedown[1] and mouseup[2] indicates the current click count. Pointer events inherit mouse events, so this attribute of PointerEvent may be non-zero. Pointer event spec [3] says detail attribute should be 0, so we have to override this attribute.

[1] https://www.w3.org/TR/uievents/#event-type-mousedown
[2] https://www.w3.org/TR/uievents/#event-type-mouseup
[3] https://w3c.github.io/pointerevents/#pointer-event-types
Flags: needinfo?(btseng)
Comment on attachment 8787496 [details] [diff] [review]
Bug 1296491 - [Pointer Event] Investigate failure of pointerevent_attributes_mouse-manual.html (V1)

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

f=me after appending the comment inlined in the patch, thanks!

::: dom/events/PointerEvent.cpp
@@ +31,5 @@
>      mEvent->mTime = PR_Now();
>      mEvent->mRefPoint = LayoutDeviceIntPoint(0, 0);
>      mouseEvent->inputSource = nsIDOMMouseEvent::MOZ_SOURCE_UNKNOWN;
>    }
> +  mDetail = 0;

// 5.2 Pointer Event types, for all pointer events, |detail| attribute SHOULD be 0.
  mDetail = 0;
Attachment #8787496 - Flags: feedback+
Flags: needinfo?(btseng)
(Assignee)

Comment 7

2 years ago
Created attachment 8788025 [details] [diff] [review]
Bug 1296491 - Set attribute 'detail' of PointerEvent to 0 (V2)
Attachment #8787496 - Attachment is obsolete: true
Attachment #8788025 - Flags: feedback+
(Assignee)

Updated

2 years ago
Attachment #8788025 - Flags: review?(bugs)

Updated

2 years ago
Attachment #8788025 - Flags: review?(bugs) → review+
(Assignee)

Comment 8

2 years ago
Created attachment 8788286 [details] [diff] [review]
Bug 1296491 - Set attribute 'detail' of PointerEvent to 0 (V3)

Updated the patch summary
Attachment #8788025 - Attachment is obsolete: true
Attachment #8788286 - Flags: review+
Attachment #8788286 - Flags: feedback+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 10

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/17e78729eb38
[Pointer Event] Investigate failure of pointerevent_attributes_mouse-manual.html. r=bevistseng, r=smaug
Keywords: checkin-needed

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/17e78729eb38
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.