Closed Bug 1255806 Opened 8 years ago Closed 8 years ago

marionette/event.js is still using obsolete nsIDOMWindowUtils.sendKeyEvent()

Categories

(Remote Protocol :: Marionette, defect)

defect
Not set
major

Tracking

(firefox48 fixed, firefox49 fixed)

RESOLVED FIXED
mozilla49
Tracking Status
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: masayuki, Assigned: automatedtester)

References

Details

(Keywords: pi-marionette-server)

Attachments

(1 file)

marionette/event.js is still using nsIDOMWindowUtils.sendKeyEvent() which was already obsolete for a year (at least).

When I work on bug 1154183, I hit this issue. Bug 1154183 will change shortcut key handling of reserved shortcut keys (e.g., Ctrl+T, Ctrl+W, Ctrl+N). They will be handled with keydown event. But charCode of keydown event is always 0. Therefore, native event handler and TextInputProcessor add some necessary information to WidgetKeyboardEvent which is internal class of DOM KeyboardEvent.

However, nsIDOMWindowUtils.sendKeyEvent() cannot set such additional information due to its poor arguments. Therefore, all automated tests should use nsITextInputProcessor instead.

Looks like that marionette/event.js is ported from EventUtils.js for mochitests. So, marionette staff should port current EventUtils.synthesizeKey() and _getTIP().

Andreas Tolfsen, could you take a look for this? Bug 1154183 should be fixed as soon as possible.
Flags: needinfo?(ato)
I changed the approach to fix bug 1154183, then, this becomes not necessary to fix soon. However, this should be done for bug 1134542, anyway. So, I still hope that somebody of Marionette staff tries to fix this bug.
No longer blocks: 1154183
Assignee: nobody → dburns
Flags: needinfo?(ato)
nsIDOMWindowUtils.sendKeyEvent() has been made obsolete so this removes its use
in Marionette

Review commit: https://reviewboard.mozilla.org/r/51715/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51715/
Attachment #8750981 - Flags: review?(ato)
Can you confirm whether this patch is an import of (select features from) the newer EventUtils.js used by Mochitests?
Flags: needinfo?(dburns)
(In reply to Andreas Tolfsen ‹:ato› from comment #4)
> Can you confirm whether this patch is an import of (select features from)
> the newer EventUtils.js used by Mochitests?

Yup, literally copied things over and then "restyled" them to match the rest of the file.
Flags: needinfo?(dburns)
Comment on attachment 8750981 [details]
MozReview Request: Bug 1255806: remove use of nsIDOMWindowUtils.sendKeyEvent from Marionette r?ato

https://reviewboard.mozilla.org/r/51715/#review49081

Well try appears to be passing, which wasn’t the case when I tried to import the whole, new Mochitest EventUtils.js file. Please ship it.
Attachment #8750981 - Flags: review?(ato) → review+
https://hg.mozilla.org/mozilla-central/rev/1f0dc5f2cf72
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.