mike-jumper commented on a change in pull request #592:
URL: https://github.com/apache/guacamole-client/pull/592#discussion_r578791816
##########
File path: guacamole/src/main/webapp/app/client/controllers/clientController.js
##########
@@ -219,6 +220,12 @@ angular.module('client').controller('clientController',
['$scope', '$routeParams
remaining: 15
};
+ // Catch window or tab closing (ctrl-w) and prompt the user
+ var ctrlwlistener = function onBeforeUnload(e) {
+ e.preventDefault();
+ e.returnValue = '';
+ };
Review comment:
1. Please correct the indentation here. The body of the function appears
to be indented far further than elsewhere (we use 4-space indents for each
level), and the closing brace of the function definition is misaligned.
2. Please use JSDoc to document functions, their return values, and their
parameters.
3. I think calling this function `ctrlwlistener()` is a bit misleading. Not
all browsers will use Ctrl+W for closing a tab or window, nor is this function
directly related to that shortcut.
##########
File path: guacamole/src/main/webapp/app/client/controllers/clientController.js
##########
@@ -776,6 +783,8 @@ angular.module('client').controller('clientController',
['$scope', '$routeParams
// Client error
else if (connectionState ===
ManagedClientState.ConnectionState.CLIENT_ERROR) {
+ // Stop intercepting window close
+ $window.removeEventListener('beforeunload', ctrlwlistener, false);
Review comment:
Why repeatedly add, remove, add, remove, etc. the listener vs. checking
the relevant state within the listener itself?
What about connections that are running in the background, as will occur if
the user navigates back to the home screen or to a settings screen without
disconnecting?
----------------------------------------------------------------
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:
[email protected]
