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

Status: RESOLVED FIXED

Description

[Ming-Chou Shih [:stone]]

Comment 1

[Ming-Chou Shih [:stone]]

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

Comment 2

[Ming-Chou Shih [:stone]]

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

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

Comment 3

[Ming-Chou Shih [:stone]]

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.

Comment 4

[Bevis Tseng[:bevis][:btseng](Exited)]

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

Comment 5

[Ming-Chou Shih [:stone]]

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

Comment 6

[Bevis Tseng[:bevis][:btseng](Exited)]

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;

Comment 7

[Ming-Chou Shih [:stone]]

Created attachment 8788025 [details] [diff] [review]
Bug 1296491 - Set attribute 'detail' of PointerEvent to 0 (V2)

Comment 8

[Ming-Chou Shih [:stone]]

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

Updated the patch summary

Comment 9

[Ming-Chou Shih [:stone]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1e2776fc09e69ab1e69c7d3665467be59cc03c23

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/17e78729eb38
[Pointer Event] Investigate failure of pointerevent_attributes_mouse-manual.html. r=bevistseng, r=smaug

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/17e78729eb38