Keyboard shortcuts don't work 100% in Google Chrome

[Joel]

I'm not sure if this has been logged already, but keyboard shortcuts
don't work 100% in Google Chrome. They all seem to work fine in
Firefox, which I find kind of ironic, considering Gerrit seems to be a
Google project. I guess everyone isn't forced to use Google Chrome at
Google?

The shortcuts which don't work properly in Google Chrome are (Tested
in Ubuntu 10.04 and Windows XP):
<Ctrl> + d : discard draft comment, it adds a bookmark instead.
<Ctrl> + s : save draft comment, it tries to "Save File" instead.
c : Create a new inline comment. It does work, but when the draft
textarea box appears it has the "c" character in it.

Cheers,

-Joel

[Pat Notz]

On Sun, Jul 25, 2010 at 9:37 PM, Joel <joel.p...@gmail.com> wrote:
> The shortcuts which don't work properly in Google Chrome are (Tested
> in Ubuntu 10.04 and Windows XP):
> <Ctrl> + d : discard draft comment, it adds a bookmark instead.
> <Ctrl> + s : save draft comment, it tries to "Save File" instead.
> c : Create a new inline comment.  It does work, but when the draft
> textarea box appears it has the "c" character in it.

in Chrome 5.0.375.99 on MacOS X 10.5.8 I observe the same problem with
the 'c' shortcut. <Ctrl>+d and <Ctrl>+s don't work at all.

[Shawn Pearce]

Yea, I've seen this and have yet to fix it. I do use Google Chrome,
but we're not forced to use it, or any other browser for that matter.
What's odd is, this seems like a bug in Chrome because IIRC these keys
worked in Safari. Safari and Chrome are both based on WebKit and tend
to have similar behaviors around such things. At least Chrome is
consistent between Linux and Mac OS X. :-)

I stopped trying to track the bug down because as far as I can tell,
the JavaScript code is correct for the event handling here. Chrome
just isn't giving me Ctrl-S or Ctrl-D when in the comment edit box,
and even though I canceled the "c" event when I made the comment edit
box, Chrome is still stuffing a "c" into it. It might be possible to
work around the "c" thing by testing to see if this is Chrome, and if
so, if the box contains just "c", and if so, delete the text. Ick.

[Joel]

When I do <Ctrl> + s in Gmail, it saves a draft in Google Chrome
instead of opening up the save box. Gmail and Gerrit both use GWT
don't they? Or is Gmail some super special custom version of GWT?

I just tried this on Safari 5.0 (7533.16) in Windows, and I have
exactly the same problem as Google Chrome, so it looks like maybe it's
a problem with newer versions of WebKit?

I did a quick search about how control keys work in Javascript and it
seems there is some property on the javascript keydown event object
called ctrlKey.

I found a small jQuery plugin that worked in Google Chrome:

Taken from: http://www.gmarwaha.com/blog/2009/06/16/ctrl-key-combination-simple-jquery-plugin/

$.ctrl = function(key, callback, args) {
$(document).keydown(function(e) {
if(!args) args=[]; // IE barks when args is null
if(e.keyCode == key.charCodeAt(0) && e.ctrlKey) {
callback.apply(this, args);
return false;
}
});
};

And can be used with:
$.ctrl('S', function() {
alert("Saved");
});

I'm guessing you can't use jQuery in Gerrit, but I'm sure all the
general principles apply. I think the key thing in the keydown event
handler is probably the return false, which should be preventing the
default action, which in this case is the browsers default action of
save page. But there could be some other stuff going on in the jQuery
library that knows how to handle webkit better.

With the "c" problem, to me it looks like the keydown event is not
being stopped properly, which in jQuery I'd use event.stopPropagation
http://api.jquery.com/event.stopPropagation/

I'm happy to have a play around with this problem if you can point me
to the code that handles the "<Ctrl> + s" event and the "c" event. My
GWT mojo is very low (I've only done the Stock Watcher tutorial) so it
would take me forever to find the correct line of code.

Cheers,

-Joel

On Jul 27, 3:13 am, Shawn Pearce <s...@google.com> wrote:
> On Mon, Jul 26, 2010 at 06:04, Pat Notz <patn...@gmail.com> wrote:

[Shawn Pearce]

On Mon, Jul 26, 2010 at 16:28, Joel <joel.p...@gmail.com> wrote:
> When I do <Ctrl> + s in Gmail, it saves a draft in Google Chrome
> instead of opening up the save box.  Gmail and Gerrit both use GWT
> don't they?  Or is Gmail some super special custom version of GWT?

GMail predates GWT and thus doesn't use it.

> I just tried this on Safari 5.0 (7533.16) in Windows, and I have
> exactly the same problem as Google Chrome, so it looks like maybe it's
> a problem with newer versions of WebKit?

Right, that's my thought.

> I'm happy to have a play around with this problem if you can point me
> to the code that handles the "<Ctrl> + s" event and the "c" event.  My
> GWT mojo is very low (I've only done the Stock Watcher tutorial) so it
> would take me forever to find the correct line of code.

Look at the CommentEditorPanel class. On line 97 is where we are
trying to handle the key:

if ((event.isControlKeyDown() || event.isMetaKeyDown())
&& !event.isAltKeyDown() && !event.isShiftKeyDown()) {
switch (event.getCharCode()) {
case 's':
event.preventDefault();
onSave(NULL_CALLBACK);
return;

case 'd':
event.preventDefault();
if (isNew()) {
onDiscard();
} else if (Window.confirm(PatchUtil.C.confirmDiscard())) {
onDiscard();
} else {
text.setFocus(true);
}
return;
}

[Joel]

On Jul 27, 9:35 am, Shawn Pearce <s...@google.com> wrote:
> GMail predates GWT and thus doesn't use it.

Right, I will have to stop telling people gmail is an example of gwt
then ;-)

> Look at the CommentEditorPanel class.  On line 97 is where we are
> trying to handle the key:

Thanks for this, I'll see if I can make any sense of it.

[Joel]

Hi Shawn,

It seems that the problem is related to the fact that you have to use
the addKeyDownHandler instead of addKeyPressHandler

This code works still in firefox and mostly works in google chrome:

text.addKeyDownHandler(new KeyDownHandler() {
@Override
public void onKeyDown(final KeyDownEvent event) {
if (event.getNativeKeyCode() == KeyCodes.KEY_ESCAPE
&& !event.isAnyModifierKeyDown()) {
event.preventDefault();
if (isNew()) {
onDiscard();
} else {
render();
}
return;

}

if ((event.isControlKeyDown() || event.isMetaKeyDown())
&& !event.isAltKeyDown() && !event.isShiftKeyDown()) {

if ("S".codePointAt(0) == event.getNativeKeyCode()) {
event.preventDefault();
onSave(NULL_CALLBACK);
return;
}
else if ("D".codePointAt(0) == event.getNativeKeyCode()) {

event.preventDefault();
if (isNew()) {
onDiscard();
} else if (Window.confirm(PatchUtil.C.confirmDiscard())) {
onDiscard();
} else {
text.setFocus(true);
}
return;
}
}

expandTimer.schedule(250);
}
});

The only problem it has is that after you press Ctrl-S then the "j"
and "k" keys don't do anything anymore, until you click on the page
with the mouse.

Maybe you know why the focus is lost afterwards? Maybe the focus
would need to be set again?

[Shawn Pearce]

On Tue, Jul 27, 2010 at 02:22, Joel <joel.p...@gmail.com> wrote:
>
> It seems that the problem is related to the fact that you have to use
> the addKeyDownHandler instead of addKeyPressHandler

Thanks. This fix will be in 2.1.4.1, which I'm hoping to get out
early next week.

[Joel]

Thanks for the fix it works great :-)
But now I can't press enter on either the Save/Discard comment buttons
in Chrome or Firefox, would this be a side effect of the change?

On Aug 20, 10:23 am, Shawn Pearce <s...@google.com> wrote: