SlimerJS not compatible with Firefox 60 : key support issues

[kaosko]

versions

• SlimerJS: 1.0.0, 1.1.0-pre
• Firefox: 60 (59 works ok)
• Operating system:
  Ubuntu 16.04.2 LTS

Steps to reproduce the issue

In application.ini, change
MaxVersion=60.*
Try to take a screenshot of any webpage with slimer.js. Add --debug=true

Actual results:

The last important lines with --debug=true:
JavaScript strict warning: resource://slimerjs/slQTKeyCodeToDOMCode.jsm, line 33: ReferenceError: reference to undefined property "nsIDOMKeyEvent"

Script Error: c is undefined
Stack:
-> resource://slimerjs/slQTKeyCodeToDOMCode.jsm: 36
-> resource://slimerjs/slQTKeyCodeToDOMCode.jsm: 36
-> resource://slimerjs/slimer-sdk/webpage.js: 15

Expected results:

Able to take a screenshot and operate normally.

[kaosko]

Poked around a little bit and asked about it at #firefox IRC channel:

10:19 kaosko2 Components.interfaces.nsIDOMKeyEvent is gone in FF 60. seems like KeyboardEvent is the replacement. how I access it (is that a class?) in a plugin?
10:22 Mossop kaosko2: It should just be available in the DOM
10:23 kaosko2 sure ok but I don't even have the window, is there a different way to access it? trying to fix https://github.com/laurentj/slimerjs/blob/master/src/modules/slQTKeyCodeToDOMCode.jsm
10:24 kaosko2 it happily calls: const c = Components.interfaces.nsIDOMKeyEvent; which now fails in FF 60
10:32 adrian17 (looks like FF devtools code also keeps an independent key code mapping, maybe you could copy it instead: https://searchfox.org/mozilla-central/source/devtools/client/shared/keycodes.js )
10:34 kaosko2 adrian17 thanks - how would I access that?
10:37 kaosko2 adrian17 mm yeah I guess. not a huge fan keeping yet another copy but yeah I guess that'd be one way

As a test, I copied the keycodes from https://searchfox.org/mozilla-central/source/devtools/client/shared/keycodes.js, then changed slQTKeyCodeToDOMCode.jsm:

const KeyCodes {
...
}
const c = KeyCodes;

That seemed to be the only error with FF 60. Copying them is probably not the best way to go but you be the judge.

[kaosko]

Perhaps convertQTKeyCode should simply accept window or the KeyboardEvent as a parameter, made a quick hack to validate that would work. All the calls to it are in webpage.js so obviously questionable whether the function should then remain in slQTKeyCodeToDOMCode.jsm at all, but at least this way you wouldn't need to maintain a separate key code mapping.

[justinklemm]

@kaosko Nice work. I just saw this come up and came here to see if anyone had investigated yet. Any chance you'd be willing to share the update you made to convertQTKeyCode so we can take a look? A PR would be great, but a quick copy/paste would be useful too. I haven't jumped into the code myself yet.

[kaosko]

Like I said, that was just a total hack. I basically made c a var and passed window to the function:

function convertQTKeyCode(QTKeyCode, window){
	c = window.top.KeyboardEvent;

That's certainly not what you want to do in actual code. Several ways to go about it but right off the bat, I might just drop the whole QtKeys hashmap and refactor it all into one big switch/case statement. Had I been super convinced that's the best way, I would have already sent you a pull request. Happy to do it though if you agree that's the way to go. What's the significance of adding that 'k' in the key code? That wasn't too clear to me.

[justinklemm]

@kaosko Ah, gotcha. Thanks for clarifying. I'm not an actual maintainer of the repo, just I use it and usually pop in when a Firefox upgrade presents issues. We'll need @laurentj to take a look and give his thoughts on the best approach to this issue.

[laurentj]

Hi,

Thank you for investigations. I'll will add the keycode.js or perhaps I will pass the window object. There are other issues related to keyEvent. For example they also dropped the nsIDOMWindowsUtils.sendKeyEvent() method, so webpage.sendEvent() does not work anymore. I will fix that.

I might just drop the whole QtKeys hashmap and refactor it all into one big switch/case statement.

Well, a switch/case statement in this case will be slower than the actual hashmap IMHO, as keys are indexed in the hashmap..

What's the significance of adding that 'k' in the key code

Nothing. I didn't like numbers as keys in a hashmap, but I admit this is stupid :-)

For the record, convertQTKeyCode exists because in PhantomJS, key codes to give to webpage.sendEvent() were in fact QT key codes...

[birtles]

/cc @masayuki-nakano who can recommend what to use instead of this (since he removed these interfaces)

[masayuki-nakano]

Unfortunately, it requires to access window.KeyboardEvent as above comments discussed. So, module needs to receive a window or KeyboardEvent object at least once.

FYI: nsIDOMWindowsUtils.sendKeyEvent() was not good API since it was too stateless. I.e., even when some attributes should be overwritten with fixed values, it dispatched keyboard events with odd attribute values or odd keyboard event type. Now, it was replaced with nsITextInputProcessor. Perhaps, this out test API is a good example of how to use it:
https://searchfox.org/mozilla-central/rev/eb51d734a8c06ac5973291be9bcc74ed4e999bca/testing/mochitest/tests/SimpleTest/EventUtils.js#883-961

[MarkR42]

When running unit tests, for me it fails very early on with a syntax error in

src/modules/addon-sdk/sdk/url.js

about line 131.

I can fix this trivially, I think. After that, the sendKeyEvent problem described above, dominates.

[MarkR42]

I prepared this branch

https://github.com/MarkR42/slimerjs/tree/firefox60

Which works for me in Firefox 60, however, it fails a lot of unit tests because of the sendKeyEvent problems described above. (I do not use sendKeyEvent in my applications)