From 58ded70d150c31df2fbd78c0320af6edc72610fc Mon Sep 17 00:00:00 2001 From: Solly Ross Date: Tue, 24 Mar 2015 15:05:38 -0400 Subject: [PATCH 1/2] Create RFB object on connect In e543525faa9cf0d683f41e183e89cd909f3dd229, we switched to creating a new RFB object on disconnect. This caused issues, however, since any errors were only displayed briefly before the new "loaded" text was displayed instead. Now, we create the RFB object on connect. This essentially removes the usefulness of the "loaded" state, but prevents the aforementioned problem. To facilitate this, the code which does detection of cursor URI support was moved from this Display constructor (which now calls the new function) into its own function, `Util.browserSupportsCursorURIs()`. Fixes #467 --- include/display.js | 26 +++----------------------- include/ui.js | 36 +++++++++++++++++------------------- include/util.js | 23 +++++++++++++++++++++++ tests/test.display.js | 16 +++++----------- vnc.html | 4 ++-- 5 files changed, 50 insertions(+), 55 deletions(-) diff --git a/include/display.js b/include/display.js index d10d9b9a..201acf33 100644 --- a/include/display.js +++ b/include/display.js @@ -86,29 +86,9 @@ var Display; } // Determine browser support for setting the cursor via data URI scheme - var curDat = []; - for (var i = 0; i < 8 * 8 * 4; i++) { - curDat.push(255); - } - try { - var curSave = this._target.style.cursor; - Display.changeCursor(this._target, curDat, curDat, 2, 2, 8, 8); - if (this._target.style.cursor) { - if (this._cursor_uri === null || this._cursor_uri === undefined) { - this._cursor_uri = true; - } - Util.Info("Data URI scheme cursor supported"); - this._target.style.cursor = curSave; - } else { - if (this._cursor_uri === null || this._cursor_uri === undefined) { - this._cursor_uri = false; - } - Util.Warn("Data URI scheme cursor not supported"); - this._target.style.cursor = "none"; - } - } catch (exc) { - Util.Error("Data URI scheme cursor test exception: " + exc); - this._cursor_uri = false; + if (this._cursor_uri || this._cursor_uri === null || + this._cursor_uri === undefined) { + this._cursor_uri = Util.browserSupportsCursorURIs(this._target); } Util.Debug("<< Display.constructor"); diff --git a/include/ui.js b/include/ui.js index 25f8617c..a72d5fa9 100644 --- a/include/ui.js +++ b/include/ui.js @@ -97,8 +97,6 @@ var UI; UI.initSetting('path', 'websockify'); UI.initSetting('repeaterID', ''); - UI.initRFB(); - var autoconnect = WebUtil.getQueryVar('autoconnect', false); if (autoconnect === 'true' || autoconnect == '1') { autoconnect = true; @@ -136,7 +134,7 @@ var UI; Util.addEvent(window, 'load', UI.keyboardinputReset); Util.addEvent(window, 'beforeunload', function () { - if (UI.rfb_state === 'normal') { + if (UI.rfb && UI.rfb_state === 'normal') { return "You are currently connected."; } } ); @@ -213,12 +211,14 @@ var UI; $D("noVNC_connect_button").onclick = UI.connect; $D("noVNC_resize").onchange = function () { - var connected = UI.rfb_state === 'normal' ? true : false; + var connected = UI.rfb && UI.rfb_state === 'normal'; UI.enableDisableClip(connected); }; }, onresize: function (callback) { + if (!UI.rfb) return; + var size = UI.getCanvasLimit(); if (size && UI.rfb_state === 'normal' && UI.rfb.get_display()) { @@ -480,7 +480,7 @@ var UI; } else { UI.updateSetting('encrypt'); UI.updateSetting('true_color'); - if (UI.rfb.get_display().get_cursor_uri()) { + if (Util.browserSupportsCursorURIs()) { UI.updateSetting('cursor'); } else { UI.updateSetting('cursor', !UI.isTouchDevice); @@ -536,7 +536,7 @@ var UI; //Util.Debug(">> settingsApply"); UI.saveSetting('encrypt'); UI.saveSetting('true_color'); - if (UI.rfb.get_display().get_cursor_uri()) { + if (Util.browserSupportsCursorURIs()) { UI.saveSetting('cursor'); } @@ -558,7 +558,7 @@ var UI; WebUtil.selectStylesheet(UI.getSetting('stylesheet')); WebUtil.init_logging(UI.getSetting('logging')); UI.setViewClip(); - UI.setViewDrag(UI.rfb.get_viewportDrag()); + UI.setViewDrag(UI.rfb && UI.rfb.get_viewportDrag()); //Util.Debug("<< settingsApply"); }, @@ -642,13 +642,6 @@ var UI; break; } - switch (state) { - case 'fatal': - case 'failed': - case 'disconnected': - UI.initRFB(); - } - if (typeof(msg) !== 'undefined') { $D('noVNC-control-bar').setAttribute("class", klass); $D('noVNC_status').innerHTML = msg; @@ -659,13 +652,12 @@ var UI; // Disable/enable controls depending on connection state updateVisualState: function() { - var connected = UI.rfb_state === 'normal' ? true : false; + var connected = UI.rfb && UI.rfb_state === 'normal'; //Util.Debug(">> updateVisualState"); $D('noVNC_encrypt').disabled = connected; $D('noVNC_true_color').disabled = connected; - if (UI.rfb && UI.rfb.get_display() && - UI.rfb.get_display().get_cursor_uri()) { + if (Util.browserSupportsCursorURIs()) { $D('noVNC_cursor').disabled = connected; } else { UI.updateSetting('cursor', !UI.isTouchDevice); @@ -780,6 +772,8 @@ var UI; throw new Error("Must set host and port"); } + UI.initRFB(); + UI.rfb.set_encrypt(UI.getSetting('encrypt')); UI.rfb.set_true_color(UI.getSetting('true_color')); UI.rfb.set_local_cursor(UI.getSetting('cursor')); @@ -809,11 +803,15 @@ var UI; }, displayBlur: function() { + if (!UI.rfb) return; + UI.rfb.get_keyboard().set_focused(false); UI.rfb.get_mouse().set_focused(false); }, displayFocus: function() { + if (!UI.rfb) return; + UI.rfb.get_keyboard().set_focused(true); UI.rfb.get_mouse().set_focused(true); }, @@ -882,7 +880,7 @@ var UI; // Toggle/set/unset the viewport drag/move button setViewDrag: function(drag) { - if (!UI.rfb) { return; } + if (!UI.rfb) return; UI.updateViewDragButton(); @@ -953,7 +951,7 @@ var UI; // sending keyCodes in the normal keyboard events when using on screen keyboards. keyInput: function(event) { - if (!UI.rfb) { return; } + if (!UI.rfb) return; var newValue = event.target.value; diff --git a/include/util.js b/include/util.js index 02e72256..ed0e3cde 100644 --- a/include/util.js +++ b/include/util.js @@ -508,6 +508,29 @@ Util.stopEvent = function (e) { else { e.returnValue = false; } }; +Util._cursor_uris_supported = null; + +Util.browserSupportsCursorURIs = function () { + if (Util._cursor_uris_supported === null) { + try { + var target = document.createElement('canvas'); + target.style.cursor = 'url("data:image/x-icon;base64,AAACAAEACAgAAAIAAgA4AQAAFgAAACgAAAAIAAAAEAAAAAEAIAAAAAAAEAAAAAAAAAAAAAAAAAAAAAAAAAD/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////AAAAAAAAAAAAAAAAAAAAAA==") 2 2, default'; + + if (target.style.cursor) { + Util.Info("Data URI scheme cursor supported"); + Util._cursor_uris_supported = true; + } else { + Util.Warn("Data URI scheme cursor not supported"); + Util._cursor_uris_supported = false; + } + } catch (exc) { + Util.Error("Data URI scheme cursor test exception: " + exc); + Util._cursor_uris_supported = false; + } + } + + return Util._cursor_uris_supported; +}; // Set browser engine versions. Based on mootools. Util.Features = {xpath: !!(document.evaluate), air: !!(window.runtime), query: !!(document.querySelector)}; diff --git a/tests/test.display.js b/tests/test.display.js index d54cb82f..56dfc220 100644 --- a/tests/test.display.js +++ b/tests/test.display.js @@ -28,35 +28,29 @@ describe('Display/Canvas Helper', function () { describe('checking for cursor uri support', function () { beforeEach(function () { - this._old_change_cursor = Display.changeCursor; + this._old_browser_supports_cursor_uris = Util.browserSupportsCursorURIs; }); it('should disable cursor URIs if there is no support', function () { - Display.changeCursor = function(target) { - target.style.cursor = undefined; - }; + Util.browserSupportsCursorURIs = function () { return false; }; var display = new Display({ target: document.createElement('canvas'), prefer_js: true, viewport: false }); expect(display._cursor_uri).to.be.false; }); it('should enable cursor URIs if there is support', function () { - Display.changeCursor = function(target) { - target.style.cursor = 'pointer'; - }; + Util.browserSupportsCursorURIs = function () { return true; }; var display = new Display({ target: document.createElement('canvas'), prefer_js: true, viewport: false }); expect(display._cursor_uri).to.be.true; }); it('respect the cursor_uri option if there is support', function () { - Display.changeCursor = function(target) { - target.style.cursor = 'pointer'; - }; + Util.browserSupportsCursorURIs = function () { return false; }; var display = new Display({ target: document.createElement('canvas'), prefer_js: true, viewport: false, cursor_uri: false }); expect(display._cursor_uri).to.be.false; }); afterEach(function () { - Display.changeCursor = this._old_change_cursor; + Util.browserSupportsCursorURIs = this._old_browser_supports_cursor_uris; }); }); diff --git a/vnc.html b/vnc.html index b8bda05a..b8d11c70 100644 --- a/vnc.html +++ b/vnc.html @@ -46,7 +46,7 @@ -
+
-
Loading
+
From d9fc1c7be45f7be9f127636d5d63c3c224f05d1d Mon Sep 17 00:00:00 2001 From: Solly Ross Date: Tue, 24 Mar 2015 16:02:53 -0400 Subject: [PATCH 2/2] Throw exceptions from RFB constructor Previously, if an error was thrown from the Display constructor in the RFB constructor, we would attempt to use `RFB#updateState` to handle this. However, `RFB#updateState` attempts to close the WebSocket connection, which doesn't exist at this point. In the constructor, it's probably just better to raise an exception instead (making sure to clean up anything relevant). Fixes #460 --- include/rfb.js | 62 +++++++++++++++++++++++++++----------------------- include/ui.js | 22 +++++++++++------- vnc_auto.html | 30 ++++++++++++++---------- 3 files changed, 66 insertions(+), 48 deletions(-) diff --git a/include/rfb.js b/include/rfb.js index 6fcdab6f..b9db39c4 100644 --- a/include/rfb.js +++ b/include/rfb.js @@ -159,11 +159,13 @@ var RFB; this._encStats[this._encodings[i][1]] = [0, 0]; } + // NB: nothing that needs explicit teardown should be done + // before this point, since this can throw an exception try { this._display = new Display({target: this._target}); } catch (exc) { Util.Error("Display exception: " + exc); - this._updateState('fatal', "No working Display"); + throw exc; } this._keyboard = new Keyboard({target: this._focusContainer, @@ -217,9 +219,11 @@ var RFB; } else { Util.Warn("Using web-socket-js bridge. Flash version: " + Util.Flash.version); if (!Util.Flash || Util.Flash.version < 9) { - this._updateState('fatal', "WebSockets or Adobe Flash is required"); + this._cleanupSocket('fatal'); + throw new Exception("WebSockets or Adobe Flash is required"); } else if (document.location.href.substr(0, 7) === 'file://') { - this._updateState('fatal', "'file://' URL is incompatible with Adobe Flash"); + this._cleanupSocket('fatal'); + throw new Exception("'file://' URL is incompatible with Adobe Flash"); } else { this._updateState('loaded', 'noVNC ready: WebSockets emulation, ' + rmode); } @@ -398,6 +402,32 @@ var RFB; } }, + _cleanupSocket: function (state) { + if (this._sendTimer) { + clearInterval(this._sendTimer); + this._sendTimer = null; + } + + if (this._msgTimer) { + clearInterval(this._msgTimer); + this._msgTimer = null; + } + + if (this._display && this._display.get_context()) { + this._keyboard.ungrab(); + this._mouse.ungrab(); + if (state !== 'connect' && state !== 'loaded') { + this._display.defaultCursor(); + } + if (Util.get_logging() !== 'debug' || state === 'loaded') { + // Show noVNC logo on load and when disconnected, unless in + // debug mode + this._display.clear(); + } + } + + this._sock.close(); + }, /* * Page states: @@ -432,31 +462,7 @@ var RFB; */ if (state in {'disconnected': 1, 'loaded': 1, 'connect': 1, 'disconnect': 1, 'failed': 1, 'fatal': 1}) { - - if (this._sendTimer) { - clearInterval(this._sendTimer); - this._sendTimer = null; - } - - if (this._msgTimer) { - clearInterval(this._msgTimer); - this._msgTimer = null; - } - - if (this._display && this._display.get_context()) { - this._keyboard.ungrab(); - this._mouse.ungrab(); - if (state !== 'connect' && state !== 'loaded') { - this._display.defaultCursor(); - } - if (Util.get_logging() !== 'debug' || state === 'loaded') { - // Show noVNC logo on load and when disconnected, unless in - // debug mode - this._display.clear(); - } - } - - this._sock.close(); + this._cleanupSocket(state); } if (oldstate === 'fatal') { diff --git a/include/ui.js b/include/ui.js index a72d5fa9..93550780 100644 --- a/include/ui.js +++ b/include/ui.js @@ -159,13 +159,19 @@ var UI; }, initRFB: function () { - UI.rfb = new RFB({'target': $D('noVNC_canvas'), - 'onUpdateState': UI.updateState, - 'onXvpInit': UI.updateXvpVisualState, - 'onClipboard': UI.clipReceive, - 'onFBUComplete': UI.FBUComplete, - 'onFBResize': UI.updateViewDragButton, - 'onDesktopName': UI.updateDocumentTitle}); + try { + UI.rfb = new RFB({'target': $D('noVNC_canvas'), + 'onUpdateState': UI.updateState, + 'onXvpInit': UI.updateXvpVisualState, + 'onClipboard': UI.clipReceive, + 'onFBUComplete': UI.FBUComplete, + 'onFBResize': UI.updateViewDragButton, + 'onDesktopName': UI.updateDocumentTitle}); + return true; + } catch (exc) { + UI.updateState(null, 'fatal', null, 'Unable to create RFB client -- ' + exc); + return false; + } }, addMouseHandlers: function() { @@ -772,7 +778,7 @@ var UI; throw new Error("Must set host and port"); } - UI.initRFB(); + if (!UI.initRFB()) return; UI.rfb.set_encrypt(UI.getSetting('encrypt')); UI.rfb.set_true_color(UI.getSetting('true_color')); diff --git a/vnc_auto.html b/vnc_auto.html index 9fd2272a..ec18ab84 100644 --- a/vnc_auto.html +++ b/vnc_auto.html @@ -216,18 +216,24 @@ return; } - rfb = new RFB({'target': $D('noVNC_canvas'), - 'encrypt': WebUtil.getQueryVar('encrypt', - (window.location.protocol === "https:")), - 'repeaterID': WebUtil.getQueryVar('repeaterID', ''), - 'true_color': WebUtil.getQueryVar('true_color', true), - 'local_cursor': WebUtil.getQueryVar('cursor', true), - 'shared': WebUtil.getQueryVar('shared', true), - 'view_only': WebUtil.getQueryVar('view_only', false), - 'onUpdateState': updateState, - 'onXvpInit': xvpInit, - 'onPasswordRequired': passwordRequired, - 'onFBUComplete': FBUComplete}); + try { + rfb = new RFB({'target': $D('noVNC_canvas'), + 'encrypt': WebUtil.getQueryVar('encrypt', + (window.location.protocol === "https:")), + 'repeaterID': WebUtil.getQueryVar('repeaterID', ''), + 'true_color': WebUtil.getQueryVar('true_color', true), + 'local_cursor': WebUtil.getQueryVar('cursor', true), + 'shared': WebUtil.getQueryVar('shared', true), + 'view_only': WebUtil.getQueryVar('view_only', false), + 'onUpdateState': updateState, + 'onXvpInit': xvpInit, + 'onPasswordRequired': passwordRequired, + 'onFBUComplete': FBUComplete}); + } catch (exc) { + UI.updateState(null, 'fatal', null, 'Unable to create RFB client -- ' + exc); + return; // don't continue trying to connect + } + rfb.connect(host, port, password, path); };