1285804 - Enable test_pointerevent_pointerleave_pen-manual.html and test_pointerevent_pointerout_pen-manual.html

Status: RESOLVED FIXED

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

Description

[Ben Hsu [:HoPang]]

After a short investigation, I think the root cause of these two testcases might be identical, so I create this new bug to track them.

Comment 1

[Ben Hsu [:HoPang]]

The root cause of this issue is that the mExitFrom of synthesized mouse events is default to WidgetMouseEvent::eChild, which makes the testcases never enter the "else branch" [0] and thus no pointerleave or pointerout event is launched correspondingly. At this moment, I only have a quick fix for this fix, which still need to be polished.

[0]: https://dxr.mozilla.org/mozilla-central/source/dom/events/EventStateManager.cpp#677

Comment 2

[Ben Hsu [:HoPang]]

Attachment: Part 1: Introduce a new message to nsIDOMWindowUtils::sendMouseEvent() for automated testing

Comment 3

[Ben Hsu [:HoPang]]

Attachment: Part 1: Introduce a new message to nsIDOMWindowUtils::sendMouseEvent() for automated testing

Comment 4

[Ben Hsu [:HoPang]]

Attachment: Part 2: Update and enable testcases

Comment 5

[Ben Hsu [:HoPang]]

There are two patches to fix this issue, where the first one is for the automated testing framework, while the second one is for the testcases themselves.

In the first patch, I add a new message to describe the vanishing behavior of a pen. Since our current pointer event implementation is based on WidgetMouseEvent and WidgetTouchEvent, and a pen is encapsulated as a mouse in the widget level, so I introduce the new message to WidgetMouseEvent instead of WidgetPointerEvent. Another thing is that, the mMessage and the mExitFrom of the WidgetMouseEvent describing the vanishing of the pen are eMouseExitFromWidget and WidgetMouseEvent::eTopLevel, respectively.

As the title of the second patch, I do two things in the second patch. One is that I update the widget event sequences of the two testcases, since the they should use the new message to describe the vanishing of the pen. The other is enabling the two testcases in their mochitest.ini.

Comment 6

[Ben Hsu [:HoPang]]

Comment on attachment 8781021 [details] [diff] [review]
Part 1: Introduce a new message to nsIDOMWindowUtils::sendMouseEvent() for automated testing

Hi Bevis,

May I have your feedback on these two patches as well? There is a slightly detailed explanation in comment 5. However, should you have any question, do please feel free to pin me :)

Comment 7

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

Comment on attachment 8781021 [details] [diff] [review]
Part 1: Introduce a new message to nsIDOMWindowUtils::sendMouseEvent() for automated testing

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

f=me after the following suggestions are addressed.

::: dom/base/nsContentUtils.cpp
@@ +8158,1 @@
>  

if (exitFromTopLevelWidget) {
  event.mExitFrom = WidgetMouseEvent::eTopLevel;
}

::: dom/interfaces/base/nsIDOMWindowUtils.idl
@@ +279,5 @@
>    const long MODIFIER_SYMBOLLOCK = 0x0800;
>    const long MODIFIER_OS         = 0x1000;
>  
>    /** Synthesize a mouse event. The event types supported are:
> +   *    mousedown, mouseup, mousemove, mouseover, mouseout, mouseexit,

how about *mousegone* for pen vanishing?

@@ +303,5 @@
>     *
> +   * NOTE: mouseexit is used to represent the vanishing of an input device such
> +   * a pen leaving its digitizer. The mMessage and the mExitFrom of the derived
> +   * WidgetMouseEvent are eMouseExitFromWidget and WidgetMouseEvent::eTopLevel,
> +   * respectively.

"mouseexit" is used to represent the vanishing of an input device such a pen leaving its digitizer with exitFrom reason of the eMouseExitFromWidget event set to WidgetMouseEvent::eTopLevel.

Comment 8

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

Comment on attachment 8781022 [details] [diff] [review]
Part 2: Update and enable testcases

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

lgtm, thanks!

Comment 9

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Hi Ben, are you blocked by anything?

Comment 10

[Ben Hsu [:HoPang]]

Nope, I just cannot process multiple bugs at one time :P

Comment 11

[Ben Hsu [:HoPang]]

Attachment: Part 1: Introduce a new message to nsIDOMWindowUtils::sendMouseEvent() for test automation

Comment 12

[Ben Hsu [:HoPang]]

Attachment: Part 2: Update and enable testcases

Comment 13

[Ben Hsu [:HoPang]]

Lacking the ability to synthesize the right WidgetPointerEvent (mMessage == eMouseExitFromWidget, mExitFrom == WidgetMouseEvent::eTopLevel), we cannot make Gecko do what we desired when a pen leaves the screen, and thus we failed to pass the testcases.

There are two patches provided to solve this issue. In the first patch, I extend the capability of nsIWindowUtils by introducing a new message - mousecancel. According to this message, we are able to tell when to synthesize the desired WidgetMouseEvent. In the other patch, I simply update the platform message sequences and enable them.

The name, mousecancel, might be a little weird, since a mouse can never be removed. However, current Gecko wraps pen actions into WidgetMouseEvents, so I think we had better keep the "mouse" here. On the other hand, the word "cancel" is adopted to conform the Draft of WebDriver[1].

[0]: https://dxr.mozilla.org/mozilla-central/source/dom/events/EventStateManager.cpp#677
[1]: https://w3c.github.io/webdriver/webdriver-spec.html#terminology-1

Comment 14

[Ben Hsu [:HoPang]]

Comment on attachment 8786359 [details] [diff] [review]
Part 1: Introduce a new message to nsIDOMWindowUtils::sendMouseEvent() for test automation

Hi Olli,

Do you mind reviewing these patches?
Please refer to comment 13 for more detailed description.

Comment 15

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

Comment on attachment 8786359 [details] [diff] [review]
Part 1: Introduce a new message to nsIDOMWindowUtils::sendMouseEvent() for test automation

>+  } else if (aType.EqualsLiteral("mousecancel")) {
>+    msg =  eMouseExitFromWidget;
nit, extra space after =

Comment 16

[Ben Hsu [:HoPang]]

Attachment: (V2) Part 1: Introduce a new message to nsIDOMWindowUtils::sendMouseEvent() for test automation

Comment addressed

Comment 17

[Ben Hsu [:HoPang]]

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

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Part 1 needs rebasing.

Comment 19

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Hi Ben, in case you didn't notice comment 18. :)

Comment 20

[Ben Hsu [:HoPang]]

Sorry and thanks, I thought it were landed ...

Comment 21

[Ben Hsu [:HoPang]]

Attachment: (V3) Part 1: Introduce a new message to nsIDOMWindowUtils::sendMouseEvent() for test automation

Rebased :)

Comment 22

[Ben Hsu [:HoPang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=113f20629f71

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/87ffc1af99e0
Part 1: Introduce a new message to nsIDOMWindowUtils::sendMouseEvent() for test automation. r=smaug, f=btseng
https://hg.mozilla.org/integration/mozilla-inbound/rev/95146077a1b7
Part 2: Update and enable testcases. r=smaug, f=btseng

Comment 24

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/87ffc1af99e0
https://hg.mozilla.org/mozilla-central/rev/95146077a1b7