Closed Bug 1285804 Opened 4 years ago Closed 4 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: bhsu, Assigned: bhsu)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 5 obsolete files)

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.
Blocks: 1284758
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
Assignee: nobody → bhsu
Priority: -- → P2
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 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 :)
Attachment #8781021 - Flags: feedback?(btseng)
Attachment #8781022 - Flags: feedback?(btseng)
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.
Attachment #8781021 - Flags: feedback?(btseng) → feedback+
Comment on attachment 8781022 [details] [diff] [review]
Part 2: Update and enable testcases

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

lgtm, thanks!
Attachment #8781022 - Flags: feedback?(btseng) → feedback+
Hi Ben, are you blocked by anything?
Flags: needinfo?(bhsu)
Nope, I just cannot process multiple bugs at one time :P
Flags: needinfo?(bhsu)
Attachment #8781022 - Attachment is obsolete: true
Attachment #8786360 - Flags: feedback+
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 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.
Attachment #8786359 - Flags: review?(bugs)
Attachment #8786360 - Flags: review?(bugs)
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 =
Attachment #8786359 - Flags: review?(bugs) → review+
Attachment #8786360 - Flags: review?(bugs) → review+
Comment addressed
Attachment #8786359 - Attachment is obsolete: true
Attachment #8787066 - Flags: review+
Attachment #8787066 - Flags: feedback+
Part 1 needs rebasing.
Keywords: checkin-needed
Hi Ben, in case you didn't notice comment 18. :)
Flags: needinfo?(bhsu)
Sorry and thanks, I thought it were landed ...
Flags: needinfo?(bhsu)
Pushed by ryanvm@gmail.com:
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
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/87ffc1af99e0
https://hg.mozilla.org/mozilla-central/rev/95146077a1b7
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Duplicate of this bug: 1299202
You need to log in before you can comment on or make changes to this bug.