You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2020/02/01 19:48:22 UTC
[GitHub] [guacamole-client] madmath03 opened a new pull request #470:
GUACAMOLE-124: Add full-screen action
madmath03 opened a new pull request #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470
Signed-off-by: mathieu.brunot <ma...@monogramm.io>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] mike-jumper commented on a change in pull
request #470: GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#discussion_r383100250
##########
File path: guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js
##########
@@ -134,6 +134,48 @@ angular.module('navigation').directive('guacUserMenu', [function guacUserMenu()
return authenticationService.isAnonymous();
};
+ /**
+ * Requests fullscreen for the Guacamole page.
+ */
+ $scope.fullscreen = function fullscreen() {
+ var elem = document.documentElement;
+ var fullscreenElem = !(!document.fullscreenElement &&
+ !document.msFullscreenElement &&
+ !document.mozFullScreenElement &&
+ !document.webkitFullscreenElement);
+
+ if (!fullscreenElem) {
+ if (!elem.requestFullscreen) {
+ elem.requestFullscreen = (elem.mozRequestFullScreen
+ || elem.webkitRequestFullscreen
+ || elem.msRequestFullscreen).bind(elem);
+ }
+
+ if (elem.requestFullscreen) {
+ elem.requestFullscreen();
+ }
Review comment:
This appears to anticipate that there may not be any `requestFullscreen()` implementation, even browser-specific prefix versions. That's good, but such a case would also cause the previous call to `bind()` to fail, as `elem.mozRequestFullScreen || elem.webkitRequestFullscreen || elem.msRequestFullscreen` would not evaluate to something that has `bind()`.
I'd recommend avoiding both of these issues entirely, adding an anonymous stub function at the end of that chain of `||`, thus ensuring there is always a version of `requestFullscreen()` (even if it does nothing).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] madmath03 commented on issue #470:
GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
madmath03 commented on issue #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#issuecomment-582584855
> Overall this looks pretty solid to me. My only question is how much testing you've done with scaling/resizing with this? Presumably when it goes to full-screen the screen resolution changes, which, depending on the protocol in use for the connection, will result in either an on-the-fly resize (RDP, maybe SSH/Telnet), a disconnect/reconnect (RDP), or some extra black space around the screen area. What sort of behavior have you seen?
I did some more tests on different devices / browsers and connections. Here are the results:
| Device / Browser | :checkered_flag: Windows RDP | :penguin: Linux SSH | :penguin: Linux VNC (TigerVNC) |
| ------------------- | --------------- | ----------- | ------------------------ |
| :checkered_flag: Windows / Chromium | :warning: | :white_check_mark: | :white_check_mark: |
| :checkered_flag: Windows / Firefox | :warning: | :white_check_mark: | :white_check_mark: |
| :checkered_flag: Windows / Edge | :warning: | :white_check_mark: | :white_check_mark: |
| :checkered_flag: Windows / IE 11 | :no_entry_sign: | :no_entry_sign: | :no_entry_sign: |
| :robot: Android Tablet / Chromium | :warning: | :white_check_mark: | :white_check_mark: |
| :robot: Android Tablet / Firefox | :warning: | :white_check_mark: | :white_check_mark: |
| :green_apple: iPhone / Safari | :x: | :x: | :x: |
- :white_check_mark: : Fullscreen works properly and connection is immediately resized
- :warning: : Fullscreen button works but Windows RDP connection displays empty space and needs to be restarted to be resized (disconnect / reconnect, no need to go back to home)
- :no_entry_sign: : Fullscreen button works but only displays a black screen in connection, whatever the connection type
- :x: : Fullscreen button does not work
So basically:
* IE 11 and Safari do not work
* RDP connection needs reconnect (seems to be the normal behavior of RDP)
* everything else is good
@necouchman & @mike-jumper : I do not have a Mac to debug behavior on Safari, and I do not believe IE 11 is relevant anymore, so I think that's good enough to be used, or at least to be reviewed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] mike-jumper commented on a change in pull
request #470: GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#discussion_r383626894
##########
File path: guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js
##########
@@ -134,6 +134,48 @@ angular.module('navigation').directive('guacUserMenu', [function guacUserMenu()
return authenticationService.isAnonymous();
};
+ /**
+ * Requests fullscreen for the Guacamole page.
+ */
+ $scope.fullscreen = function fullscreen() {
+ var elem = document.documentElement;
+ var fullscreenElem = !(!document.fullscreenElement &&
+ !document.msFullscreenElement &&
+ !document.mozFullScreenElement &&
+ !document.webkitFullscreenElement);
+
+ if (!fullscreenElem) {
+ if (!elem.requestFullscreen) {
+ elem.requestFullscreen = (elem.mozRequestFullScreen
+ || elem.webkitRequestFullscreen
+ || elem.msRequestFullscreen).bind(elem);
+ }
+
+ if (elem.requestFullscreen) {
+ elem.requestFullscreen();
+ }
Review comment:
Assuming this is still relevant given what you mentioned regarding hiding the fullscreen option if it can't be used, I have no objection to logging a warning.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] madmath03 edited a comment on issue #470:
GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
madmath03 edited a comment on issue #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#issuecomment-582584855
> Overall this looks pretty solid to me. My only question is how much testing you've done with scaling/resizing with this? Presumably when it goes to full-screen the screen resolution changes, which, depending on the protocol in use for the connection, will result in either an on-the-fly resize (RDP, maybe SSH/Telnet), a disconnect/reconnect (RDP), or some extra black space around the screen area. What sort of behavior have you seen?
I did some more tests on different devices / browsers and connections. Here are the results:
| Device / Browser | :checkered_flag: Windows RDP | :penguin: Linux SSH | :penguin: Linux VNC (TigerVNC) |
| ------------------- | --------------- | ----------- | ------------------------ |
| :checkered_flag: Windows / Chromium | :warning: | :white_check_mark: | :white_check_mark: |
| :checkered_flag: Windows / Firefox | :warning: | :white_check_mark: | :white_check_mark: |
| :checkered_flag: Windows / Edge | :warning: | :white_check_mark: | :white_check_mark: |
| :checkered_flag: Windows / IE 11 | :no_entry_sign: | :no_entry_sign: | :no_entry_sign: |
| :robot: Android Tablet / Chromium | :warning: | :white_check_mark: | :white_check_mark: |
| :robot: Android Tablet / Firefox | :warning: | :white_check_mark: | :white_check_mark: |
| :green_apple: iPhone / Safari | :x: | :x: | :x: |
- :white_check_mark: : Fullscreen works properly and connection is immediately resized
- :warning: : Fullscreen button works but Windows RDP connection displays empty space and needs to be restarted to be resized (disconnect / reconnect, no need to go back to home)
- :no_entry_sign: : Fullscreen button works but only displays a black screen in connection, whatever the connection type
- :x: : Fullscreen button does not trigger fullscreen
So basically:
* IE 11 and Safari do not work
* RDP connection needs reconnect (seems to be the normal behavior of RDP)
* everything else is good
@necouchman & @mike-jumper : I do not have a Mac to debug behavior on Safari, and I do not believe IE 11 is relevant anymore, so I think that's good enough to be used, or at least to be reviewed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] mike-jumper commented on a change in pull
request #470: GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#discussion_r385866761
##########
File path: guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js
##########
@@ -134,6 +135,64 @@ angular.module('navigation').directive('guacUserMenu', [function guacUserMenu()
return authenticationService.isAnonymous();
};
+ /**
+ * All available actions for the current user.
+ */
+ $scope.actions = [ ];
+
+ // Initialize fullscreen functions
+ var docElem = document.documentElement;
+ var requestFullscreen = docElem.mozRequestFullScreen
+ || docElem.webkitRequestFullscreen
+ || docElem.msRequestFullscreen;
+ var exitFullscreen = document.mozCancelFullScreen
+ || document.webkitExitFullscreen
+ || document.msExitFullscreen;
+ if (!!requestFullscreen && !!exitFullscreen) {
+ // Bind browser-specific fullscreen functions
+ if (!docElem.requestFullscreen) {
+ docElem.requestFullscreen = (requestFullscreen).bind(docElem);
+ }
+ if (!document.exitFullscreen) {
+ document.exitFullscreen = (exitFullscreen).bind(document);
+ }
+
+ /**
+ * Toggles fullscreen for the Guacamole page.
+ */
+ $scope.fullscreen = function fullscreen() {
+ var fullscreenElem = !!(document.fullscreenElement
+ || document.msFullscreenElement
+ || document.mozFullScreenElement
+ || document.webkitFullscreenElement);
+
+ if (!fullscreenElem) {
+ document.documentElement.requestFullscreen();
+ }
+ else {
+ document.exitFullscreen();
+ }
+ };
+
+ /**
+ * Action which requests fullscreen for the Guacamole page.
+ */
+ var FULLSCREEN_ACTION = {
+ name : 'USER_MENU.ACTION_FULLSCREEN',
+ className : 'fullscreen',
+ callback : $scope.fullscreen
+ };
Review comment:
While I agree there is no need for these to be declared except when fullscreen is supported, this `if` block and the controller as a whole might be more readable were `FULLSCREEN_ACTION` and `$scope.fullscreen()` declared elsewhere, ideally grouped logically with `LOGOUT_ACTION` and its callback.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] echu2013 commented on issue #470: GUACAMOLE-124:
Add full-screen action
Posted by GitBox <gi...@apache.org>.
echu2013 commented on issue #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#issuecomment-609915963
@madmath03 : I've tested it on `Version 80.0.3987.149 (Official Build) (64-bit)` under `Windows 10 Pro 64-bit` and it's working OK!
IMO there should be some sort of toggle switch or visual change when full-screen toggled on, like changing "Full Screen" value to "Turn off Full Screen".
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] madmath03 commented on issue #470:
GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
madmath03 commented on issue #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#issuecomment-584542565
Hi @mike-jumper ,
Do you think this could be part of the 1.2.0 release ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] mike-jumper commented on a change in pull
request #470: GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#discussion_r385865503
##########
File path: guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js
##########
@@ -160,9 +219,9 @@ angular.module('navigation').directive('guacUserMenu', [function guacUserMenu()
};
/**
- * All available actions for the current user.
+ * Add logout action for the current user.
*/
- $scope.actions = [ LOGOUT_ACTION ];
+ $scope.actions = $scope.actions.concat( LOGOUT_ACTION );
Review comment:
Rather than repeatedly recreating the `$scope.actions` array through calling `concat()`, perhaps it would be better to keep the original initialization to `[ LOGOUT_ACTION ]` and simply `unshift(FULLSCREEN_ACTION)` when needed?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] mike-jumper commented on a change in pull
request #470: GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#discussion_r383099683
##########
File path: guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js
##########
@@ -134,6 +134,48 @@ angular.module('navigation').directive('guacUserMenu', [function guacUserMenu()
return authenticationService.isAnonymous();
};
+ /**
+ * Requests fullscreen for the Guacamole page.
+ */
+ $scope.fullscreen = function fullscreen() {
+ var elem = document.documentElement;
+ var fullscreenElem = !(!document.fullscreenElement &&
+ !document.msFullscreenElement &&
+ !document.mozFullScreenElement &&
+ !document.webkitFullscreenElement);
+
+ if (!fullscreenElem) {
+ if (!elem.requestFullscreen) {
+ elem.requestFullscreen = (elem.mozRequestFullScreen
+ || elem.webkitRequestFullscreen
+ || elem.msRequestFullscreen).bind(elem);
+ }
Review comment:
It would be better to retrieving `document.documentElement` and ensure its `requestFullScreen()` and `exitFullscreen()` are defined just once, rather than each time `$scope.fullscreen()` is invoked. If these checks were moved into the body of the controller rather than the body of this function, I think this would become more readable (and would happen to also do less work).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] mike-jumper commented on a change in pull
request #470: GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#discussion_r383098982
##########
File path: guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js
##########
@@ -134,6 +134,48 @@ angular.module('navigation').directive('guacUserMenu', [function guacUserMenu()
return authenticationService.isAnonymous();
};
+ /**
+ * Requests fullscreen for the Guacamole page.
+ */
+ $scope.fullscreen = function fullscreen() {
+ var elem = document.documentElement;
+ var fullscreenElem = !(!document.fullscreenElement &&
+ !document.msFullscreenElement &&
+ !document.mozFullScreenElement &&
+ !document.webkitFullscreenElement);
+
+ if (!fullscreenElem) {
+ if (!elem.requestFullscreen) {
+ elem.requestFullscreen = (elem.mozRequestFullScreen
+ || elem.webkitRequestFullscreen
+ || elem.msRequestFullscreen).bind(elem);
+ }
+
+ if (elem.requestFullscreen) {
+ elem.requestFullscreen();
+ }
+ } else {
Review comment:
Please don't cuddle the `else`: http://guacamole.apache.org/guac-style/#braces
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] madmath03 commented on a change in pull request
#470: GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
madmath03 commented on a change in pull request #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#discussion_r375453637
##########
File path: guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js
##########
@@ -134,6 +134,45 @@ angular.module('navigation').directive('guacUserMenu', [function guacUserMenu()
return authenticationService.isAnonymous();
};
+ /**
+ * Requests fullscreen for the Guacamole page.
+ */
+ $scope.fullscreen = function fullscreen() {
+ var elem = document.documentElement;
+
+ if (!elem.fullscreenElement && !elem.mozFullScreenElement &&
+ !elem.webkitFullscreenElement && !elem.msFullscreenElement) {
+ if (elem.requestFullscreen) {
+ elem.requestFullscreen();
+ } else if (elem.mozRequestFullScreen) { /* Firefox */
+ elem.mozRequestFullScreen();
+ } else if (elem.webkitRequestFullscreen) { /* Chrome, Safari and Opera */
+ elem.webkitRequestFullscreen();
+ } else if (elem.msRequestFullscreen) { /* IE/Edge */
+ elem.msRequestFullscreen();
+ }
+ } else {
+ if (elem.exitFullscreen) {
+ elem.exitFullscreen();
+ } else if (elem.mozCancelFullScreen) { /* Firefox */
+ elem.mozCancelFullScreen();
+ } else if (elem.webkitExitFullscreen) { /* Chrome, Safari and Opera */
+ elem.webkitExitFullscreen();
+ } else if (elem.msExitFullscreen) { /* IE/Edge */
+ elem.msExitFullscreen();
+ }
+ }
Review comment:
@mike-jumper That should be good now
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] madmath03 commented on a change in pull request
#470: GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
madmath03 commented on a change in pull request #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#discussion_r373949118
##########
File path: guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js
##########
@@ -134,6 +134,45 @@ angular.module('navigation').directive('guacUserMenu', [function guacUserMenu()
return authenticationService.isAnonymous();
};
+ /**
+ * Requests fullscreen for the Guacamole page.
+ */
+ $scope.fullscreen = function fullscreen() {
+ var elem = document.documentElement;
+
+ if (!elem.fullscreenElement && !elem.mozFullScreenElement &&
+ !elem.webkitFullscreenElement && !elem.msFullscreenElement) {
+ if (elem.requestFullscreen) {
+ elem.requestFullscreen();
+ } else if (elem.mozRequestFullScreen) { /* Firefox */
+ elem.mozRequestFullScreen();
+ } else if (elem.webkitRequestFullscreen) { /* Chrome, Safari and Opera */
+ elem.webkitRequestFullscreen();
+ } else if (elem.msRequestFullscreen) { /* IE/Edge */
+ elem.msRequestFullscreen();
+ }
+ } else {
+ if (elem.exitFullscreen) {
+ elem.exitFullscreen();
+ } else if (elem.mozCancelFullScreen) { /* Firefox */
+ elem.mozCancelFullScreen();
+ } else if (elem.webkitExitFullscreen) { /* Chrome, Safari and Opera */
+ elem.webkitExitFullscreen();
+ } else if (elem.msExitFullscreen) { /* IE/Edge */
+ elem.msExitFullscreen();
+ }
+ }
Review comment:
I have not done much testing yet, which is why I am keeping this as a draft for now.The few tests i have done so far were pretty good, ie. VNC/SSH would just resize as you would expect.
I need to test RDP, and also do tests on more browsers / devices.
And yeah, there is a standard way to enter / exit fullscreen but, as usual, not all browsers on the market moved to it yet :disappointed:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] madmath03 commented on a change in pull request
#470: GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
madmath03 commented on a change in pull request #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#discussion_r383575585
##########
File path: guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js
##########
@@ -134,6 +134,48 @@ angular.module('navigation').directive('guacUserMenu', [function guacUserMenu()
return authenticationService.isAnonymous();
};
+ /**
+ * Requests fullscreen for the Guacamole page.
+ */
+ $scope.fullscreen = function fullscreen() {
+ var elem = document.documentElement;
+ var fullscreenElem = !(!document.fullscreenElement &&
+ !document.msFullscreenElement &&
+ !document.mozFullScreenElement &&
+ !document.webkitFullscreenElement);
+
+ if (!fullscreenElem) {
+ if (!elem.requestFullscreen) {
+ elem.requestFullscreen = (elem.mozRequestFullScreen
+ || elem.webkitRequestFullscreen
+ || elem.msRequestFullscreen).bind(elem);
+ }
Review comment:
@mike-jumper If we move the tests for `requestFullScreen()` and `exitFullscreen()` into the body, then we could detect if the current browser has support for fullscreen before adding the function to the menu.
Should we still display the fullscreen menu or is it better to hide it ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] mike-jumper commented on a change in pull
request #470: GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#discussion_r383098865
##########
File path: guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js
##########
@@ -134,6 +134,48 @@ angular.module('navigation').directive('guacUserMenu', [function guacUserMenu()
return authenticationService.isAnonymous();
};
+ /**
+ * Requests fullscreen for the Guacamole page.
+ */
+ $scope.fullscreen = function fullscreen() {
+ var elem = document.documentElement;
+ var fullscreenElem = !(!document.fullscreenElement &&
+ !document.msFullscreenElement &&
+ !document.mozFullScreenElement &&
+ !document.webkitFullscreenElement);
Review comment:
`!!(foo || bar || baz)` might be more readable.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] necouchman commented on issue #470:
GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
necouchman commented on issue #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#issuecomment-582591272
@madmath03: Please note that all your commits need to have the "GAUCAMOLE-124" tag on them. Also, might want to consider squashing the commits into a smaller number - five is a lot of commits for the small amount of code that is being changed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] madmath03 commented on a change in pull request
#470: GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
madmath03 commented on a change in pull request #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#discussion_r386148294
##########
File path: guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js
##########
@@ -134,6 +135,64 @@ angular.module('navigation').directive('guacUserMenu', [function guacUserMenu()
return authenticationService.isAnonymous();
};
+ /**
+ * All available actions for the current user.
+ */
+ $scope.actions = [ ];
+
+ // Initialize fullscreen functions
+ var docElem = document.documentElement;
+ var requestFullscreen = docElem.mozRequestFullScreen
+ || docElem.webkitRequestFullscreen
+ || docElem.msRequestFullscreen;
+ var exitFullscreen = document.mozCancelFullScreen
+ || document.webkitExitFullscreen
+ || document.msExitFullscreen;
+ if (!!requestFullscreen && !!exitFullscreen) {
+ // Bind browser-specific fullscreen functions
+ if (!docElem.requestFullscreen) {
+ docElem.requestFullscreen = (requestFullscreen).bind(docElem);
+ }
+ if (!document.exitFullscreen) {
+ document.exitFullscreen = (exitFullscreen).bind(document);
+ }
+
+ /**
+ * Toggles fullscreen for the Guacamole page.
+ */
+ $scope.fullscreen = function fullscreen() {
+ var fullscreenElem = !!(document.fullscreenElement
+ || document.msFullscreenElement
+ || document.mozFullScreenElement
+ || document.webkitFullscreenElement);
+
+ if (!fullscreenElem) {
+ document.documentElement.requestFullscreen();
+ }
+ else {
+ document.exitFullscreen();
+ }
+ };
+
+ /**
+ * Action which requests fullscreen for the Guacamole page.
+ */
+ var FULLSCREEN_ACTION = {
+ name : 'USER_MENU.ACTION_FULLSCREEN',
+ className : 'fullscreen',
+ callback : $scope.fullscreen
+ };
+
+ /**
+ * Add fullscreen action for the current user.
+ */
+ $scope.actions = $scope.actions.concat( FULLSCREEN_ACTION );
Review comment:
You're right. I should have added the standard version in `requestFullscreen` and `exitFullscreen`.
Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] mike-jumper commented on a change in pull
request #470: GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#discussion_r383100579
##########
File path: guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js
##########
@@ -134,6 +134,48 @@ angular.module('navigation').directive('guacUserMenu', [function guacUserMenu()
return authenticationService.isAnonymous();
};
+ /**
+ * Requests fullscreen for the Guacamole page.
Review comment:
This function doesn't just request that the Guacamole page enter fullscreen mode, but toggles fullscreen.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] madmath03 commented on issue #470:
GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
madmath03 commented on issue #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#issuecomment-593159036
hello @mike-jumper
I've applied the modifications you requested (not a 100% sure about the last one though).
Let me know if there is anything else.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] mike-jumper commented on a change in pull
request #470: GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#discussion_r385863842
##########
File path: guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js
##########
@@ -134,6 +135,64 @@ angular.module('navigation').directive('guacUserMenu', [function guacUserMenu()
return authenticationService.isAnonymous();
};
+ /**
+ * All available actions for the current user.
+ */
+ $scope.actions = [ ];
+
+ // Initialize fullscreen functions
+ var docElem = document.documentElement;
+ var requestFullscreen = docElem.mozRequestFullScreen
+ || docElem.webkitRequestFullscreen
+ || docElem.msRequestFullscreen;
+ var exitFullscreen = document.mozCancelFullScreen
+ || document.webkitExitFullscreen
+ || document.msExitFullscreen;
+ if (!!requestFullscreen && !!exitFullscreen) {
+ // Bind browser-specific fullscreen functions
+ if (!docElem.requestFullscreen) {
+ docElem.requestFullscreen = (requestFullscreen).bind(docElem);
+ }
+ if (!document.exitFullscreen) {
+ document.exitFullscreen = (exitFullscreen).bind(document);
+ }
+
+ /**
+ * Toggles fullscreen for the Guacamole page.
+ */
+ $scope.fullscreen = function fullscreen() {
+ var fullscreenElem = !!(document.fullscreenElement
+ || document.msFullscreenElement
+ || document.mozFullScreenElement
+ || document.webkitFullscreenElement);
+
+ if (!fullscreenElem) {
+ document.documentElement.requestFullscreen();
+ }
+ else {
+ document.exitFullscreen();
+ }
+ };
+
+ /**
+ * Action which requests fullscreen for the Guacamole page.
+ */
+ var FULLSCREEN_ACTION = {
+ name : 'USER_MENU.ACTION_FULLSCREEN',
+ className : 'fullscreen',
+ callback : $scope.fullscreen
+ };
+
+ /**
+ * Add fullscreen action for the current user.
+ */
+ $scope.actions = $scope.actions.concat( FULLSCREEN_ACTION );
Review comment:
Running this only within `if (!!requestFullscreen && !!exitFullscreen) { ... }` will result in the fullscreen option being available only if there is a vendor-prefixed version of those functions available. If a browser implements the standard but no vendor-prefixed version, the option will not be present.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] madmath03 commented on issue #470:
GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
madmath03 commented on issue #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#issuecomment-598175918
@mike-jumper I've tested this on the same devices I mentioned initially and it all seems to work as expected.
Let me know if you need more changes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] madmath03 commented on issue #470:
GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
madmath03 commented on issue #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#issuecomment-582614853
@necouchman done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] mike-jumper commented on a change in pull
request #470: GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#discussion_r373899959
##########
File path: guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js
##########
@@ -134,6 +134,45 @@ angular.module('navigation').directive('guacUserMenu', [function guacUserMenu()
return authenticationService.isAnonymous();
};
+ /**
+ * Requests fullscreen for the Guacamole page.
+ */
+ $scope.fullscreen = function fullscreen() {
+ var elem = document.documentElement;
+
+ if (!elem.fullscreenElement && !elem.mozFullScreenElement &&
+ !elem.webkitFullscreenElement && !elem.msFullscreenElement) {
+ if (elem.requestFullscreen) {
+ elem.requestFullscreen();
+ } else if (elem.mozRequestFullScreen) { /* Firefox */
+ elem.mozRequestFullScreen();
+ } else if (elem.webkitRequestFullscreen) { /* Chrome, Safari and Opera */
+ elem.webkitRequestFullscreen();
+ } else if (elem.msRequestFullscreen) { /* IE/Edge */
+ elem.msRequestFullscreen();
+ }
+ } else {
+ if (elem.exitFullscreen) {
+ elem.exitFullscreen();
+ } else if (elem.mozCancelFullScreen) { /* Firefox */
+ elem.mozCancelFullScreen();
+ } else if (elem.webkitExitFullscreen) { /* Chrome, Safari and Opera */
+ elem.webkitExitFullscreen();
+ } else if (elem.msExitFullscreen) { /* IE/Edge */
+ elem.msExitFullscreen();
+ }
+ }
Review comment:
There may be a cleaner way to shim this. For example:
https://github.com/apache/guacamole-client/blob/b51d2a88b8415738f6fff19cc99bc2b3e4690da2/guacamole-common-js/src/main/webapp/modules/AudioRecorder.js#L189-L197
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] madmath03 commented on issue #470:
GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
madmath03 commented on issue #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#issuecomment-592551858
hello @mike-jumper
Let me know if the corrections applied are valid for you.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] madmath03 commented on issue #470:
GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
madmath03 commented on issue #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#issuecomment-612054709
> @madmath03 : I've tested it on `Version 80.0.3987.149 (Official Build) (64-bit)` under `Windows 10 Pro 64-bit` and it's working OK!
Glad to hear that 😃
> IMO there should be some sort of toggle switch or visual change when full-screen toggled on, like changing "Full Screen" value to "Turn off Full Screen".
That might be nice indeed, but since the action name and CSS classes are constants, I'm not sure if there is a way to do that easily though...
@necouchman & @mike-jumper is that something possible and if so how ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] madmath03 commented on a change in pull request
#470: GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
madmath03 commented on a change in pull request #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#discussion_r384653406
##########
File path: guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js
##########
@@ -134,6 +134,48 @@ angular.module('navigation').directive('guacUserMenu', [function guacUserMenu()
return authenticationService.isAnonymous();
};
+ /**
+ * Requests fullscreen for the Guacamole page.
+ */
+ $scope.fullscreen = function fullscreen() {
+ var elem = document.documentElement;
+ var fullscreenElem = !(!document.fullscreenElement &&
+ !document.msFullscreenElement &&
+ !document.mozFullScreenElement &&
+ !document.webkitFullscreenElement);
+
+ if (!fullscreenElem) {
+ if (!elem.requestFullscreen) {
+ elem.requestFullscreen = (elem.mozRequestFullScreen
+ || elem.webkitRequestFullscreen
+ || elem.msRequestFullscreen).bind(elem);
+ }
+
+ if (elem.requestFullscreen) {
+ elem.requestFullscreen();
+ }
Review comment:
Given the fact that we will hide the fullscreen option if not supported, I will as well log a warning.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] necouchman commented on a change in pull request
#470: GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#discussion_r373899221
##########
File path: guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js
##########
@@ -134,6 +134,45 @@ angular.module('navigation').directive('guacUserMenu', [function guacUserMenu()
return authenticationService.isAnonymous();
};
+ /**
+ * Requests fullscreen for the Guacamole page.
+ */
+ $scope.fullscreen = function fullscreen() {
+ var elem = document.documentElement;
+
+ if (!elem.fullscreenElement && !elem.mozFullScreenElement &&
+ !elem.webkitFullscreenElement && !elem.msFullscreenElement) {
+ if (elem.requestFullscreen) {
+ elem.requestFullscreen();
+ } else if (elem.mozRequestFullScreen) { /* Firefox */
+ elem.mozRequestFullScreen();
+ } else if (elem.webkitRequestFullscreen) { /* Chrome, Safari and Opera */
+ elem.webkitRequestFullscreen();
+ } else if (elem.msRequestFullscreen) { /* IE/Edge */
+ elem.msRequestFullscreen();
+ }
+ } else {
+ if (elem.exitFullscreen) {
+ elem.exitFullscreen();
+ } else if (elem.mozCancelFullScreen) { /* Firefox */
+ elem.mozCancelFullScreen();
+ } else if (elem.webkitExitFullscreen) { /* Chrome, Safari and Opera */
+ elem.webkitExitFullscreen();
+ } else if (elem.msExitFullscreen) { /* IE/Edge */
+ elem.msExitFullscreen();
+ }
+ }
Review comment:
Wow, this is fantastic - no reflection on you, but apparently no standard way to enter full-screen mode. :rofl:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] mike-jumper commented on a change in pull
request #470: GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#discussion_r383626528
##########
File path: guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js
##########
@@ -134,6 +134,48 @@ angular.module('navigation').directive('guacUserMenu', [function guacUserMenu()
return authenticationService.isAnonymous();
};
+ /**
+ * Requests fullscreen for the Guacamole page.
+ */
+ $scope.fullscreen = function fullscreen() {
+ var elem = document.documentElement;
+ var fullscreenElem = !(!document.fullscreenElement &&
+ !document.msFullscreenElement &&
+ !document.mozFullScreenElement &&
+ !document.webkitFullscreenElement);
+
+ if (!fullscreenElem) {
+ if (!elem.requestFullscreen) {
+ elem.requestFullscreen = (elem.mozRequestFullScreen
+ || elem.webkitRequestFullscreen
+ || elem.msRequestFullscreen).bind(elem);
+ }
Review comment:
Definitely better to hide it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] madmath03 commented on a change in pull request
#470: GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
madmath03 commented on a change in pull request #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#discussion_r383574928
##########
File path: guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js
##########
@@ -134,6 +134,48 @@ angular.module('navigation').directive('guacUserMenu', [function guacUserMenu()
return authenticationService.isAnonymous();
};
+ /**
+ * Requests fullscreen for the Guacamole page.
+ */
+ $scope.fullscreen = function fullscreen() {
+ var elem = document.documentElement;
+ var fullscreenElem = !(!document.fullscreenElement &&
+ !document.msFullscreenElement &&
+ !document.mozFullScreenElement &&
+ !document.webkitFullscreenElement);
+
+ if (!fullscreenElem) {
+ if (!elem.requestFullscreen) {
+ elem.requestFullscreen = (elem.mozRequestFullScreen
+ || elem.webkitRequestFullscreen
+ || elem.msRequestFullscreen).bind(elem);
+ }
+
+ if (elem.requestFullscreen) {
+ elem.requestFullscreen();
+ }
Review comment:
Ok.
Do you prefer the stub function to really do nothing or is it okay if it logs a warning ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] madmath03 commented on a change in pull request
#470: GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
madmath03 commented on a change in pull request #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#discussion_r373949485
##########
File path: guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js
##########
@@ -134,6 +134,45 @@ angular.module('navigation').directive('guacUserMenu', [function guacUserMenu()
return authenticationService.isAnonymous();
};
+ /**
+ * Requests fullscreen for the Guacamole page.
+ */
+ $scope.fullscreen = function fullscreen() {
+ var elem = document.documentElement;
+
+ if (!elem.fullscreenElement && !elem.mozFullScreenElement &&
+ !elem.webkitFullscreenElement && !elem.msFullscreenElement) {
+ if (elem.requestFullscreen) {
+ elem.requestFullscreen();
+ } else if (elem.mozRequestFullScreen) { /* Firefox */
+ elem.mozRequestFullScreen();
+ } else if (elem.webkitRequestFullscreen) { /* Chrome, Safari and Opera */
+ elem.webkitRequestFullscreen();
+ } else if (elem.msRequestFullscreen) { /* IE/Edge */
+ elem.msRequestFullscreen();
+ }
+ } else {
+ if (elem.exitFullscreen) {
+ elem.exitFullscreen();
+ } else if (elem.mozCancelFullScreen) { /* Firefox */
+ elem.mozCancelFullScreen();
+ } else if (elem.webkitExitFullscreen) { /* Chrome, Safari and Opera */
+ elem.webkitExitFullscreen();
+ } else if (elem.msExitFullscreen) { /* IE/Edge */
+ elem.msExitFullscreen();
+ }
+ }
Review comment:
Interesting! I will test if I can shim this like you proposed.Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [guacamole-client] madmath03 edited a comment on issue #470:
GUACAMOLE-124: Add full-screen action
Posted by GitBox <gi...@apache.org>.
madmath03 edited a comment on issue #470: GUACAMOLE-124: Add full-screen action
URL: https://github.com/apache/guacamole-client/pull/470#issuecomment-582584855
> Overall this looks pretty solid to me. My only question is how much testing you've done with scaling/resizing with this? Presumably when it goes to full-screen the screen resolution changes, which, depending on the protocol in use for the connection, will result in either an on-the-fly resize (RDP, maybe SSH/Telnet), a disconnect/reconnect (RDP), or some extra black space around the screen area. What sort of behavior have you seen?
I did some more tests on different devices / browsers and connections. Here are the results:
| Device / Browser | :checkered_flag: Windows RDP | :penguin: Linux SSH | :penguin: Linux VNC (TigerVNC) |
| ------------------- | --------------- | ----------- | ------------------------ |
| :checkered_flag: Windows / Chromium | :warning: | :white_check_mark: | :white_check_mark: |
| :checkered_flag: Windows / Firefox | :warning: | :white_check_mark: | :white_check_mark: |
| :checkered_flag: Windows / Edge | :warning: | :white_check_mark: | :white_check_mark: |
| :checkered_flag: Windows / IE 11 | :no_entry_sign: | :no_entry_sign: | :no_entry_sign: |
| :robot: Android Tablet / Chromium | :warning: | :white_check_mark: | :white_check_mark: |
| :robot: Android Tablet / Firefox | :warning: | :white_check_mark: | :white_check_mark: |
| :green_apple: iPhone / Safari | :x: | :x: | :x: |
- :white_check_mark: : Fullscreen works properly and connection is immediately resized
- :warning: : Fullscreen button works but Windows RDP connection displays empty space and needs to be restarted to be resized (disconnect / reconnect, no need to go back to home)
- :no_entry_sign: : Fullscreen button works but only displays a black screen during connection, whatever the connection type
- :x: : Fullscreen button does not trigger fullscreen
So basically:
* IE 11 and Safari do not work
* RDP connection needs reconnect (seems to be the normal behavior of RDP)
* everything else is good
@necouchman & @mike-jumper : I do not have a Mac to debug behavior on Safari, and I do not believe IE 11 is relevant anymore, so I think that's good enough to be used, or at least to be reviewed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services