Closed Bug 1337133 Opened 7 years ago Closed 7 years ago

Dispatch pointer actions for mouse pointer type

Categories

(Remote Protocol :: Marionette, defect)

Version 3
defect
Not set
normal

Tracking

(firefox52 fixed, firefox-esr52 fixed, firefox53 fixed, firefox54 fixed)

RESOLVED FIXED
mozilla54
Tracking Status
firefox52 --- fixed
firefox-esr52 --- fixed
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: impossibus, Assigned: impossibus)

References

(Blocks 1 open bug)

Details

(Keywords: pi-marionette-server)

Attachments

(16 files, 1 obsolete file)

59 bytes, text/x-review-board-request
ato
: review+
Details
59 bytes, text/x-review-board-request
ato
: review+
Details
59 bytes, text/x-review-board-request
ato
: review+
Details
59 bytes, text/x-review-board-request
ato
: review+
Details
59 bytes, text/x-review-board-request
ato
: review+
Details
59 bytes, text/x-review-board-request
ato
: review+
Details
59 bytes, text/x-review-board-request
ato
: review+
Details
59 bytes, text/x-review-board-request
ato
: review+
Details
59 bytes, text/x-review-board-request
jgraham
: review+
Details
59 bytes, text/x-review-board-request
ato
: review+
Details
59 bytes, text/x-review-board-request
ato
: review+
Details
59 bytes, text/x-review-board-request
ato
: review+
Details
59 bytes, text/x-review-board-request
ato
: review+
Details
59 bytes, text/x-review-board-request
jgraham
: review+
Details
119.94 KB, patch
Details | Diff | Splinter Review
149.79 KB, patch
Details | Diff | Splinter Review
https://w3c.github.io/webdriver/webdriver-spec.html#h-pointer-actions

Dispatch pointerDown, pointerUp, pointerMove for pointer type "mouse".
Early feedback welcome on the WIP patches if you feel like it, but they're not intended for review yet. I'll be back on Friday.
Comment on attachment 8835244 [details]
Bug 1337133 - Add number check to assert.js;

https://reviewboard.mozilla.org/r/110938/#review112392

::: testing/marionette/assert.js:153
(Diff revision 1)
> +assert.number = function (obj, msg = "") {
> +  msg = msg || error.pprint`Expected ${obj} to be number`;
> +  return assert.that(n => typeof n == "number", msg)(obj);
> +};

This does not handle NaN or Infinity.

::: testing/marionette/test_assert.js:43
(Diff revision 1)
> +add_test(function test_number() {
> +  assert.number(1);
> +  assert.number(0);
> +  assert.number(-1);
> +  assert.number(1.2);
> +  Assert.throws(() => assert.number("foo"), InvalidArgumentError);
> +  run_next_test();
> +});

Add a test for NaN and Infinity here.

::: testing/marionette/test_assert.js:57
(Diff revision 1)
>  add_test(function test_integer() {
>    assert.integer(1);
>    assert.integer(0);
>    assert.integer(-1);
>    Assert.throws(() => assert.integer("foo"), InvalidArgumentError);
> +  Assert.throws(() => assert.integer(1.2), InvalidArgumentError);

Well spotted.
Attachment #8835244 - Flags: review?(ato) → review+
Comment on attachment 8835245 [details]
Bug 1337133 - Remove primary pointer attribute;

https://reviewboard.mozilla.org/r/110940/#review112396
Attachment #8835245 - Flags: review?(ato) → review+
Comment on attachment 8835246 [details]
Bug 1337133 - Replace element with origin for pointerMove actions;

https://reviewboard.mozilla.org/r/110942/#review112402

::: testing/marionette/action.js:355
(Diff revision 1)
> +    if (!(name in this)) {
> +      throw new InvalidArgumentError(`Unknown pointer-move origin: ${obj}`);
> +    }

I’m curious, will this allow "get"?

::: testing/marionette/test_action.js:105
(Diff revision 1)
> +});
> +
> +add_test(function test_processPointerMoveActionOriginStringValidation() {
> +  let actionSequence = {type: "pointer", id: "some_id"};
> +  let actionItem = {duration: 5000, type: "pointerMove"};
> +  for (let d of ["a", "", "undefined"]) {

Should "undefined" not be allowed?
Attachment #8835246 - Flags: review?(ato) → review+
Comment on attachment 8835247 [details]
Bug 1337133 - Replace some checks in actions with calls to assert.js;

https://reviewboard.mozilla.org/r/110944/#review112404
Attachment #8835247 - Flags: review?(ato) → review+
Comment on attachment 8835248 [details]
Bug 1337133 - Add MoveTargetOutOfBoundsError as a new Webdriver error type;

https://reviewboard.mozilla.org/r/110946/#review112406
Attachment #8835248 - Flags: review?(ato) → review+
Comment on attachment 8835249 [details]
Bug 1337133 - Split test kinds, common fixtures, utilities in wdspec tests;

https://reviewboard.mozilla.org/r/110948/#review112412

::: testing/web-platform/tests/webdriver/actions/support/util.py:1
(Diff revision 1)
> +def get_keys(input_el):

Generally speaking, avoid naming stuff “util”.  There’s a danger turning these things into kitchen sinks.
Comment on attachment 8835251 [details]
Bug 1337133 - Set up fixtures for mouse actions in wdspec tests;

https://reviewboard.mozilla.org/r/110952/#review112416

::: testing/web-platform/tests/test_actions_wdspec.html:40
(Diff revision 1)
>              }
>            }
>            return key;
>          }
>  
> -        function recordEvent(event) {
> +        function recordKeyBoardEvent(event) {

Keyboard is normally one word, so the Board capitalisation seems a bit unnecessary.
(In reply to Maja Frydrychowicz (:maja_zf) from comment #11)
> Early feedback welcome on the WIP patches if you feel like it, but they're
> not intended for review yet. I'll be back on Friday.

I didn’t set r+, but generally the unreviewed draft patches look good.
Comment on attachment 8835246 [details]
Bug 1337133 - Replace element with origin for pointerMove actions;

https://reviewboard.mozilla.org/r/110942/#review112402

> I’m curious, will this allow "get"?

Ha, yeah, that aspect is kind of ugly. 

It doesn't allow "get" and I'll add tests to reflect that. The same happens with action.PointerType. 

The only reason it doesn't accept "get" is because `obj` gets capitalized, and there's no "Get" property.

> Should "undefined" not be allowed?

Whoops. `undefined` is allowed, "undefined" is silly. Will remove.
Comment on attachment 8835244 [details]
Bug 1337133 - Add number check to assert.js;

https://reviewboard.mozilla.org/r/110938/#review115252

::: testing/marionette/assert.js:153
(Diff revision 2)
> +assert.number = function (obj, msg = "") {
> +  msg = msg || error.pprint`Expected ${obj} to be number`;
> +  return assert.that(n => (typeof n == "number" && Number.isFinite(n)), msg)(obj);
> +};

I guess that `Number.isFinite` also checks the type, so this could be replaced with this:

```js
return assert.that(Number.isFinite, msg)(obj)
```
Comment on attachment 8835246 [details]
Bug 1337133 - Replace element with origin for pointerMove actions;

https://reviewboard.mozilla.org/r/110942/#review112402

> Ha, yeah, that aspect is kind of ugly. 
> 
> It doesn't allow "get" and I'll add tests to reflect that. The same happens with action.PointerType. 
> 
> The only reason it doesn't accept "get" is because `obj` gets capitalized, and there's no "Get" property.

Ouch (-:
Comment on attachment 8835248 [details]
Bug 1337133 - Add MoveTargetOutOfBoundsError as a new Webdriver error type;

https://reviewboard.mozilla.org/r/110946/#review115256

This patch’s commit message, “Add MoveTargetOutOfBoundsError to set of Webdriver errors”, is misleading now since it adds a new WebDriver error type.
Comment on attachment 8835249 [details]
Bug 1337133 - Split test kinds, common fixtures, utilities in wdspec tests;

https://reviewboard.mozilla.org/r/110948/#review115258

::: testing/web-platform/meta/MANIFEST.json
(Diff revision 2)
> -   "./test_keys_wdspec.html": [
> -    [
> -     {}
> -    ]
> -   ],

Assuming the changes to testing/web-platform/meta/MANIFEST.json are fine.

::: testing/web-platform/tests/webdriver/actions/actions.py:1
(Diff revision 2)
> +from support.refine import get_keys, filter_dict

testing/web-platform/tests/webdriver/actions/actions.py (webdriver.actions.actions) is not a great name for a Python module.  I guess this file could be renamed chain.py or sequence.py or something better?

::: testing/web-platform/tests/webdriver/actions/mouse_actions.py:1
(Diff revision 2)
> +import pytest

I would argue we could shorten the filename to  testing/web-platform/tests/webdriver/actions/mouse.py, since it already lives inside an “actions” namespace.
Attachment #8835249 - Flags: review?(ato) → review+
Comment on attachment 8835250 [details]
Bug 1337133 - Add pointer actions endpoints to wdclient;

https://reviewboard.mozilla.org/r/110950/#review115260

::: testing/web-platform/tests/tools/webdriver/webdriver/client.py:132
(Diff revision 2)
> +        # TODO change to pointerMove once https://github.com/mozilla/webdriver-rust/issues/68 resolved
> +        action = {
> +            "type": "move",
> +            "x": x,
> +            "y": y
> +        }

This appears to have been fixed now, but I guess the fix is not yet available in the geckodriver on central?

In the interim I’ve started work on https://bugzilla.mozilla.org/show_bug.cgi?id=1340637 to compile geckodriver master on the build slaves.  But I should point that it needs a follow-up patch to have wptrunner pick up the in-tree-built geckodriver.

I don’t know if that/the followup will land before this, but just making you aware of the ongoing work in this area.

::: testing/web-platform/tests/tools/webdriver/webdriver/client.py:138
(Diff revision 2)
> +        if (duration is not None):
> +            action["duration"] = duration
> +        if (origin is not None):
> +            action["origin"] = origin

Drop the paranthesis.
Attachment #8835250 - Flags: review?(ato) → review+
Comment on attachment 8835250 [details]
Bug 1337133 - Add pointer actions endpoints to wdclient;

https://reviewboard.mozilla.org/r/110950/#review115262

Can you change the commit message to not contain “(wd client)” first?  I suggest “Add pointer action endpoints to wdclient”.
Comment on attachment 8835251 [details]
Bug 1337133 - Set up fixtures for mouse actions in wdspec tests;

https://reviewboard.mozilla.org/r/110952/#review115264

::: testing/web-platform/tests/webdriver/actions/actions.py:1
(Diff revision 2)
> -from support.refine import get_keys, filter_dict
> +from support.refine import get_keys, filter_dict, get_events
> -
> -
> -def get_events(session):
> -    """Return list of key events recorded in the test_keys_page fixture."""
> -    events = session.execute_script("return allEvents.events;") or []
> -    # `key` values in `allEvents` may be escaped (see `escapeSurrogateHalf` in
> -    # test_keys_wdspec.html), so this converts them back into unicode literals.
> -    for e in events:
> -        # example: turn "U+d83d" (6 chars) into u"\ud83d" (1 char)
> -        if e["key"].startswith(u"U+"):
> -            key = e["key"]
> -            hex_suffix = key[key.index("+") + 1:]
> -            e["key"] = unichr(int(hex_suffix, 16))
> -    return events

Strictly speaking, this function could have been moved in your earlier patch.  But don’t consider that a blocker.

::: testing/web-platform/tests/webdriver/actions/key_actions.py:4
(Diff revision 2)
> -from support.refine import get_keys, filter_dict
> +from support.refine import get_keys, filter_dict, get_events
> -
> -
> -def get_events(session):
> -    """Return list of key events recorded in the test_keys_page fixture."""
> -    events = session.execute_script("return allEvents.events;") or []
> -    # `key` values in `allEvents` may be escaped (see `escapeSurrogateHalf` in
> -    # test_keys_wdspec.html), so this converts them back into unicode literals.
> -    for e in events:
> -        # example: turn "U+d83d" (6 chars) into u"\ud83d" (1 char)
> -        if e["key"].startswith(u"U+"):
> -            key = e["key"]
> -            hex_suffix = key[key.index("+") + 1:]
> -            e["key"] = unichr(int(hex_suffix, 16))
> -    return events

Same applies for this.

::: testing/web-platform/tests/webdriver/actions/support/test_actions_wdspec.html:71
(Diff revision 2)
> +          appendMessage(`${event.type}(button:${event.button}, pageX:${event.pageX}, `
> +              + `pageY:${event.pageY}, button: ${event.button}, buttons: ${event.buttons}, `
> +              + `target id: ${event.target.id}`);

Sorry for this style nit, but could you break this on the commas to make each line more syntactically relevant?

It’s a bit hard to read as it is now, and changing it will not make a great diff.  If each distinct thing is on its own line, a future diff to this will be easier to read.

::: testing/web-platform/tests/webdriver/actions/support/test_actions_wdspec.html:86
(Diff revision 2)
>          function resetEvents() {
>              allEvents.events.length = 0;
>              displayMessage("");
>          }
> +
> +        document.addEventListener("DOMContentLoaded", function() {

Space after `function` so it doesn’t look like you call a function named "function".

::: testing/web-platform/tests/webdriver/actions/support/test_actions_wdspec.html:88
(Diff revision 2)
>              displayMessage("");
>          }
> +
> +        document.addEventListener("DOMContentLoaded", function() {
> +          var keyReporter = document.getElementById("keys");
> +          ["keyup", "keypress", "keydown"].forEach(function(e) {

Same here with regards to space after `function`.

::: testing/web-platform/tests/webdriver/actions/support/test_actions_wdspec.html:100
(Diff revision 2)
> +          });
> +          window.addEventListener("mousemove", recordFirstPointerMove);
> +          //visual cue for mousemove
> +          var pointer = document.getElementById("trackPointer");
> +          window.addEventListener("mousemove", function (e) {
> +            setTimeout(function() {

And here.

FWIW I think it would be fine to use arrow functions here as they are supported by all browsers we test on.
Attachment #8835251 - Flags: review?(ato)
Comment on attachment 8838771 [details]
Bug 1337133 - Add more MouseEvent event properties to synthesizeMouseAtPoint;

https://reviewboard.mozilla.org/r/113576/#review115266

::: testing/marionette/event.js:251
(Diff revision 1)
> +  let inputSource = ("inputSource" in opts) ? opts.inputSource :
> +      Ci.nsIDOMMouseEvent.MOZ_SOURCE_MOUSE;

So just for clarity, this is 1: https://dxr.mozilla.org/mozilla-central/source/obj-x86_64-pc-linux-gnu/dist/include/nsIDOMMouseEvent.h#77

Do you mind adding a postfix comment `MOZ_SOURCE_MOUSE /* 1 */;`?

::: testing/marionette/event.js:259
(Diff revision 1)
> -  if (("type" in event) && opts.type) {
> +  if (("type" in opts) && opts.type) {
>      domutils.sendMouseEvent(
> -        opts.type, left, top, button, clickCount, modifiers);
> +        opts.type, left, top, button, clickCount, modifiers, false, pressure, inputSource,
> +        isDOMEventSynthesized, isWidgetEventSynthesized, buttons);
>    } else {
>      domutils.sendMouseEvent(
> -        "mousedown", left, top, button, clickCount, modifiers);
> +        "mousedown", left, top, button, clickCount, modifiers, false, pressure, inputSource,
> +        isDOMEventSynthesized, isWidgetEventSynthesized, buttons);
>      domutils.sendMouseEvent(
> -        "mouseup", left, top, button, clickCount, modifiers);
> +        "mouseup", left, top, button, clickCount, modifiers, false, pressure, inputSource,
> +        isDOMEventSynthesized, isWidgetEventSynthesized, buttons);

I didn’t check these changes in detail or in practice, but trusting you they are correct.
Attachment #8838771 - Flags: review?(ato) → review+
Comment on attachment 8835252 [details]
Bug 1337133 - Dispatch pointerDown and pointerUp actions for mouse;

https://reviewboard.mozilla.org/r/110954/#review115268

::: testing/marionette/action.js:553
(Diff revision 2)
> +  /**
> +   * Check whether |button| is pressed.
> +   *
> +   * @param {number} button
> +   *     Positive integer that refers to a mouse button.
> +   *
> +   * @return {boolean}
> +   *     True if |button| is in set of pressed buttons.
> +   */
> +  isPressed(button) {
> +    return this.pressed.has(button);
> +  }

If we are supposed to only accept positive integers, I would suggest adding an `assert.positiveNumber` check here for added security.

::: testing/marionette/action.js:576
(Diff revision 2)
> +   *
> +   * @return {Set}
> +   *     Set of pressed buttons.
> +   */
> +  press(button) {
> +    return this.pressed.add(button);

Same with more security here.

::: testing/marionette/action.js:589
(Diff revision 2)
> +   *
> +   * @return {boolean}
> +   *     True if |button| was present before removals, false otherwise.
> +   */
> +  release(button) {
> +    return this.pressed.delete(button);

And here.

::: testing/marionette/action.js:876
(Diff revision 2)
> +  constructor(type, button = 0) {
> +    this.type = type;
> +    this.button = button;
> +    this.buttons = 0;
> +  }

Maybe we can add more security here too?

::: testing/marionette/action.js:882
(Diff revision 2)
> +  update(inputState) {
> +    let allButtons = Array.from(inputState.pressed);
> +    this.buttons = allButtons.reduce((a, i) => a + Math.pow(2, i), 0);
> +  }

Nice.  Joy to read.

::: testing/marionette/action.js:1069
(Diff revision 2)
>      resolve();
>    });
>  }
>  
>  /**
> + * Dispatch a pointerDown action equivalent to pressing a button on the pointer device.

Nit: This line is a bit long.

::: testing/marionette/action.js:1091
(Diff revision 2)
> +      case action.PointerType.mouse:
> +        let mouseEvent = new action.Mouse("mousedown", a.button);
> +        mouseEvent.update(inputState);
> +        event.synthesizeMouseAtPoint(inputState.x, inputState.y, mouseEvent, win);
> +        break;
> +      case action.PointerType.pen:
> +      case action.PointerType.touch:
> +        throw new UnsupportedOperationError("Only 'mouse' pointer type is supported.");

You appear to have the wrong casing for `mouse`, `pen`, and `touch` here.

They should be:

- `action.PointerType.Mouse`
- `action.PointerType.Pen`
- `action.PointerType.Touch`

It also would make sense to add a default case wherein you throw `TypeError` if `inputState.subtype` isn’t recognised.

::: testing/marionette/action.js:1105
(Diff revision 2)
> +    resolve();
> +  });
> +}
> +
> +/**
> + * Dispatch a pointerUp action equivalent to releasing a button on the pointer device

Break at ~75 character (or run the line through fmt(1) (-:).

Also add a final stop.

::: testing/marionette/action.js:1124
(Diff revision 2)
> +    switch (inputState.subtype) {
> +      case action.PointerType.mouse:
> +        let mouseEvent = new action.Mouse("mouseup", a.button);
> +        mouseEvent.update(inputState);
> +        event.synthesizeMouseAtPoint(inputState.x, inputState.y,
> +            mouseEvent, win);
> +        break;
> +      case action.PointerType.pen:
> +      case action.PointerType.touch:
> +        throw new UnsupportedOperationError("Only 'mouse' pointer type is supported.");
> +    }

I am tempted to ask you to add a default case here too.

Also add a new line break betwen the two branches.

::: testing/web-platform/tests/webdriver/actions/mouse_actions.py:6
(Diff revision 2)
>  from support.refine import get_events
>  
>  
>  def test_nothing(session, test_actions_page, mouse_chain):
>      mouse_chain \
> -        .pointer_down(1) \
> +        .pointer_down(0) \

Maybe consider introducing a constant in the future so that we don’t have magic numbers floating around.
Attachment #8835252 - Flags: review?(ato) → review+
Comment on attachment 8835253 [details]
Bug 1337133 - Dispatch pointerMove action for mouse;

https://reviewboard.mozilla.org/r/110956/#review115272

I made a cursory pass on this patch today regarding its technical aspects.  I want to go over it in more detail next week to compare it to the specification prose.

::: testing/marionette/action.js:971
(Diff revision 2)
> + * @param {obj=} center
> + *     Object representing x and y coordinates of an element center-point.
> + *     This is only used if |a.origin| is a web element reference.

I would use `Map.<string, number>` as type information.

::: testing/marionette/action.js:978
(Diff revision 2)
> + *     This is only used if |a.origin| is a web element reference.
> + *
> + * @return {Array.<number>}
> + *     x and y coordinates of pointer destination.
> + */
> +action.computePointerDestination = function(a, inputState, center) {

Mark `center` as optional:

```js
center = undefined
```

::: testing/marionette/action.js:979
(Diff revision 2)
> +  let x = a.x;
> +  let y = a.y;

This is of course absolutely fine, but you could shorten it to

```js
const {x, y} = a;
```

::: testing/marionette/action.js:996
(Diff revision 2)
> +      assert.in("x", center);
> +      assert.in("y", center);
> +      x += center.x;
> +      y += center.y;
> +  }
> +  return [x, y];

Unless you have a specific reason to return a two-element array, I would suggest changing this to return a dictionary with `x` and `y` fields.

This would have parity with the input to this function, be in sync with what we do elsewhere in Marionette when we operate on coordinates, and have a clearer syntactical meaning since an Array can have an arbitrary number of elements.

::: testing/marionette/action.js:1200
(Diff revision 2)
> + *     as appropriate.
> + */
> +function dispatchPointerMove(a, inputState, tickDuration, seenEls, container) {
> +  const timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
> +  // interval between pointermove increments in ms, based on common vsync: 60fps
> +  const refresh = 17;

I would rename this `fps60`.

::: testing/marionette/action.js:1208
(Diff revision 2)
> +      let uuid = a.origin[element.Key] || a.origin[element.LegacyKey];
> +      let rects = seenEls.get(uuid, container).getClientRects();
> +      elementCenter = element.getInViewCentrePoint(rects[0], container.frame);

Note that `rects[0]` may be empty if the element is outside the viewport.  You can either check if `rects` has a length of over 0, or call `element.isInView` first.

::: testing/marionette/action.js:1214
(Diff revision 2)
> +      throw new MoveTargetOutOfBoundsError(
> +          `(${targetX}, ${targetY}) is out of bounds of viewport ` +
> +          `width (${container.frame.innerWidth}) and height (${container.frame.innerHeight}).`);

Drop full stop in error message.

::: testing/marionette/action.js:1229
(Diff revision 2)
> +      return;
> +    }
> +
> +    const distanceX = targetX - startX;
> +    const distanceY = targetY - startY;
> +    let intermediatePointerEvents = Task.spawn(function*() {

Space after `function*`.

::: testing/marionette/action.js:1230
(Diff revision 2)
> +    }
> +
> +    const distanceX = targetX - startX;
> +    const distanceY = targetY - startY;
> +    let intermediatePointerEvents = Task.spawn(function*() {
> +      // wait |refresh| ms before performing first incremental pointer move

s/refresh/60fps/

::: testing/marionette/action.js:1231
(Diff revision 2)
> +      yield new Promise(resolveTimer =>
> +          timer.initWithCallback(resolveTimer, refresh, Ci.nsITimer.TYPE_ONE_SHOT)
> +      );

Move the `);` up to :1232.

::: testing/marionette/action.js:1232
(Diff revision 2)
> +    const distanceX = targetX - startX;
> +    const distanceY = targetY - startY;
> +    let intermediatePointerEvents = Task.spawn(function*() {
> +      // wait |refresh| ms before performing first incremental pointer move
> +      yield new Promise(resolveTimer =>
> +          timer.initWithCallback(resolveTimer, refresh, Ci.nsITimer.TYPE_ONE_SHOT)

To shorten these lines, because you call it multiple times, I would suggest assigining `Ci.nsITimer.TYPE_ONE_SHOT` to a constant at the beginning of the function:

```js
const ONE_SHOT = Ci.nsITimer.TYPE_ONE_SHOT;
```

::: testing/marionette/action.js:1234
(Diff revision 2)
> +      let durationRatio = Math.floor(Date.now() - start) / duration;
> +      const epsilon = refresh / 1000;
> +      while ((1 - durationRatio) > epsilon) {
> +        let x = Math.floor(durationRatio * distanceX + startX);
> +        let y = Math.floor(durationRatio * distanceY + startY);
> +        performOnePointerMove(inputState, x, y, container.frame);
> +        // wait |refresh| ms before performing next pointer move
> +        yield new Promise(resolveTimer =>
> +          timer.initWithCallback(resolveTimer, refresh, Ci.nsITimer.TYPE_ONE_SHOT)
> +        );
> +        durationRatio = Math.floor(Date.now() - start) / duration;
> +      }

I would request some feedback from jgraham on this, to check if this is his intention.  It looks technically correct to me.

::: testing/marionette/action.js:1249
(Diff revision 2)
> +        durationRatio = Math.floor(Date.now() - start) / duration;
> +      }
> +    });
> +    // perform last pointer move after all incremental moves are resolved and
> +    // durationRatio is close enough to 1
> +    intermediatePointerEvents.then(function() {

Use a pointer function or place a space after `function`.

::: testing/marionette/action.js:1261
(Diff revision 2)
> +  switch (inputState.subtype) {
> +    case action.PointerType.mouse:
> +      let mouseEvent = new action.Mouse("mousemove");
> +      mouseEvent.update(inputState);
> +      //TODO both pointermove (if available) and mousemove
> +      event.synthesizeMouseAtPoint(targetX, targetY, mouseEvent, win);
> +      break;
> +    case action.PointerType.pen:
> +    case action.PointerType.touch:
> +      throw new UnsupportedOperationError("Only 'mouse' pointer type is supported.");
> +  }

Add a default case, just… in case.

::: testing/marionette/action.js:1270
(Diff revision 2)
> +      //TODO both pointermove (if available) and mousemove
> +      event.synthesizeMouseAtPoint(targetX, targetY, mouseEvent, win);
> +      break;
> +    case action.PointerType.pen:
> +    case action.PointerType.touch:
> +      throw new UnsupportedOperationError("Only 'mouse' pointer type is supported.");

Drop full stop.

::: testing/marionette/action.js:1315
(Diff revision 2)
> +function inViewPort(x, y, win) {
> +  assert.number(x);
> +  assert.number(y);
> +  // Viewport includes scrollbars if rendered.
> +  return !(x < 0 || y < 0 || x > win.innerWidth || y > win.innerHeight);
> +}

Please use `element.isInView` instead.

::: testing/marionette/test_action.js:214
(Diff revision 2)
> +  run_next_test();
> +});
> +
> +add_test(function test_computePointerDestinationElement() {
> +  // origin represents a web element
> +  let act = { type: "pointerMove", x: 100, y: 200, origin: {}};

Can an origin be a dict?
Attachment #8835253 - Flags: review?(ato) → review+
Comment on attachment 8838772 [details]
Bug 1337133 - Add a wdspec test for mouse actions;

https://reviewboard.mozilla.org/r/113578/#review115274

::: testing/web-platform/tests/webdriver/actions/mouse_actions.py:5
(Diff revision 1)
> +    x = 82
> +    y = 187

Are these randomly chosen coordinates or do they have some significance?
Attachment #8838772 - Flags: review?(ato) → review+
Comment on attachment 8835250 [details]
Bug 1337133 - Add pointer actions endpoints to wdclient;

https://reviewboard.mozilla.org/r/110950/#review115260

> This appears to have been fixed now, but I guess the fix is not yet available in the geckodriver on central?
> 
> In the interim I’ve started work on https://bugzilla.mozilla.org/show_bug.cgi?id=1340637 to compile geckodriver master on the build slaves.  But I should point that it needs a follow-up patch to have wptrunner pick up the in-tree-built geckodriver.
> 
> I don’t know if that/the followup will land before this, but just making you aware of the ongoing work in this area.

Yep, waiting for availability on central. Thanks for the summary. :)
Comment on attachment 8835253 [details]
Bug 1337133 - Dispatch pointerMove action for mouse;

https://reviewboard.mozilla.org/r/110956/#review115272

> Please use `element.isInView` instead.

I'm using this to check destination pointer coordinates, which aren't necessarily associated with an element.

> Can an origin be a dict?

No, it would get rejected by `action.Action.fromJson`. Origin validation is tested in `test_processPointerMoveAction`. 

The origin value in this line is a dummy value to test the `default` case in `computePointerDestination`. I'll clarify that.
Comment on attachment 8838772 [details]
Bug 1337133 - Add a wdspec test for mouse actions;

https://reviewboard.mozilla.org/r/113578/#review115274

> Are these randomly chosen coordinates or do they have some significance?

Random coords that are well inside the `#outer` div that has event listeners attached in the test page. Changed the code a bit to make that clearer.
Comment on attachment 8835252 [details]
Bug 1337133 - Dispatch pointerDown and pointerUp actions for mouse;

Could you have a quick look to see if this matches your intention in the spec?
Attachment #8835252 - Flags: feedback?(james)
Comment on attachment 8835248 [details]
Bug 1337133 - Add MoveTargetOutOfBoundsError as a new Webdriver error type;

https://reviewboard.mozilla.org/r/110946/#review115256

Fixed.
Comment on attachment 8839273 [details]
Bug 1337133 - Fix creation of pointer InputState;

https://reviewboard.mozilla.org/r/113946/#review115914
Attachment #8839273 - Flags: review?(ato) → review+
Comment on attachment 8835251 [details]
Bug 1337133 - Set up fixtures for mouse actions in wdspec tests;

https://reviewboard.mozilla.org/r/110952/#review115916
Attachment #8835251 - Flags: review?(ato) → review+
Comment on attachment 8835253 [details]
Bug 1337133 - Dispatch pointerMove action for mouse;

https://reviewboard.mozilla.org/r/110956/#review115272

> I'm using this to check destination pointer coordinates, which aren't necessarily associated with an element.

Right, yes, understood.
Comment on attachment 8839680 [details]
Bug 1337133 - Move update of inputStateMap to action.Sequence.fromJson;

https://reviewboard.mozilla.org/r/114258/#review115926
Attachment #8839680 - Flags: review?(ato) → review+
Comment on attachment 8835253 [details]
Bug 1337133 - Dispatch pointerMove action for mouse;

https://reviewboard.mozilla.org/r/110956/#review115920

::: testing/marionette/action.js:1228
(Diff revision 4)
> + *     as appropriate.
> + */
> +function dispatchPointerMove(a, inputState, tickDuration, seenEls, container) {
> +  const timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
> +  // interval between pointermove increments in ms, based on common vsync
> +  const fps60 = 17;

I wonder if the 2/3ms per step drift from 60fps is important here.

::: testing/marionette/action.js:1240
(Diff revision 4)
> +    if (element.isWebElementReference(a.origin)) {
> +      let uuid = a.origin[element.Key] || a.origin[element.LegacyKey];
> +      let el = seenEls.get(uuid, container);
> +      let rects = el.getClientRects();
> +      if (rects.length == 0) {
> +        throw new MoveTargetOutOfBoundsError(error.pprint`Target element ${el}` +

I don't see this in the spec. Am I missing something?

::: testing/marionette/action.js:1270
(Diff revision 4)
> +      // wait |fps60| ms before performing first incremental pointer move
> +      yield new Promise(resolveTimer =>
> +          timer.initWithCallback(resolveTimer, fps60, ONE_SHOT)
> +      );
> +      let durationRatio = Math.floor(Date.now() - start) / duration;
> +      const epsilon = fps60 / 1000;

It's not really clear where this comes from. It seems like what you actully care about is whether `time - start > duration - fps60` i.e. whether we are at a time where we should reach the target in the next interval.

::: testing/marionette/action.js:1304
(Diff revision 4)
> +      //TODO both pointermove (if available) and mousemove
> +      event.synthesizeMouseAtPoint(targetX, targetY, mouseEvent, win);
> +      break;
> +    case action.PointerType.Pen:
> +    case action.PointerType.Touch:
> +      throw new UnsupportedOperationError("Only 'mouse' pointer type is supported");

Hopefully we already checked this earlier when processing the actions? Otherwise one can end up with a partially complete action chain.

::: testing/marionette/test_action.js:186
(Diff revision 4)
>  
>    }
>    run_next_test();
>  });
>  
> +add_test(function test_computePointerDestinationViewport() {

I would really like to see some wpt tests for this feature. Is there some reason that's hard at the moment?
Attachment #8835253 - Flags: review?(james) → review-
Comment on attachment 8835253 [details]
Bug 1337133 - Dispatch pointerMove action for mouse;

https://reviewboard.mozilla.org/r/110956/#review115920

> I wonder if the 2/3ms per step drift from 60fps is important here.

The spec says "model the overall pointer move as a series of small movements occuring at an implementation defined rate (e.g. one movement per vsync)." so my thinking is that an interval that is slightly less frequent than vsync guarantees that we don't dispatch an unnecessarily high number of movements.

> I don't see this in the spec. Am I missing something?

Whoops, brain fail: I was thinking of the element as the target, not the origin, when I added that line. 

Since it's actually alright for the origin to be outide the viewport, I can also simplify this section to use `element.coordinates(el)`.

> It's not really clear where this comes from. It seems like what you actully care about is whether `time - start > duration - fps60` i.e. whether we are at a time where we should reach the target in the next interval.

You're right. Since we're looking at the duration ratio, intuitively the epsilon should be based on `fps60 / duration`. If we make it exactly that, the last movement will be up to twice as big as the others in the series. So I'll make the epsilon smaller than `fps60 / duration` to err on the side of more precise movements when we're close to the target.

> Hopefully we already checked this earlier when processing the actions? Otherwise one can end up with a partially complete action chain.

Good point, will do. (There's no earlier check, now.)

> I would really like to see some wpt tests for this feature. Is there some reason that's hard at the moment?

Agreed. I wanted to write more wpt tests as a follow-up, but now that you mention it, I see there's no geckodriver support for pointer-move origin: https://github.com/mozilla/webdriver-rust/issues/71
Comment on attachment 8835253 [details]
Bug 1337133 - Dispatch pointerMove action for mouse;

https://reviewboard.mozilla.org/r/110956/#review115920

> Agreed. I wanted to write more wpt tests as a follow-up, but now that you mention it, I see there's no geckodriver support for pointer-move origin: https://github.com/mozilla/webdriver-rust/issues/71

Until geckodriver is updated, perhaps I can reassure you with the following test, which passes locally with the changes in my next review request:

```python
def test_click_element_center(session, test_actions_page, mouse_chain):
    outer = session.find.css("#outer", all=False)
    outer_rect = outer.rect
    center_x = outer_rect["width"] / 2 + outer_rect["x"];
    center_y = outer_rect["height"] / 2 + outer_rect["y"];
    button = 0
    mouse_chain \
    # using element instead of origin to circumvent out-of-date webdriver-rust
        .pointer_move(0, 0, element=outer) \
        .pointer_down(button) \
        .pointer_up(button) \
        .perform()

    events = get_events(session)
    assert len(events) == 4
    for e in events:
        if e["type"] == "mousedown":
            # TODO use pytest.approx once we upgrade to pytest > 3.0
            assert abs(e["pageX"] - center_x) < 1
            assert abs(e["pageY"] - center_y) < 1
            assert e["target"] == "outer"
```
Comment on attachment 8835253 [details]
Bug 1337133 - Dispatch pointerMove action for mouse;

https://reviewboard.mozilla.org/r/110956/#review115920

> Good point, will do. (There's no earlier check, now.)

Earlier check added in https://reviewboard.mozilla.org/r/110954/diff/4-5/
Comment on attachment 8840129 [details]
Bug 1337133 - Remove references to pen and touch in test_action.js;

https://reviewboard.mozilla.org/r/114648/#review116384
Attachment #8840129 - Flags: review?(james) → review+
Comment on attachment 8835253 [details]
Bug 1337133 - Dispatch pointerMove action for mouse;

https://reviewboard.mozilla.org/r/110956/#review116388
Attachment #8835253 - Flags: review?(james) → review+
Attachment #8835247 - Attachment is obsolete: true
I've confirmed that the merge conflicts post-rebase haven't changed anything, and that the wpt tests pass with the harness prior to the wpt update in Bug 1340474 (which broke the wpt harness as in https://bugzilla.mozilla.org/show_bug.cgi?id=1308969#c26).
Pushed by mjzffr@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7902afebdf2f
Add number check to assert.js; r=ato+446296
https://hg.mozilla.org/integration/autoland/rev/397ef772d437
Remove primary pointer attribute; r=ato+446296
https://hg.mozilla.org/integration/autoland/rev/0aa0fd540ef0
Replace element with origin for pointerMove actions; r=ato+446296
https://hg.mozilla.org/integration/autoland/rev/2d7d58a97e46
Add MoveTargetOutOfBoundsError as a new Webdriver error type; r=ato+446296
https://hg.mozilla.org/integration/autoland/rev/be1184fd9911
Split test kinds, common fixtures, utilities in wdspec tests; r=ato+446296
https://hg.mozilla.org/integration/autoland/rev/1e827bb8b11a
Add pointer actions endpoints to wdclient; r=ato+446296
https://hg.mozilla.org/integration/autoland/rev/86ed4af7aed4
Set up fixtures for mouse actions in wdspec tests; r=ato+446296
https://hg.mozilla.org/integration/autoland/rev/e7edf0e6f5f8
Add more MouseEvent event properties to synthesizeMouseAtPoint; r=ato+446296
https://hg.mozilla.org/integration/autoland/rev/6d8950160186
Fix creation of pointer InputState; r=ato+446296
https://hg.mozilla.org/integration/autoland/rev/b2fc3f9520fe
Remove references to pen and touch in test_action.js; r=jgraham
https://hg.mozilla.org/integration/autoland/rev/aba48a32380e
Dispatch pointerDown and pointerUp actions for mouse; r=ato+446296
https://hg.mozilla.org/integration/autoland/rev/356602cac222
Dispatch pointerMove action for mouse; r=ato+446296,jgraham
https://hg.mozilla.org/integration/autoland/rev/47ac31461e9d
Add a wdspec test for mouse actions; r=ato+446296
https://hg.mozilla.org/integration/autoland/rev/fa0e8a07bd1e
Move update of inputStateMap to action.Sequence.fromJson; r=ato+446296
Sheriffs: Please uplift to Aurora and Beta under test-only.
Whiteboard: [checkin-needed-aurora][checkin-needed-beta]
Beta is unrealistic because the merge from beta -> release happened yesterday. We might be able to try to uplift to esr52 later if allowed.
Whiteboard: [checkin-needed-aurora][checkin-needed-beta] → [checkin-needed-aurora]
needs rebasing for aurora because failed like 

^[[CTomcats-MBP-462:mozilla-central Tomcat$ hg graft -er 397ef772d437
grafting 401816:397ef772d437 "Bug 1337133 - Remove primary pointer attribute; r=ato+446296"
merging testing/marionette/action.js
merging testing/marionette/test_action.js
warning: conflicts while merging testing/marionette/action.js! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(mjzffr)
Whiteboard: [checkin-needed-aurora]
Test-only uplift to aurora and esr52, please. Please uplift Bug 1335240, first, which will fix the above conflict. Thanks.
Flags: needinfo?(mjzffr)
Whiteboard: [checkin-needed-aurora][checkin-needed-esr52]
I made it as far as rev 1e827bb8b11a before hitting more conflicts (and that was after adding bug 1336214 into the mix to get past some earlier conflicts). I think you're going to need to provide a branch patch or three here. Please check the remaining commits for conflicts as well before requesting uplift again.
Flags: needinfo?(mjzffr)
Whiteboard: [checkin-needed-aurora][checkin-needed-esr52]
Sorry. Let's try that again. Please uplift to aurora, a=testonly. Do the uplift for Bug 1328726 first.
Flags: needinfo?(mjzffr)
(I'll tackle the esr52 request when I return on Friday.)
Whiteboard: [checkin-needed-aurora]
I didn't get to setting up the other uplift patches today. The next chance I'll get will be on Tuesday.
Whats left to do to get this into ESR52?
Flags: needinfo?(mjzffr)
Please uplift the attached patch to esr52, test-only.
Note: there have been lots of changes to the wpt harness since 52, including session setup and tear-down, so I had to fix conflicts and disable some tests. All the actions tests work as on 54.
Flags: needinfo?(mjzffr)
Whiteboard: [checkin-needed-esr52]
applying the pointer_actions esr52 patch cause

:

patching file testing/web-platform/meta/MANIFEST.json
Hunk #1 FAILED at 38754
1 out of 1 hunks FAILED -- saving rejects to file testing/web-platform/meta/MANIFEST.json.rej
unable to find 'testing/web-platform/meta/webdriver/actions.py.ini' for patching
(use '--prefix' to apply patch relative to the current directory)
1 out of 1 hunks FAILED -- saving rejects to file testing/web-platform/meta/webdriver/actions.py.ini.rej
unable to find 'testing/web-platform/tests/test_keys_wdspec.html' for patching
(use '--prefix' to apply patch relative to the current directory)
1 out of 1 hunks FAILED -- saving rejects to file testing/web-platform/tests/test_keys_wdspec.html.rej
unable to find 'testing/web-platform/tests/webdriver/actions.py' for patching
(use '--prefix' to apply patch relative to the current directory)
1 out of 1 hunks FAILED -- saving rejects to file testing/web-platform/tests/webdriver/actions.py.rej
unable to find 'testing/web-platform/tests/webdriver/support/keys.py' for patching
(use '--prefix' to apply patch relative to the current directory)
1 out of 1 hunks FAILED -- saving rejects to file testing/web-platform/tests/webdriver/support/keys.py.rej
patching file testing/web-platform/tests/tools/webdriver/webdriver/client.py
Hunk #1 FAILED at 77
1 out of 1 hunks FAILED -- saving rejects to file testing/web-platform/tests/tools/webdriver/webdriver/client.py.rej
testing/web-platform/tests/webdriver/support/__init__.py not tracked!
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh pointer_actions_esr52.patch

from a clean clone the directory of mozilla-esr52/testing/web-platform/tests/webdriver

Tomcats-MacBook-Pro-820:webdriver Tomcat$ ls -all
total 40
drwxr-xr-x    7 Tomcat  wheel   238 Mar 13 11:09 .
drwxr-xr-x  114 Tomcat  wheel  3876 Mar 13 11:09 ..
-rw-r--r--    1 Tomcat  wheel    44 Mar 13 11:09 OWNERS
-rw-r--r--    1 Tomcat  wheel  2458 Mar 13 11:09 README.md
-rw-r--r--    1 Tomcat  wheel   900 Mar 13 11:09 contexts.py
-rw-r--r--    1 Tomcat  wheel   506 Mar 13 11:09 interface.html
-rw-r--r--    1 Tomcat  wheel  3062 Mar 13 11:09 navigation.py

so that seems to explain some errors from above.

Maja do i miss something here ?
Flags: needinfo?(mjzffr)
Thanks for the info. Could you try again? I'm fairly confident that the patch should apply cleanly. It does on my end, even on a fresh clone.

If you look at https://hg.mozilla.org/releases/mozilla-esr52/file/tip/testing/web-platform/tests/webdriver, the expected files, like actions.py, are there.

Those files got added to esr52 in the following push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-esr52&revision=a1b92421760f87cc141ea33aa51e7206cf1d17df

Namely, with:
> hg log --removed --stat testing/web-platform/tests/webdriver/actions.py testing/web-platform/meta/webdriver/actions.py.ini testing/web-platform/tests/test_keys_wdspec.html testing/web-platform/tests/webdriver/support/keys.py testing/web-platform/tests/webdriver/support/__init__.py -r "::esr52"

We can see:
changeset:   398788:f98c4db03e53
user:        Maja Frydrychowicz 
date:        Tue Jan 24 16:59:04 2017 -0500
summary:     Bug 1328726 - Add web-platform wdspec tests for key actions. r=ato, r=jgraham, a=test-only

 testing/web-platform/tests/test_keys_wdspec.html         |   55 ++++++++
 testing/web-platform/tests/webdriver/actions.py          |  231 +++++++++++++++++++++++++++++++++++++
 testing/web-platform/tests/webdriver/support/__init__.py |    0
 testing/web-platform/tests/webdriver/support/keys.py     |  812 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 1098 insertions(+), 0 deletions(-)

changeset:   398789:17913deae97f
user:        Maja Frydrychowicz 
date:        Wed Mar 08 17:22:58 2017 -0500
summary:     Bug 1328726 - Add wdspec test for emoji in key actions. r=ato, r=jgraham, a=test-only

 testing/web-platform/tests/test_keys_wdspec.html |  31 +++++++++++++++++++++++--------
 testing/web-platform/tests/webdriver/actions.py  |  48 ++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 61 insertions(+), 18 deletions(-)

changeset:   398790:a1b92421760f
user:        Maja Frydrychowicz 
date:        Wed Jan 25 17:16:45 2017 -0500
summary:     Bug 1328726 - Disable wdspec tests on linux64 debug. r=jgraham, a=test-only

 testing/web-platform/meta/webdriver/actions.py.ini |  5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)


One of the changesets in the supplied patch moves a bunch of these files elsewhere (Bug 1337133 - Split test kinds, common fixtures, utilities in wdspec tests.) Maybe that's where you're running into trouble? But again, the patch import works for me.

Thanks.
Flags: needinfo?(mjzffr)
Hi Maja, i tried again and it failed again with a super new clone.

From 

cannot patch testing/web-platform/tests/webdriver/actions/conftest.py: file is not tracked.

Log from the patching

renaming 1337133 to pointer_actions_esr52.patch
applying pointer_actions_esr52.patch
patching file testing/marionette/assert.js
patching file testing/marionette/test_assert.js
patching file testing/marionette/action.js
patching file testing/marionette/test_action.js
patching file testing/marionette/action.js
patching file testing/marionette/test_action.js
patching file testing/marionette/error.js
patching file testing/web-platform/meta/MANIFEST.json
patching file testing/web-platform/meta/webdriver/actions.py.ini
patching file testing/web-platform/meta/webdriver/actions/key.py.ini
patching file testing/web-platform/meta/webdriver/actions/sequence.py.ini
patching file testing/web-platform/tests/test_keys_wdspec.html
patching file testing/web-platform/tests/webdriver/actions.py
patching file testing/web-platform/tests/webdriver/actions/conftest.py
patching file testing/web-platform/tests/webdriver/actions/key.py
patching file testing/web-platform/tests/webdriver/actions/mouse.py
patching file testing/web-platform/tests/webdriver/actions/sequence.py
patching file testing/web-platform/tests/webdriver/actions/support/keys.py
patching file testing/web-platform/tests/webdriver/actions/support/refine.py
patching file testing/web-platform/tests/webdriver/actions/support/test_actions_wdspec.html
patching file testing/web-platform/tests/webdriver/support/keys.py
patching file testing/web-platform/tests/tools/webdriver/webdriver/client.py
patching file testing/web-platform/tests/webdriver/actions/conftest.py
adding testing/web-platform/meta/webdriver/actions/key.py.ini
adding testing/web-platform/meta/webdriver/actions/sequence.py.ini
adding testing/web-platform/tests/webdriver/actions/__init__.py
adding testing/web-platform/tests/webdriver/actions/conftest.py
adding testing/web-platform/tests/webdriver/actions/key.py
adding testing/web-platform/tests/webdriver/actions/mouse.py
adding testing/web-platform/tests/webdriver/actions/sequence.py
adding testing/web-platform/tests/webdriver/actions/support/__init__.py
adding testing/web-platform/tests/webdriver/actions/support/keys.py
adding testing/web-platform/tests/webdriver/actions/support/refine.py
adding testing/web-platform/tests/webdriver/actions/support/test_actions_wdspec.html
cannot patch testing/web-platform/tests/webdriver/actions/conftest.py: file is not tracked
committing files:
testing/marionette/action.js
testing/marionette/assert.js
testing/marionette/error.js
testing/marionette/test_action.js
testing/marionette/test_assert.js
testing/web-platform/meta/MANIFEST.json
testing/web-platform/meta/webdriver/actions/key.py.ini
testing/web-platform/meta/webdriver/actions/sequence.py.ini
testing/web-platform/tests/tools/webdriver/webdriver/client.py
testing/web-platform/tests/webdriver/actions/__init__.py
testing/web-platform/tests/webdriver/actions/conftest.py
testing/web-platform/tests/webdriver/actions/key.py
testing/web-platform/tests/webdriver/actions/mouse.py
testing/web-platform/tests/webdriver/actions/sequence.py
testing/web-platform/tests/webdriver/actions/support/__init__.py
testing/web-platform/tests/webdriver/actions/support/keys.py
testing/web-platform/tests/webdriver/actions/support/refine.py
testing/web-platform/tests/webdriver/actions/support/test_actions_wdspec.html
committing manifest
committing changelog
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh pointer_actions_esr52.patch
Flags: needinfo?(mjzffr)
Not sure why it's not working for you.

<maja_zf> KWierso: hey, could you check if this uplift patch applies cleanly for you? https://bugzilla.mozilla.org/show_bug.cgi?id=1337133#c137 -- I can't reproduce tomcat's issue. Maybe the difference is that he's using mq and I'm not?
<•KWierso> maja_zf: yeah, applies to esr52 just fine for me
Flags: needinfo?(mjzffr)
The same patch (https://bug1337133.bmoattachments.org/attachment.cgi?id=8846216) should also be uplifted to release. I've tested the patch locally and it applies without issue. (Bug 1328726 needs to be uplifted first.)
Whiteboard: [checkin-needed-esr52] → [checkin-needed-esr52] [checkin-needed-release]
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f257c058033a
Add pointer actions endpoints to wdclient; r=ato+446296
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/mozilla-inbound/rev/85aec7a95087
Add pointer actions endpoints to wdclient; r=ato+446296
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b4d0ea9dc717
Add pointer actions endpoints to wdclient; r=ato+446296
Flags: needinfo?(james)
https://hg.mozilla.org/mozilla-central/rev/1ee09646e328
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.