Bug 5352 - We should stop modifying the canvas size in the HTML5 client
Summary: We should stop modifying the canvas size in the HTML5 client
Status: CLOSED FIXED
Alias: None
Product: ThinLinc
Classification: Unclassified
Component: Web Access (show other bugs)
Version: trunk
Hardware: PC Unknown
: P2 Normal
Target Milestone: 4.4.0
Assignee: Samuel Mannehed
URL:
Keywords: ossman_tester, prosaic
Depends on:
Blocks: 4846 5332
  Show dependency treegraph
 
Reported: 2014-11-13 14:24 CET by Samuel Mannehed
Modified: 2015-04-30 13:22 CEST (History)
1 user (show)

See Also:
Acceptance Criteria:


Attachments

Description Samuel Mannehed cendio 2014-11-13 14:24:26 CET
This is how a resize currently works in our HTML5 client:

1. The browser's window.onresize is invoked
2. We wait 0.5s and then send a resize request to the server
3. The server resizes the remote session
4. The server sends a response to the client that the request was successful
5. The client resizes the viewport to the new size
6. The client resizes the canvas to the new size

This has the effect that in, for example, Firefox 33 on Android you will get browser-side panning. This is a problem since we have no way of controlling this, thus the "panning swipes" will also be sent to the server as mouse events.

A better solution would be to always have the canvas covering 100% of the page and only resize the viewport. This way we can hopefully avoid problems wrt canvas size since the browser itself will handle it.
Comment 1 Samuel Mannehed cendio 2015-01-09 12:17:40 CET
We can't have the style attributes width and height being 100% for canvas since that will result in incorrect scaling. The canvas element has html attributes called width and height which are the correct ones to use. The canvas size will have to be resized in the javascript code.

See:
http://stackoverflow.com/questions/4938346/canvas-width-and-height-in-html5
Comment 2 Samuel Mannehed cendio 2015-01-26 10:32:33 CET
However, we should still be able to have the canvas covering the whole page. As I mentioned, the canvas has two different widths and heights; the layout size and the resolution. If you only modify the layout size the content will scale, so we will have to modify both. 

The display code will have to be modified to not having the canvas following the size of the viewport. The canvas will instead follow the page.
Comment 3 Samuel Mannehed cendio 2015-02-11 16:24:54 CET
(In reply to comment #2)
> However, we should still be able to have the canvas covering the whole page. As
> I mentioned, the canvas has two different widths and heights; the layout size
> and the resolution. If you only modify the layout size the content will scale,
> so we will have to modify both. 
> 
> The display code will have to be modified to not having the canvas following
> the size of the viewport. The canvas will instead follow the page.

I need to further clarify this:

When "clipping"/panning, i.e. when using a touch device, the canvas will always be the size of the browser window. If the session is smaller, the session will only be drawn on a part of the canvas (the viewport).

On non-touch devices we want scrollbars when the session is larger than the browser-window. This means that both the canvas and the viewport can be larger than the browser window.
Comment 4 Samuel Mannehed cendio 2015-02-17 14:58:21 CET
So, when the browser window changes we will always change the canvas layout size and the canvas resolution should always follow the layout size.
Comment 5 Samuel Mannehed cendio 2015-03-02 11:47:53 CET
I consider my work finished, I have submitted a pullrequest upstream with my changes. I will await comments/approval before I do a vendordrop and bring it to ThinLinc.

https://github.com/kanaka/noVNC/pull/464
Comment 6 Samuel Mannehed cendio 2015-03-06 12:37:37 CET
I adapted the PR to changes made to upstream master branch.
Comment 7 Samuel Mannehed cendio 2015-03-11 08:11:19 CET
Brought to ThinLinc by vendordrop in commit 30125.
Comment 8 Samuel Mannehed cendio 2015-03-11 08:15:11 CET
Tester should verify that automatic resize, serverside resize, panning and scrollbars work as intended.
Comment 9 Samuel Mannehed cendio 2015-03-16 17:26:14 CET
Had forgotten to commit the changes to vnc.tmpl, done in r30144.
Comment 10 Samuel Mannehed cendio 2015-03-26 18:46:44 CET
(In reply to comment #8)
> Tester should verify that automatic resize, serverside resize, panning and
> scrollbars work as intended.

Just because I'm nice, here is a matrix for the tests needed:

                  | touch | desktop |
                --+-------+---------+
xrandr incr. size |       |         |
                --+-------+---------+
xrandr decr. size |       |         |
                --+-------+---------+
  auto incr. size |       |         |
                --+-------+---------+
  auto decr. size |       |         |
                --+-------+---------+
 centered session |       |         |
                --+-------+---------+
          panning |       |         |
                --+-------+---------+

If the session is smaller than the window, it should be centered horizontally. On touch, panning is done with the "hand", on desktop you should get scrollbars.
Comment 11 Samuel Mannehed cendio 2015-04-13 15:57:49 CEST
The an issue which was brought to ThinLinc in the vendordrop here (r30125) cause the proper status messages not being shown if the session is disconnected.
Comment 12 Samuel Mannehed cendio 2015-04-13 16:37:30 CEST
The issue was fixed upstream: https://github.com/kanaka/noVNC/pull/474

Brought to ThinLinc in vendordrop - r30230
Update vnc.tmpl to match vnc.html - r30231
Removed a confusing status string - r30227
Comment 13 Pierre Ossman cendio 2015-04-30 13:22:59 CEST
Tested with Firefox 36 on Fedora 21, and Chrome 42 on Android 5:

                  | touch | desktop |
                --+-------+---------+
xrandr incr. size |   ✔   |    ✔    |
                --+-------+---------+
xrandr decr. size |   ✔   |    ✔    |
                --+-------+---------+
  auto incr. size |   ✔   |    ✔    |
                --+-------+---------+
  auto decr. size |   ✔   |    ✔    |
                --+-------+---------+
 centered session |   ✔   |    ✔    |
                --+-------+---------+
          panning |   ✔   |    ✔    |
                --+-------+---------+

Having it centered just horizontally and not vertically is a bit odd though. As is having a white filler for the unused areas.

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