Comment 2 Samuel Mannehed 2021-11-17 15:57:41 CET

Note: 
As a part of the vendordrop we will remove some compatibility code for old browsers [1]. One of the removed hunks seems to be required for Safari 13 support according to a comment [2]. However, this seems to not be the case after checking MDN and caniuse.com. Even if this was still relevant all devices that support iPadOS 13 (Safari 13) also support iPadOS 14 [3].

[1] https://github.com/novnc/noVNC/commit/5b5b747494742eddf2bab63f29c1e4a25d59efc9 
[2] https://github.com/novnc/noVNC/commit/c90d53565ad731be7a6131f0141145ad4db93c0d
[3] https://en.wikipedia.org/wiki/IPadOS_14#Supported_devices

Comment 4 Samuel Mannehed 2021-11-17 16:56:53 CET

Things left to test on this bug is:

* Browser window resize on all platforms.
* Japanese and Korean keyboard layouts. See details here [1, 2]

[1] https://github.com/novnc/noVNC/commit/60c7518f8c32704615b4953bae28783786817cdc
[2] https://github.com/novnc/noVNC/pull/1498

Comment 5 Frida Flodin 2021-11-22 09:30:32 CET

I found a regression when running webaccess with the new noVNC vendordrop. When having a larger session than the browser window, scrollbars should be shown. This is not the case in the latest server build (2355). I have tested tl-4.13.0 server on the same machine and there I get the scrollbars.

Tested with Chrome Version 96.0 on Fedora 34, ThinLinc server on Ubuntu 20.04 with GNOME desktop.

Also tested on Firefox 94.0, there the scrollbars are shown as they should. So it might only be Chrome that has the issue?

Comment 6 Linn 2021-11-25 10:46:28 CET

Tested japanese keyboard fixes with server build 2355 on RHEL 8. 
Webaccess clients on Fedora 33 with Firefox 92, and Windows 10 with Firefox 94.

Setup
=====
Lenovo keyboard, layout OADG109A
Server with locale ja_JP.utf8

Changed keymap to japanese:
> [cendio@lab-237 ~]$ sudo localectl set-keymap jp-OADG109A
> [cendio@lab-237 ~]$ sudo localectl set-x11-keymap jp
> [cendio@lab-237 ~]$ localectl status
>    System Locale: LANG=ja_JP.utf8
>        VC Keymap: jp
>       X11 Layout: jp

Fixes to test
=============
 - Hankaku/Zenkaku key (Halfwidth/fullwith characters)
 - Eisu key (Alphanumeric)
 - Windows specific:
   - Hankaku/Zenkaku key and Kana/Romaji key handling
   - Fake release of Hankaku/Zenkaku key, Kana/Romaji key and Eisu key


Result
======

Unfortunately, I was unable to test the because of the issues listed below. I did compare the behaviour against ThinLinc 4.13.0 on RHEL 8, and both versions behave the same. 

Issues
------
Note: The key codes and and key symbols have been retrieved by running 'xev' in the session.

  - Hankaku/Zenkaku key is not recognised correctly in neither Linux or Windows
    - Linux: Key code 49, key symbol 0xa7 (§)
    - Windows: Key code 253, key symbol 0x60 (`)
    * Should be key symbol 0xff2a [1]
    
  - Eisu key gives the wrong key code in both Linux and Windows
    - Linux: Key code 66, key symbol 0xffe5 (Caps_Lock)
    - Windows: Key code 66, key symbol 0xffe5 (Caps_Lock)
    * Should be key symbol 0xff30 [1]
    
  - On Windows, the Kana/Romaji key isn't recognised at all - 'xev' gives no output.


Other Notes
===========
By running 'xdotool' on a local Fedora 33 machine, I was able to simulate the correct keys for Hankaku/Zenkaku and Eisu. 
> xdotool key <key symbol>
However, I wasn't able to see the key presses in 'xev' inside a ThinLinc session


[1]: https://gitlab.com/cunidev/gestures/-/wikis/xdotool-list-of-key-codes

Comment 7 Linn 2021-11-25 13:40:51 CET

Tested korean keyboard layout fix [1] with server build 2355 on RHEL 8 webaccess client on Fedora 33 with Firefox 92.

The fix is regarding the Hangul key and the Hanja key, and the only way I could trigger these keys is by running the below commands.
> # Hangul
> xdotool key 0xff34
> # Hanja
> xdotool key 0xff31
I've also tried changing to korean keyboard layout in GNOME and using my swedish keyboard as well as a japanese OADG109A keyboard without being able to trigger the correct key press.

Since 'xdotool' doesn't work inside the ThinLinc session, unfortunately no testing for this can be done. I did a comparison with ThinLinc 4.13.0 on RHEL 8, and both versions behaved the same.


[1]: https://github.com/novnc/noVNC/commit/60c7518f8c32704615b4953bae28783786817cdc

Comment 8 Frida Flodin 2021-11-26 12:41:33 CET

(In reply to Frida from comment #5)
> I found a regression when running webaccess with the new noVNC vendordrop.
> When having a larger session than the browser window, scrollbars should be
> shown. This is not the case in the latest server build (2355). I have tested
> tl-4.13.0 server on the same machine and there I get the scrollbars.
> 
> Tested with Chrome Version 96.0 on Fedora 34, ThinLinc server on Ubuntu
> 20.04 with GNOME desktop.
> 
> Also tested on Firefox 94.0, there the scrollbars are shown as they should.
> So it might only be Chrome that has the issue?

This is actually a bug in Firefox [1]. The correct behavior for resizeObserver
is to observe when scrollbars appear, since this changes the size of the content
area. I have tested this in other browsers both desktop variant and on a touch
device (where you pan instead of scrollbars). The result is that for all desktop
browsers except Firefox session can't be larger than the window. Tested the
browsers on Windows 10, macOS 12.0.1, Fedora 34, and Android 11 on Samsung tab
S7.


| Browser        | OS      | Session can be larger than window? |
|----------------+---------+------------------------------------|
| Firefox 94.0.2 | Windows | Yes                                |
| Firefox 94.0.2 | macOS   | Yes                                |
| Firefox 94.0   | Linux   | Yes                                |
| Firefox 94.1.2 | Android | Yes                                |
| Chrome 96.0    | Widnows | No                                 |
| Chrome 96.0    | macOS   | No                                 |
| Chrome 96.0    | Linux   | No                                 |
| Chrome 96.0    | Android | Yes                                |
| Edge 96.0      | Windows | No                                 |
| Edge 95.0      | Android | Yes                                |
| Safari 15.1    | macOS   | No                                 |

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1733042

Comment 9 Samuel Mannehed 2021-12-08 16:58:34 CET

Upstream issue for Comment #5:

https://github.com/novnc/noVNC/issues/1616

Comment 10 Samuel Mannehed 2021-12-13 09:05:25 CET

A fix is available upstream:

https://github.com/novnc/noVNC/pull/1617

There's been a number of changes on the master branch of noVNC since our last vendordrop however. Some of those changes would mean that we'd have to redo a large portion of testing already done.

At this point in the cycle, we have decided to cherry-pick these fixes instead of doing another vendordrop.

Comment 17 Samuel Mannehed 2021-12-13 12:24:28 CET

I've tested a Web Access from a Fedora 34 server built with these changes on..

**Fedora 34:**
- Firefox 95
- Chrome 96

**Android 12:**
- Firefox 95
- Chrome 96
- Edge 96

**Windows 10:**
- Firefox 95
- Chrome 96
- Edge 96

**iPad OS 15.1:**
- Safari 15.1

**macOS 12:**
- Firefox 95
- Chrome 96
- Edge 96
- Safari 15.1

The things I tested was:

* Making size smaller (server initiated)
* Making size larger (server initiated)
* Making size smaller by resizing the browser window
* Making size larger by resizing the browser window
* Scrollbars work when session is too big (non-touch)
* Panning works when session is too big (touch devices)
* Reconnect with a different browser size causes resize

Comment 18 Samuel Mannehed 2021-12-13 13:14:50 CET

With the resize regression now being fixed, this bug is ready for 'verify'.

What remains on this bug is mainly verification of the resize changes. These have been quite major changes, so this should be tested on all platforms.

The Korean and Japanase keyboard layout changes have already been tested in comment #6 and comment #7. The tester should review the testing-notes and only needs to test things that might have been missed in the first round of testing.

Comment 19 William Sjöblom 2021-12-15 15:34:58 CET

It seems like the issues mentioned in comment 6 stems from the fact that the
client keyboard layout was not set up correctly as I did not experience those
issues (using Chrome 96 running on Fedora 34). Despite that, we don't have any
compentence (nor any known customers) with these keyboards. So the priority for
these kinds of issues is not the highest.

I also did a quick test of the Hanja and Hangul keys (with scancodes 0xf1 and
0xf2) and both registers as expected in `xev' inside the session. These are
impossible with a regular ANSI or ISO layout keyboard since these don't have
any corresponding to the 0xf1/0xf2 scancodes. Thus, you need either a
reprogrammable keyboard or a native Korean keyboard (or other Asian keyboards
with these scancodes). The above argument about competence and customers also applies here.

What remains to be tested now is resizing behavior.

Comment 20 William Sjöblom 2021-12-16 14:52:59 CET

Tested the acceptance criteria on Fedora 34, macOS 12.0.1, and Windows
10 using build #2376 running on Fedora Server 35 (running GNOME 41).

1. Server-side resizes (using for example xrandr -s) should work when
   using Web Access
2. When the session is larger than the browser window (can be caused by
   a server-side resize), it should be possible to scroll or pan to view
   those parts of the session
3. Browser window resizes should result in a session that fits the
   browser window
4. The Web Access session should resize to fit the browser window when
   starting a new session
5. The Web Access session should resize to fit the browser window when
   reconnecting to an existing session

The first two criteria related to server-side resizing were tested by
changing the resolution in `gnome-control-center', making sure that the
requested resolution is correctly assigned and that scroll-bars appear
and are usable when in a higher than "native" resolution. The
resolutions were changed in the following order:

Native → High → Low → Native → Low → High → Native

(with "Native", "High", and "Low" denoting the browser window
dimensions, higher than browser window dimensions, and lower than
browser window dimensions respectively)

For criterion 3 the all available tiling screen edges (depending on
platform) were tested along with "regular" manual resizing and manual
resizing between various extremes. Toggling browser fullscreen were also
tested. For Fedora 34 specifically, the tiling features of the pop-shell
GNOME extension was also tested.

Criterion 4 and 5 are relatively self-explanatory but note that the
browser window was resized between disconnecting and reconnecting.

The acceptance criteria were all verified without any issues on the
following platforms and browsers:
• Fedora 34
  • Firefox 95
  • Chrome 96
• macOS 12.0.1
  • Safari 15.1
  • Firefox 95
  • Chrome 96
• Windows 10
  • Edge 96
  • Chrome 96
  • Firefox 95

Apart from testing, I also had a look at the changes to browser window
resize handling which looks very reasonable!

Comment 21 William Sjöblom 2021-12-16 16:52:42 CET

I have now also tested the mobile platforms and web browsers using the
same test cases as in comment 20 for acceptance criteria 1, 2, 4, and 5
for criterion 3 I instead performed the following:

• Switching device orientation
• (iPad OS) Resizing the web browser in side-by-side view
• (Android) Resizing the web browser in "floating window" mode

The following platforms and browsers were tested against the Fedora
Server 35 machine also used in comment 20.
• Android 11
  • Chrome 96
  • Firefox 95
• iPad OS 15.1
  • Safari
  • Chrome 96
  • Firefox 40.1


I experienced no problems or surprises during testing. So, all-in-all,
the changes to browser resize handling look great! I'll have a final
look to see if any testing areas were forgotten during the vendor drop.

Comment 22 William Sjöblom 2021-12-17 09:58:49 CET

I had a skim through the log between the 2020-06-15 vendor drop and the noVNC in the ctc tree and it seems like the testing performed in this bug is explicit in covering everything except some minor lower-level fixes and changes. For all these lower-level changes, I deem the tests presented in comment 20 and comment 21 serves to test any regressions.

My above tests include verifying the following functionality on applicable platforms:
• Mouse input
• Touch input
• Multi-touch input
• Keyboard input
• Virtual keyboard input
• Continuous screen updates

I've also had a look at the issues reported to upstream noVNC since the last vendor drop and none of them are seemingly irrelevant to us.

So, all in all, the testing performed for this bug (along that for bug 7559, bug 6059, and bug 7550) seemingly covers all changes made since the last vendor drop.

Marking as closed!