Closed Bug 1296491 Opened 3 years ago Closed 3 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: stone, Assigned: stone)

References

Details

Attachments

(1 file, 2 obsolete files)

No description provided.
Depends on: 1296185
Priority: -- → P2
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
[1] defines the attribute 'detail' of all pointer events should be 0.

[1]https://w3c.github.io/pointerevents/#pointer-event-types
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)
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)
Attachment #8787496 - Attachment is obsolete: true
Attachment #8788025 - Flags: feedback+
Attachment #8788025 - Flags: review?(bugs)
Attachment #8788025 - Flags: review?(bugs) → review+
Updated the patch summary
Attachment #8788025 - Attachment is obsolete: true
Attachment #8788286 - Flags: review+
Attachment #8788286 - Flags: feedback+
Keywords: checkin-needed
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
https://hg.mozilla.org/mozilla-central/rev/17e78729eb38
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.