494071 - Using keypress without modifiers doesn't insert characters into text fields

Status: VERIFIED FIXED

(Testing Graveyard :: Mozmill, defect)

Description

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

This bug is a regression from the work on bug 483172. As the summary says it is not possible to enter characters into text fields e.g. the locationbar.

As an example see attachment 377542 [details].

Comment 1

[Adam Christian]

So it turns out that to do the kinds of things whimboo wants to do we are going to have to change the key event API.

We can translate from a string to a charCode, but there is currently no way to allow for a keyCode of a charCode of null or 0, whkch is required for something like backspace (8) and our logic currently will just guess that you meant you wanted the charCode for 8 and turn it into 56.

I want to see what other people think, but I think a better syntax for all this would be:

  controller.keypress(urlbar, null, 8, modifiers);

In this example the empty string is either a string character or char code, and 8 is the keyCode. Since all the modifiers are false, this would just be an empty object.

Another example:

  controller.keypress(urlbar, '1', null, {});

This way you have the control to specify what you want, and the modifier syntax is cleaner instead of a bunch of ugly booleans.

  controller.keypress(urlbar, '1', null, {altKey:true, metaKey:true});

Everyone think this is reasonable?

Comment 2

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

That looks great. It's a similar way like it is used in mochitests. Having an array with the modifiers is much cleaner as having to add them as different parameters.

I would like to see this way. Any concerns? What about the Thunderbird guys? CC'ing Mark and Gary.

Comment 3

[Adam Christian]

All my tests have worked correctly with the new api, Committed revision 465.

I also tried this test with the modified API and it works as expected:

var mozmill = {}; Components.utils.import('resource://mozmill/modules/mozmill.js', mozmill);
var elementslib = {}; Components.utils.import('resource://mozmill/modules/elementslib.js', elementslib);

var setupModule = function(module) {
  controller = mozmill.getBrowserController();
}

var testRecorded = function () {
  var urlbar = new elementslib.Lookup(controller.window.document, '/id("main-window")/id("navigator-toolbox")/id("nav-bar")/id("urlbar-container")/id("urlbar")/anon({"class":"autocomplete-textbox-container"})/anon({"anonid":"textbox-input-box"})/anon({"anonid":"input"})');
  
  // enter a search term into the urlbar
  controller.type(urlbar, "term");
  controller.sleep(1000);
  
  // explicitely click the urlbar
  controller.click(urlbar);
  
  // Enter a letter and remove it with backspace
  controller.keypress(urlbar, 49, null, {});
  controller.sleep(1000);
  controller.keypress(urlbar, null, 8, {});
  controller.sleep(1000);
  
  // press return
  controller.keypress(urlbar, null, 13, {});
}

Comment 4

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

Attachment: example patch for better keypress handling (checked-in)

Adam, I'm not fully happy with this solution. Why we cannot do it the way as I have it rawly implemented inside the patch? We do not have to differentiate between special virtual keys or normal characters. That is handled inside the synthesizeKey function (which also has to be patched due to the missing window scope). Further we do not have to differentiate between window or an element as target. We just fire the same code. Probably we would need a focus check which is not implemented right now. I'll attach an example which I really love!

Comment 5

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

Attachment: example for proposed solution

This will work with the latest proposal.

Comment 6

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

Attachment: correct example

Now with the saved content.

Comment 7

[Adam Christian]

Committed revision 469.

Comment 8

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

Attachment: Followup patch

This is a follow-up patch to fix the type and open functions which will break right now due to the changes.

Comment 9

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

Attachment: Full example test for all changes

Comment 10

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

Attachment: Follow-up patch against extension folder (checked-in)

Comment 11

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

Adam, you have already checked in those changes right? Further we need an updated Mozmill documentation for keypress. Adding whiteboard entry.

Comment 12

[Adam Christian]

yes the patch is checked in.

Comment 13

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

Ok, the follow-up is http://code.google.com/p/mozmill/source/detail?r=470

Marking fixed. I'll verify with all the updated tests shortly.

Comment 14

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

I have updated existing Mozmill tests for Firefox. Everything looks fine. With the new API we also detect if elements are not visible! Yay!

Comment 15

[David Bolter [:davidb] (NeedInfo me for attention)]

So is bug 490548 still valid?

Comment 16

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

Yes, it is for mouse events. We should probably check to use those EventUtils functions too which simulates mouse events. I'll run a test the next days to check that.

Comment 17

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

Docs are already uptodate.