Closed Bug 1293174 Opened 5 years ago Closed 5 years ago

[Pointer Event] Implement implicit pointer capture for touch

Categories

(Core :: DOM: Events, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: stone, Assigned: stone)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 4 obsolete files)

Blocks: 1292437
Assignee: nobody → sshih
There are some discussions whether we should implicit capture pointer for touch or all input types in https://github.com/w3c/pointerevents/issues/8. It's not concluded yet. Implement it in case we want to do some experiments and use preference to control on/off. Set it's default value to off since it isn't part of spec at this moment.
Attachment #8788367 - Flags: feedback?(btseng)
Removed redundant modifications to gfxPrefs.h
Attachment #8788367 - Attachment is obsolete: true
Attachment #8788367 - Flags: feedback?(btseng)
Attachment #8788709 - Flags: feedback?(btseng)
Comment on attachment 8788709 [details] [diff] [review]
Bug 1293174 - [Pointer Event] Implement implicit pointer capture for touch (V2)

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

Please see my comments below, thanks!

::: dom/events/test/pointerevents/bug1293174_implicit_pointer_capture_for_touch-1.html
@@ +38,5 @@
> +  target1.addEventListener("pointermove", (e) => {
> +    ok(false, "pointer should be captured by target0, target1 should never receive pointer events");
> +  }, false);
> +
> +  parent.turnOnOffPointerEvents( function() {

We are not supposed to do this twice in a test.
Please try to create a iframe element dynamically in parent window to see if we can skip this workaround.
Or figure out a correct solution to prevent this if a static iframe is needed.

::: dom/events/test/pointerevents/bug1293174_implicit_pointer_capture_for_touch-2.html
@@ +39,5 @@
> +  target1.addEventListener("pointermove", (e) => {
> +    ++pointermove_count_of_target1;
> +  }, false);
> +
> +  parent.turnOnOffPointerEvents( function() {

ditto

::: dom/events/test/pointerevents/test_bug1293174_implicit_pointer_capture_for_touch-1.html
@@ +8,5 @@
> +  <title>Test for Bug 1293174</title>
> +  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
> +  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
> +  <script type="text/javascript">
> +    function prepareTest() {

let's see if we can move these common use functions into a help.js

@@ +12,5 @@
> +    function prepareTest() {
> +      SimpleTest.waitForExplicitFinish();
> +      turnOnOffPointerEvents(startTest);
> +    }
> +    function turnOnOffPointerEvents(callback) {

ditto
Attachment #8788709 - Flags: feedback?(btseng)
Use pointer events helper functions to refine the test cases
Attachment #8788709 - Attachment is obsolete: true
Attachment #8789708 - Flags: feedback?(btseng)
Comment on attachment 8789708 [details] [diff] [review]
Bug 1293174 - [Pointer Event] Implement implicit pointer capture for touch (V3)

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

Please see my comment below:

::: dom/events/test/pointerevents/mochitest_support_external.js
@@ +27,5 @@
>    SpecialPowers.pushPrefEnv({
>      "set": [
>        ["dom.w3c_pointer_events.enabled", true],
> +      ["layout.css.touch_action.enabled", true],
> +      ["dom.w3c_pointer_events.implicit_capture", implicitCapture]

This is NG since we named this function as turn"On"PointerEvents() while all other preferences are true but implicit_capture is set depends on the value of |implicitCaputure|.

Please have a dedicated function named 
enableImplicitPointerCapture(enabled, callback) {
} 
for toggling this feature.
Attachment #8789708 - Flags: feedback?(btseng)
Attachment #8792450 - Flags: feedback?(btseng)
Comment on attachment 8792450 [details] [diff] [review]
Bug 1293174 - [Pointer Event] Implement implicit pointer capture for touch (V4)

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

LGTM, thanks!
Attachment #8792450 - Flags: feedback?(btseng) → feedback+
Attachment #8792450 - Flags: review?(bugs)
Comment on attachment 8792450 [details] [diff] [review]
Bug 1293174 - [Pointer Event] Implement implicit pointer capture for touch (V4)

>+    <meta name="author" content="Maksim Lebedev" />
This looks wrong. In couple of places. Just remove it.
Attachment #8792450 - Flags: review?(bugs) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b577f6e25606
[Pointer Event] Implement implicit pointer capture for touch. f=bevistseng. r=smaug
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b577f6e25606
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.