1801601 - Incorrect usage of event.sendKeyDown/sendKeyUp in legacyaction.sys.mjs

Status: NEW

(Remote Protocol :: Marionette, defect, P3)

Description

[Julian Descottes [:jdescottes]]

The code at https://searchfox.org/mozilla-central/rev/83b86005c6913c2062419efb8aabdf2e683aa47f/remote/marionette/legacyaction.sys.mjs#303-312

    case "keyDown":
      lazy.event.sendKeyDown(pack[1], keyModifiers, this.container.frame);
      this.actions(chain, touchId, i, keyModifiers, cb);
      break;

    case "keyUp":
      lazy.event.sendKeyUp(pack[1], keyModifiers, this.container.frame);
      this.actions(chain, touchId, i, keyModifiers, cb);
      break;

uses the wrong signature for sendKeyDown & sendKeyUp. The methods take 2 arguments:

/**
 * Synthesize a keydown event for a single key.
 *
 * @param {Object} key
 *     Key data as returned by keyData.getData
 * @param {Window} win
 *     Window object.
 */
event.sendKeyDown = function(key, win) {
  event.sendSingleKey(key, win, "keydown");
};

/**
 * Synthesize a keyup event for a single key.
 *
 * @param {Object} key
 *     Key data as returned by keyData.getData
 * @param {Window} win
 *     Window object.
 */
event.sendKeyUp = function(key, win) {
  event.sendSingleKey(key, win, "keyup");
};

Note that this code is not covered according to moz-coverage.

Comment 1

[Julian Descottes [:jdescottes]]

Is it correct that legacyactions are only used for touch events now and we could remove the non-touch helpers?

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1686666

:jgraham, since you are the author of the regressor, bug 1686666, could you take a look? Also, could you set the severity field?

For more information, please visit auto_nag documentation.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Marionette:SingleTap is the only command in use from legacyactions.sys.mjs right now and on bug 1792529 I want to get rid of it. Nevertheless before this can happen (after the a11y question has been solved) it should be fine to get rid of all the not used methods.

Comment 4

[Julian Descottes [:jdescottes]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #3)

Marionette:SingleTap is the only command in use from legacyactions.sys.mjs right now and on bug 1792529 I want to get rid of it. Nevertheless before this can happen (after the a11y question has been solved) it should be fine to get rid of all the not used methods.

Sounds good, let's remove the ni? for James

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Maybe we could remove as many as possible methods for now including dispatchActions which doesn't seem to be used anymore based on this review comment?

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:whimboo, could you have a look please?

For more information, please visit auto_nag documentation.