You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by mike-jumper <gi...@git.apache.org> on 2018/01/15 04:57:49 UTC
[GitHub] guacamole-client pull request #227: GUACAMOLE-161: Handle known platform/bro...
GitHub user mike-jumper opened a pull request:
https://github.com/apache/guacamole-client/pull/227
GUACAMOLE-161: Handle known platform/browser keyboard quirks semantically.
This change adds a new private set of boolean flags within `Guacamole.Keyboard` which tracks whether certain platform-specific and browser-specific quirks are present, particularly those which can be reliably detected only through platform/browser-sniffing, specifically:
* Whether keyup events cannot really be relied upon at all (they are randomly dropped on iOS)
* Whether the keyup event is specifically unreliable for Caps Lock (a quirk of the Mac OS X event model, as noted by @flangelo in #209: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent#Special_cases)
* Whether the "Alt" key is safe to allow through to the browser, as it never represents a keyboard shortcut.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mike-jumper/guacamole-client keyboard-quirks
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/guacamole-client/pull/227.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #227
----
commit 10f200b5301177ec24e26470977252943258120c
Author: Michael Jumper <mj...@...>
Date: 2018-01-15T04:35:45Z
GUACAMOLE-161: Semantically represent platform/browser key event quirks.
commit 67df92eabd4546b3b481b0b9478c9b5d80d6b2c4
Author: Michael Jumper <mj...@...>
Date: 2018-01-15T04:44:22Z
GUACAMOLE-161: Do not rely on receiving keyup for Caps Lock on Mac (only keydown is dispatched).
----
---
[GitHub] guacamole-client pull request #227: GUACAMOLE-232: Handle known platform/bro...
Posted by necouchman <gi...@git.apache.org>.
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/227#discussion_r161612019
--- Diff: guacamole-common-js/src/main/webapp/modules/Keyboard.js ---
@@ -55,6 +55,47 @@ Guacamole.Keyboard = function(element) {
*/
this.onkeyup = null;
+ /**
+ * Set of known platform-specific or browser-specific quirks which must be
+ * accounted for to properly interpret key events, even if the only way to
+ * reliably detect that quirk is to platform/browser-sniff.
+ *
+ * @private
+ * @type {Object.<String, Boolean>}
+ */
+ var quirks = {
+
+ /**
+ * Whether keyup events are universally unreliable.
+ *
+ * @type {Boolean}
+ */
+ keyupUnreliable: false,
+
+ /**
+ * Whether the Alt key is actually a modifier for typable keys and is
+ * thus never used for keyboard shortcuts.
+ *
+ * @type {Boolean}
+ */
+ altIsTypableOnly: false
+
+ };
+
+ // Set quirk flags depending on platform/browser, if such information is
+ // available
+ if (navigator && navigator.platform) {
+
+ // All keyup events are unreliable on iOS (sadly)
+ if (navigator.platform.match(/ipad|iphone|ipod/i))
+ quirks.keyupUnreliable = true;
+
+ // The Alt key on Mac is never used for keyboard shortcuts
+ else if (navigator.platform.match(/^mac/i))
+ quirks.altIsTypableOnly = true;
+
--- End diff --
What kind of impact does this have on Ctrl-Alt-Shift for accessing the Guacamole Client menu, and the recently-added Ctrl-Alt-End for sending Ctrl-Alt-Delete?
---
[GitHub] guacamole-client pull request #227: GUACAMOLE-232: Handle known platform/bro...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/guacamole-client/pull/227
---
[GitHub] guacamole-client pull request #227: GUACAMOLE-232: Handle known platform/bro...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/227#discussion_r161618151
--- Diff: guacamole-common-js/src/main/webapp/modules/Keyboard.js ---
@@ -55,6 +55,47 @@ Guacamole.Keyboard = function(element) {
*/
this.onkeyup = null;
+ /**
+ * Set of known platform-specific or browser-specific quirks which must be
+ * accounted for to properly interpret key events, even if the only way to
+ * reliably detect that quirk is to platform/browser-sniff.
+ *
+ * @private
+ * @type {Object.<String, Boolean>}
+ */
+ var quirks = {
+
+ /**
+ * Whether keyup events are universally unreliable.
+ *
+ * @type {Boolean}
+ */
+ keyupUnreliable: false,
+
+ /**
+ * Whether the Alt key is actually a modifier for typable keys and is
+ * thus never used for keyboard shortcuts.
+ *
+ * @type {Boolean}
+ */
+ altIsTypableOnly: false
+
+ };
+
+ // Set quirk flags depending on platform/browser, if such information is
+ // available
+ if (navigator && navigator.platform) {
+
+ // All keyup events are unreliable on iOS (sadly)
+ if (navigator.platform.match(/ipad|iphone|ipod/i))
+ quirks.keyupUnreliable = true;
+
+ // The Alt key on Mac is never used for keyboard shortcuts
+ else if (navigator.platform.match(/^mac/i))
+ quirks.altIsTypableOnly = true;
+
--- End diff --
This particular one shouldn't have any impact, as it's actually just a refactored approach to the same behavior handled previously:
https://github.com/apache/guacamole-client/blob/c4ba495cca96f9823742083a8ecbaafb6be3c704/guacamole-common-js/src/main/webapp/modules/Keyboard.js#L187-L189
vs.
https://github.com/apache/guacamole-client/blob/960e83f780425d93bf064f0187fd481cb848b150/guacamole-common-js/src/main/webapp/modules/Keyboard.js#L241-L242
The other `keyupUnreliable` quirk for iOS might impact keyboard shortcuts in general, though. I'll retest.
---
[GitHub] guacamole-client pull request #227: GUACAMOLE-232: Handle known platform/bro...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/227#discussion_r161620053
--- Diff: guacamole-common-js/src/main/webapp/modules/Keyboard.js ---
@@ -55,6 +55,47 @@ Guacamole.Keyboard = function(element) {
*/
this.onkeyup = null;
+ /**
+ * Set of known platform-specific or browser-specific quirks which must be
+ * accounted for to properly interpret key events, even if the only way to
+ * reliably detect that quirk is to platform/browser-sniff.
+ *
+ * @private
+ * @type {Object.<String, Boolean>}
+ */
+ var quirks = {
+
+ /**
+ * Whether keyup events are universally unreliable.
+ *
+ * @type {Boolean}
+ */
+ keyupUnreliable: false,
+
+ /**
+ * Whether the Alt key is actually a modifier for typable keys and is
+ * thus never used for keyboard shortcuts.
+ *
+ * @type {Boolean}
+ */
+ altIsTypableOnly: false
+
+ };
+
+ // Set quirk flags depending on platform/browser, if such information is
+ // available
+ if (navigator && navigator.platform) {
+
+ // All keyup events are unreliable on iOS (sadly)
+ if (navigator.platform.match(/ipad|iphone|ipod/i))
+ quirks.keyupUnreliable = true;
+
+ // The Alt key on Mac is never used for keyboard shortcuts
+ else if (navigator.platform.match(/^mac/i))
+ quirks.altIsTypableOnly = true;
+
--- End diff --
Apparently, keydown for modifier keys on iOS is not currently properly handled (the key identity is missing from the event), but the tracking of modifier state used `Guacamole.Keyboard` to automatically release modifiers can be used to also properly press them, and things should then work as expected.
I'll make the necessary change.
---
[GitHub] guacamole-client pull request #227: GUACAMOLE-232: Handle known platform/bro...
Posted by necouchman <gi...@git.apache.org>.
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/227#discussion_r161811586
--- Diff: guacamole-common-js/src/main/webapp/modules/Keyboard.js ---
@@ -55,6 +55,47 @@ Guacamole.Keyboard = function(element) {
*/
this.onkeyup = null;
+ /**
+ * Set of known platform-specific or browser-specific quirks which must be
+ * accounted for to properly interpret key events, even if the only way to
+ * reliably detect that quirk is to platform/browser-sniff.
+ *
+ * @private
+ * @type {Object.<String, Boolean>}
+ */
+ var quirks = {
+
+ /**
+ * Whether keyup events are universally unreliable.
+ *
+ * @type {Boolean}
+ */
+ keyupUnreliable: false,
+
+ /**
+ * Whether the Alt key is actually a modifier for typable keys and is
+ * thus never used for keyboard shortcuts.
+ *
+ * @type {Boolean}
+ */
+ altIsTypableOnly: false
+
+ };
+
+ // Set quirk flags depending on platform/browser, if such information is
+ // available
+ if (navigator && navigator.platform) {
+
+ // All keyup events are unreliable on iOS (sadly)
+ if (navigator.platform.match(/ipad|iphone|ipod/i))
+ quirks.keyupUnreliable = true;
+
+ // The Alt key on Mac is never used for keyboard shortcuts
+ else if (navigator.platform.match(/^mac/i))
+ quirks.altIsTypableOnly = true;
+
--- End diff --
:+1:
---
[GitHub] guacamole-client pull request #227: GUACAMOLE-232: Handle known platform/bro...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/227#discussion_r161623546
--- Diff: guacamole-common-js/src/main/webapp/modules/Keyboard.js ---
@@ -55,6 +55,47 @@ Guacamole.Keyboard = function(element) {
*/
this.onkeyup = null;
+ /**
+ * Set of known platform-specific or browser-specific quirks which must be
+ * accounted for to properly interpret key events, even if the only way to
+ * reliably detect that quirk is to platform/browser-sniff.
+ *
+ * @private
+ * @type {Object.<String, Boolean>}
+ */
+ var quirks = {
+
+ /**
+ * Whether keyup events are universally unreliable.
+ *
+ * @type {Boolean}
+ */
+ keyupUnreliable: false,
+
+ /**
+ * Whether the Alt key is actually a modifier for typable keys and is
+ * thus never used for keyboard shortcuts.
+ *
+ * @type {Boolean}
+ */
+ altIsTypableOnly: false
+
+ };
+
+ // Set quirk flags depending on platform/browser, if such information is
+ // available
+ if (navigator && navigator.platform) {
+
+ // All keyup events are unreliable on iOS (sadly)
+ if (navigator.platform.match(/ipad|iphone|ipod/i))
+ quirks.keyupUnreliable = true;
+
+ // The Alt key on Mac is never used for keyboard shortcuts
+ else if (navigator.platform.match(/^mac/i))
+ quirks.altIsTypableOnly = true;
+
--- End diff --
OK - that should be better. I've updated the way that `Guacamole.Keyboard` resynchronizes modifier state such that it also presses modifiers (it previously only released them), and cleaned things up somewhat to avoid repeating the same series of checks.
---