[pve-devel] [PATCH novnc] add patches to workaround fullscreen issue in android app

[pve-devel] [PATCH novnc] add patches to workaround fullscreen issue in android app

[Dominik Csapak]

patch 15 hides the fullscreen button,
patch 16 make the error message dismissable

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 ...-button-on-isFullscreen-get-variable.patch | 39 +++++++++++++++
 debian/patches/0017-make-error-hideable.patch | 47 +++++++++++++++++++
 debian/patches/series                         |  2 +
 3 files changed, 88 insertions(+)
 create mode 100644 debian/patches/0016-hide-fullscreen-button-on-isFullscreen-get-variable.patch
 create mode 100644 debian/patches/0017-make-error-hideable.patch

diff --git a/debian/patches/0016-hide-fullscreen-button-on-isFullscreen-get-variable.patch b/debian/patches/0016-hide-fullscreen-button-on-isFullscreen-get-variable.patch
new file mode 100644
index 0000000..1cdafd2
--- /dev/null
+++ b/debian/patches/0016-hide-fullscreen-button-on-isFullscreen-get-variable.patch
@@ -0,0 +1,39 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Dominik Csapak <d.csapak@proxmox.com>
+Date: Mon, 20 Dec 2021 09:28:35 +0100
+Subject: [PATCH] hide fullscreen button on 'isFullscreen' get variable
+
+workaround for android app, since the webview does not allow
+'requestFullscreen' apparently, so we make the webview fullscreen and
+hide the button here.
+
+Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
+---
+ app/pve.js | 7 +++++++
+ 1 file changed, 7 insertions(+)
+
+diff --git a/app/pve.js b/app/pve.js
+index 486bd5c..042eb7c 100644
+--- a/app/pve.js
++++ b/app/pve.js
+@@ -12,6 +12,7 @@ export default function PVEUI(UI){
+     this.nodename = WebUtil.getQueryVar('node');
+     this.resize = WebUtil.getQueryVar('resize');
+     this.cmd = WebUtil.getQueryVar('cmd');
++    this.fullscreen = WebUtil.getQueryVar('isFullscreen');
+     this.lastFBWidth = undefined;
+     this.lastFBHeight = undefined;
+     this.sizeUpdateTimer = undefined;
+@@ -304,6 +305,12 @@ PVEUI.prototype = {
+ 		.classList.add('pve_hidden');
+ 	}
+ 
++	// hide fullscren button when it's already fullscreen (e.g. android app)
++	if (me.fullscreen) {
++	    document.getElementById('noVNC_fullscreen_button')
++		.classList.add('pve_hidden');
++	}
++
+ 	// add command logic
+ 	var commandArray = [
+ 	    { cmd: 'start', kvm: 1, lxc: 1},
diff --git a/debian/patches/0017-make-error-hideable.patch b/debian/patches/0017-make-error-hideable.patch
new file mode 100644
index 0000000..9322ade
--- /dev/null
+++ b/debian/patches/0017-make-error-hideable.patch
@@ -0,0 +1,47 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Dominik Csapak <d.csapak@proxmox.com>
+Date: Mon, 20 Dec 2021 09:34:36 +0100
+Subject: [PATCH] make error hideable
+
+by clicking on it
+
+Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
+---
+ app/error-handler.js | 17 +++++++++++++++--
+ 1 file changed, 15 insertions(+), 2 deletions(-)
+
+diff --git a/app/error-handler.js b/app/error-handler.js
+index 81a6cba..6ce8013 100644
+--- a/app/error-handler.js
++++ b/app/error-handler.js
+@@ -15,6 +15,18 @@
+ 
+ (function _scope() {
+     "use strict";
++    function hideError() {
++	const msg = document.getElementById('noVNC_fallback_errormsg');
++
++	// close it
++	document.getElementById('noVNC_fallback_error')
++	    .classList.remove("noVNC_open");
++
++	// remove all children
++	while (msg.firstChild) {
++	    msg.removeChild(msg.firstChild);
++	}
++    }
+ 
+     // Fallback for all uncought errors
+     function handleError(event, err) {
+@@ -52,8 +64,9 @@
+                 msg.appendChild(div);
+             }
+ 
+-            document.getElementById('noVNC_fallback_error')
+-                .classList.add("noVNC_open");
++            const node = document.getElementById('noVNC_fallback_error');
++	    node.classList.add("noVNC_open");
++	    node.onclick = hideError;
+         } catch (exc) {
+             document.write("noVNC encountered an error.");
+         }
diff --git a/debian/patches/series b/debian/patches/series
index 4913924..a520e27 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -13,3 +13,5 @@
 0013-Revert-Remove-the-default-value-of-wsProtocols.patch
 0014-avoid-passing-deprecated-upgrade-parameter.patch
 0015-create-own-class-for-hidden-buttons.patch
+0016-hide-fullscreen-button-on-isFullscreen-get-variable.patch
+0017-make-error-hideable.patch
-- 
2.30.2

Re: [pve-devel] [PATCH novnc] add patches to workaround fullscreen issue in android app

[Thomas Lamprecht]

On 20/12/2021 12:33, Dominik Csapak wrote:
> patch 15 hides the fullscreen button,
> patch 16 make the error message dismissable
> 

this seems broken?

The d/patches/series file gets 16 and 17 added:

0016-hide-fullscreen-button-on-isFullscreen-get-variable.patch
0017-make-error-hideable.patch

So above is at least confusing, but actually this diff then only includes the
0017 patch, which then makes it completely wrong?!

> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  ...-button-on-isFullscreen-get-variable.patch | 39 +++++++++++++++
>  debian/patches/0017-make-error-hideable.patch | 47 +++++++++++++++++++
>  debian/patches/series                         |  2 +
>  3 files changed, 88 insertions(+)
>  create mode 100644 debian/patches/0016-hide-fullscreen-button-on-isFullscreen-get-variable.patch
>  create mode 100644 debian/patches/0017-make-error-hideable.patch
> 
> diff --git a/debian/patches/0016-hide-fullscreen-button-on-isFullscreen-get-variable.patch b/debian/patches/0016-hide-fullscreen-button-on-isFullscreen-get-variable.patch
> new file mode 100644
> index 0000000..1cdafd2
> --- /dev/null
> +++ b/debian/patches/0016-hide-fullscreen-button-on-isFullscreen-get-variable.patch
> @@ -0,0 +1,39 @@
> +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
> +From: Dominik Csapak <d.csapak@proxmox.com>
> +Date: Mon, 20 Dec 2021 09:28:35 +0100
> +Subject: [PATCH] hide fullscreen button on 'isFullscreen' get variable
> +
> +workaround for android app, since the webview does not allow
> +'requestFullscreen' apparently, so we make the webview fullscreen and
> +hide the button here.
> +
> +Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> +---
> + app/pve.js | 7 +++++++
> + 1 file changed, 7 insertions(+)
> +
> +diff --git a/app/pve.js b/app/pve.js
> +index 486bd5c..042eb7c 100644
> +--- a/app/pve.js
> ++++ b/app/pve.js
> +@@ -12,6 +12,7 @@ export default function PVEUI(UI){
> +     this.nodename = WebUtil.getQueryVar('node');
> +     this.resize = WebUtil.getQueryVar('resize');
> +     this.cmd = WebUtil.getQueryVar('cmd');
> ++    this.fullscreen = WebUtil.getQueryVar('isFullscreen');
> +     this.lastFBWidth = undefined;
> +     this.lastFBHeight = undefined;
> +     this.sizeUpdateTimer = undefined;
> +@@ -304,6 +305,12 @@ PVEUI.prototype = {
> + 		.classList.add('pve_hidden');
> + 	}
> + 
> ++	// hide fullscren button when it's already fullscreen (e.g. android app)
> ++	if (me.fullscreen) {
> ++	    document.getElementById('noVNC_fullscreen_button')
> ++		.classList.add('pve_hidden');
> ++	}
> ++
> + 	// add command logic
> + 	var commandArray = [
> + 	    { cmd: 'start', kvm: 1, lxc: 1},
> diff --git a/debian/patches/0017-make-error-hideable.patch b/debian/patches/0017-make-error-hideable.patch
> new file mode 100644
> index 0000000..9322ade
> --- /dev/null
> +++ b/debian/patches/0017-make-error-hideable.patch
> @@ -0,0 +1,47 @@
> +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
> +From: Dominik Csapak <d.csapak@proxmox.com>
> +Date: Mon, 20 Dec 2021 09:34:36 +0100
> +Subject: [PATCH] make error hideable
> +
> +by clicking on it
> +
> +Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> +---
> + app/error-handler.js | 17 +++++++++++++++--
> + 1 file changed, 15 insertions(+), 2 deletions(-)
> +
> +diff --git a/app/error-handler.js b/app/error-handler.js
> +index 81a6cba..6ce8013 100644
> +--- a/app/error-handler.js
> ++++ b/app/error-handler.js
> +@@ -15,6 +15,18 @@
> + 
> + (function _scope() {
> +     "use strict";
> ++    function hideError() {
> ++	const msg = document.getElementById('noVNC_fallback_errormsg');
> ++
> ++	// close it
> ++	document.getElementById('noVNC_fallback_error')
> ++	    .classList.remove("noVNC_open");
> ++
> ++	// remove all children
> ++	while (msg.firstChild) {
> ++	    msg.removeChild(msg.firstChild);
> ++	}
> ++    }
> + 
> +     // Fallback for all uncought errors
> +     function handleError(event, err) {
> +@@ -52,8 +64,9 @@
> +                 msg.appendChild(div);
> +             }
> + 
> +-            document.getElementById('noVNC_fallback_error')
> +-                .classList.add("noVNC_open");
> ++            const node = document.getElementById('noVNC_fallback_error');
> ++	    node.classList.add("noVNC_open");
> ++	    node.onclick = hideError;
> +         } catch (exc) {
> +             document.write("noVNC encountered an error.");
> +         }
> diff --git a/debian/patches/series b/debian/patches/series
> index 4913924..a520e27 100644
> --- a/debian/patches/series
> +++ b/debian/patches/series
> @@ -13,3 +13,5 @@
>  0013-Revert-Remove-the-default-value-of-wsProtocols.patch
>  0014-avoid-passing-deprecated-upgrade-parameter.patch
>  0015-create-own-class-for-hidden-buttons.patch
> +0016-hide-fullscreen-button-on-isFullscreen-get-variable.patch
> +0017-make-error-hideable.patch

Re: [pve-devel] [PATCH novnc] add patches to workaround fullscreen issue in android app

[Dominik Csapak]

On 12/20/21 20:53, Thomas Lamprecht wrote:
> On 20/12/2021 12:33, Dominik Csapak wrote:
>> patch 15 hides the fullscreen button,
>> patch 16 make the error message dismissable
>>
> 
> this seems broken?
> 
> The d/patches/series file gets 16 and 17 added:
> 
> 0016-hide-fullscreen-button-on-isFullscreen-get-variable.patch
> 0017-make-error-hideable.patch
> 
> So above is at least confusing, but actually this diff then only includes the
> 0017 patch, which then makes it completely wrong?!

what do you mean?
admittedly i forgot to mention that this only applies on top of the
v1.3.0 upgrade, but the patches are ...

> 
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>>   ...-button-on-isFullscreen-get-variable.patch | 39 +++++++++++++++
>>   debian/patches/0017-make-error-hideable.patch | 47 +++++++++++++++++++
>>   debian/patches/series                         |  2 +
>>   3 files changed, 88 insertions(+)
>>   create mode 100644 debian/patches/0016-hide-fullscreen-button-on-isFullscreen-get-variable.patch
>>   create mode 100644 debian/patches/0017-make-error-hideable.patch

both here? namely..

>>
>> diff --git a/debian/patches/0016-hide-fullscreen-button-on-isFullscreen-get-variable.patch b/debian/patches/0016-hide-fullscreen-button-on-isFullscreen-get-variable.patch
>> new file mode 100644
>> index 0000000..1cdafd2
>> --- /dev/null
>> +++ b/debian/patches/0016-hide-fullscreen-button-on-isFullscreen-get-variable.patch

here and...

>> @@ -0,0 +1,39 @@
>> +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
>> +From: Dominik Csapak <d.csapak@proxmox.com>
>> +Date: Mon, 20 Dec 2021 09:28:35 +0100
>> +Subject: [PATCH] hide fullscreen button on 'isFullscreen' get variable
>> +
>> +workaround for android app, since the webview does not allow
>> +'requestFullscreen' apparently, so we make the webview fullscreen and
>> +hide the button here.
>> +
>> +Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> +---
>> + app/pve.js | 7 +++++++
>> + 1 file changed, 7 insertions(+)
>> +
>> +diff --git a/app/pve.js b/app/pve.js
>> +index 486bd5c..042eb7c 100644
>> +--- a/app/pve.js
>> ++++ b/app/pve.js
>> +@@ -12,6 +12,7 @@ export default function PVEUI(UI){
>> +     this.nodename = WebUtil.getQueryVar('node');
>> +     this.resize = WebUtil.getQueryVar('resize');
>> +     this.cmd = WebUtil.getQueryVar('cmd');
>> ++    this.fullscreen = WebUtil.getQueryVar('isFullscreen');
>> +     this.lastFBWidth = undefined;
>> +     this.lastFBHeight = undefined;
>> +     this.sizeUpdateTimer = undefined;
>> +@@ -304,6 +305,12 @@ PVEUI.prototype = {
>> + 		.classList.add('pve_hidden');
>> + 	}
>> +
>> ++	// hide fullscren button when it's already fullscreen (e.g. android app)
>> ++	if (me.fullscreen) {
>> ++	    document.getElementById('noVNC_fullscreen_button')
>> ++		.classList.add('pve_hidden');
>> ++	}
>> ++
>> + 	// add command logic
>> + 	var commandArray = [
>> + 	    { cmd: 'start', kvm: 1, lxc: 1},
>> diff --git a/debian/patches/0017-make-error-hideable.patch b/debian/patches/0017-make-error-hideable.patch
>> new file mode 100644
>> index 0000000..9322ade
>> --- /dev/null
>> +++ b/debian/patches/0017-make-error-hideable.patch

here?

>> @@ -0,0 +1,47 @@
>> +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
>> +From: Dominik Csapak <d.csapak@proxmox.com>
>> +Date: Mon, 20 Dec 2021 09:34:36 +0100
>> +Subject: [PATCH] make error hideable
>> +
>> +by clicking on it
>> +
>> +Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> +---
>> + app/error-handler.js | 17 +++++++++++++++--
>> + 1 file changed, 15 insertions(+), 2 deletions(-)
>> +
>> +diff --git a/app/error-handler.js b/app/error-handler.js
>> +index 81a6cba..6ce8013 100644
>> +--- a/app/error-handler.js
>> ++++ b/app/error-handler.js
>> +@@ -15,6 +15,18 @@
>> +
>> + (function _scope() {
>> +     "use strict";
>> ++    function hideError() {
>> ++	const msg = document.getElementById('noVNC_fallback_errormsg');
>> ++
>> ++	// close it
>> ++	document.getElementById('noVNC_fallback_error')
>> ++	    .classList.remove("noVNC_open");
>> ++
>> ++	// remove all children
>> ++	while (msg.firstChild) {
>> ++	    msg.removeChild(msg.firstChild);
>> ++	}
>> ++    }
>> +
>> +     // Fallback for all uncought errors
>> +     function handleError(event, err) {
>> +@@ -52,8 +64,9 @@
>> +                 msg.appendChild(div);
>> +             }
>> +
>> +-            document.getElementById('noVNC_fallback_error')
>> +-                .classList.add("noVNC_open");
>> ++            const node = document.getElementById('noVNC_fallback_error');
>> ++	    node.classList.add("noVNC_open");
>> ++	    node.onclick = hideError;
>> +         } catch (exc) {
>> +             document.write("noVNC encountered an error.");
>> +         }
>> diff --git a/debian/patches/series b/debian/patches/series
>> index 4913924..a520e27 100644
>> --- a/debian/patches/series
>> +++ b/debian/patches/series
>> @@ -13,3 +13,5 @@
>>   0013-Revert-Remove-the-default-value-of-wsProtocols.patch
>>   0014-avoid-passing-deprecated-upgrade-parameter.patch
>>   0015-create-own-class-for-hidden-buttons.patch
>> +0016-hide-fullscreen-button-on-isFullscreen-get-variable.patch
>> +0017-make-error-hideable.patch
> 

Re: [pve-devel] [PATCH novnc] add patches to workaround fullscreen issue in android app

[Thomas Lamprecht]

On 21/12/2021 08:05, Dominik Csapak wrote:
> On 12/20/21 20:53, Thomas Lamprecht wrote:
>> On 20/12/2021 12:33, Dominik Csapak wrote:
>>> patch 15 hides the fullscreen button,
>>> patch 16 make the error message dismissable
>>>
>>
>> this seems broken?
>>
>> The d/patches/series file gets 16 and 17 added:
>>
>> 0016-hide-fullscreen-button-on-isFullscreen-get-variable.patch
>> 0017-make-error-hideable.patch
>>
>> So above is at least confusing, but actually this diff then only includes the
>> 0017 patch, which then makes it completely wrong?!
> 
> what do you mean?
> admittedly i forgot to mention that this only applies on top of the
> v1.3.0 upgrade, but the patches are ...

ah, I got confused by the truncated name in the short log and you wrongly referring
to patch 15 and patch 16 in the commit message..

[pve-devel] applied: Re: [PATCH novnc] add patches to workaround fullscreen issue in android app

[Thomas Lamprecht]

On 20/12/2021 12:33, Dominik Csapak wrote:
> patch 15 hides the fullscreen button,
> patch 16 make the error message dismissable
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  ...-button-on-isFullscreen-get-variable.patch | 39 +++++++++++++++
>  debian/patches/0017-make-error-hideable.patch | 47 +++++++++++++++++++
>  debian/patches/series                         |  2 +
>  3 files changed, 88 insertions(+)
>  create mode 100644 debian/patches/0016-hide-fullscreen-button-on-isFullscreen-get-variable.patch
>  create mode 100644 debian/patches/0017-make-error-hideable.patch
> 
>

now with the 1.3 rebase done first  this applied cleanly, thanks!
FYI: I adapted the commit message so that the patch references are correct there.

IMO the "make error dismissable" should be upstreamed in some form, not sure if
clicking on it is the best general solution, as that can make copying an error
text hard, but either a X (text-)button or the like would surely be great for
them too.