www.cendio.com
Bug 4865 - rewrite/clean up noVNC's keyboard handling
: rewrite/clean up noVNC's keyboard handling
Status: CLOSED FIXED
: ThinLinc
Web Access
: trunk
: PC Unknown
: P2 Normal
: 4.2.0
Assigned To:
:
:
:
: 3523
  Show dependency treegraph
 
Reported: 2013-10-23 16:42 by
Modified: 2014-05-13 10:36 (History)


Attachments
Test protocol of keyboards in different platform / browser combinations. (17.21 KB, text/x-markdown)
2014-05-12 14:15, Henrik Andersson
Details


Note

You need to log in before you can comment on or make changes to this bug.


Description From cendio 2013-10-23 16:42:12
There is actually some standardisation effort going on for HTML's
keyDown/keyPress/keyUp events:

http://www.w3.org/TR/DOM-Level-3-Events/#events-keyboardevents

We should make sure noVNC properly follows this and clearly separate out the
compatibility hacks for older/non-conformant browsers. This should solve
several known bugs in the current implementation.
------- Comment #1 From cendio 2013-10-23 16:57:15 -------
Also interesting for the hacky portions:

https://dvcs.w3.org/hg/dom3events/raw-file/tip/html/Note-KeyProps.html
------- Comment #2 From cendio 2013-10-24 11:42:24 -------
Might be interesting:
https://github.com/kanaka/noVNC/pull/314
------- Comment #3 From cendio 2013-11-06 14:47:53 -------
See bug 4808 for one fairly comprehensive list of keyboard bugs.
------- Comment #4 From cendio 2013-12-11 09:11:25 -------
For now, this bug is about checking out the upstream work. If it turns out that
they are not targeting the standard, then we'll have to split things up in to
multiple bugs.
------- Comment #5 From cendio 2014-03-06 17:02:37 -------
Been trying the upstream code. A first glance says that it will not fix many
issues, but it will also not introduce many new.

It seems like new browser versions have changed a few things in this as well.
For example, Firefox on Android seems to not use keyEvents anymore which leads
to the fix for bug 4602 (input event fallback implemented for Chrome on
Android) being used. A problem with backspace seems to have been introduced in
newer versions on both Chrome and Firefox on Android.

I have reported a bug for this upstream:
https://github.com/kanaka/noVNC/issues/344
------- Comment #6 From cendio 2014-04-04 19:57:54 -------
(In reply to comment #5)
> A problem with backspace seems to have been introduced in
> newer versions on both Chrome and Firefox on Android.
> 
> I have reported a bug for this upstream:
> https://github.com/kanaka/noVNC/issues/344

This bug was extended to include fixing this backspace problem on Android.
I committed this work upstream and the fix is included in the vendordrop in
commit 28807.
------- Comment #7 From cendio 2014-04-04 19:58:36 -------
(In reply to comment #5)
> Been trying the upstream code. A first glance says that it will not fix many
> issues, but it will also not introduce many new.

The upstream code fixes Alt-Gr where it was broken before.

I have tested the following platforms and browsers:

Windows 7: IE10
Windows 8: IE11, Firefox 27, Chrome 33
Fedora 18: Firefox 27, Firefox 28, Chrome 33, Chrome 34
Mac OS X 10.9: Safari 7
Android 4.4: Firefox 27, Firefox 28, Chrome 33, Chrome 34
iOS 7: Safari 7, Chrome 32

The only regression that has been spotted so far with the upstream code is that
keyboard input doesn't work properly on IE10. A fix seems to be in the works
for this: https://github.com/kanaka/noVNC/issues/352
I tested the code mentioned in that issue and it does indeed seem to work.

Overall the keyboard code works well in all browsers on all client platforms
that I have tested with a few exceptions, these are not regressions:

- Dead keys doesn't work in Firefox on Linux. Upstream-bug:
https://github.com/kanaka/noVNC/issues/350
- When pressing the Enter/Go-key in Chrome on Android, the on screen keyboard
is closed. Upstream-bug: https://github.com/kanaka/noVNC/issues/356
------- Comment #8 From cendio 2014-04-04 22:49:25 -------
I reopen this and wait for the IE10 fix to be properly committed upstream. Will
do a new vendor drop after that.
------- Comment #9 From cendio 2014-04-07 10:17:57 -------
Commit 28811 - vnc.tmpl updated to reflect changes in vnc.html from the
vendordrop.
------- Comment #10 From cendio 2014-04-08 17:10:05 -------
(In reply to comment #7)
> - When pressing the Enter/Go-key in Chrome on Android, the on screen keyboard
> is closed. Upstream-bug: https://github.com/kanaka/noVNC/issues/356

It is more important to have a properly functioning Enter key on Chrome than to
not have text suggestions. The vendordrop in commit 28823 fixes this.

The resulting problem with text suggestions is illustrated well by the
screenshots in the upstream bug: https://github.com/kanaka/noVNC/issues/357
------- Comment #11 From cendio 2014-04-09 15:40:14 -------
*** Bug 5079 has been marked as a duplicate of this bug. ***
------- Comment #12 From cendio 2014-04-09 15:54:53 -------
(In reply to comment #11)
> *** Bug 5079 has been marked as a duplicate of this bug. ***

The problem found here was due to a mistake in the vendordrop in r28807. This
problem was fixed in r28826.

Another merge miss was fixed in r28825.
------- Comment #13 From cendio 2014-04-14 16:32:12 -------
(In reply to comment #8)
> I reopen this and wait for the IE10 fix to be properly committed upstream. Will
> do a new vendor drop after that.

Fixed in commit 28846.

Two additional fixes were also committed:

r28841 - keep vnc.tmpl in sync with vnc.html
r28840 - add missing import

Marking this as resolved now.
------- Comment #14 From cendio 2014-05-12 14:15:36 -------
Created an attachment (id=542) [details]
Test protocol of keyboards in different platform / browser combinations.

Attached file includes test cases and protocol of issue for keyboard handling.
Pierre has verified that the results is not worse than before.
------- Comment #15 From cendio 2014-05-12 14:40:01 -------
Opened a new bug #5135 with up-to-date keyboard issues, and closing this now
when it shows better results then before.