From ee5cae9fee484794211d89de55e9d454abd4d599 Mon Sep 17 00:00:00 2001 From: Samuel Mannehed Date: Fri, 10 Nov 2017 12:52:39 +0100 Subject: [PATCH] Replace updatestate event with connect Instead of exposing all the internal connection states, the RFB module will now only send events on connect and on disconnect. This makes it simpler for the application and gets rid of the double events that were being sent on disconnect (previously updatestate and disconnect). --- app/ui.js | 86 +++++++++++++++++++++++++++----------------- core/rfb.js | 31 ++++++++-------- docs/API.md | 27 ++++---------- tests/test.rfb.js | 91 +++++++++++++++++++++++++++++------------------ vnc_lite.html | 52 +++++++++------------------ 5 files changed, 149 insertions(+), 138 deletions(-) diff --git a/app/ui.js b/app/ui.js index 8bc704b2..0ee0c00b 100644 --- a/app/ui.js +++ b/app/ui.js @@ -105,7 +105,7 @@ var UI = { UI.updateViewClip(); - UI.updateVisualState(); + UI.updateVisualState('init'); document.documentElement.classList.remove("noVNC_loading"); @@ -388,53 +388,41 @@ var UI = { * VISUAL * ------v------*/ - updateState: function(event) { - var msg; + // Disable/enable controls depending on connection state + updateVisualState: function(state) { document.documentElement.classList.remove("noVNC_connecting"); document.documentElement.classList.remove("noVNC_connected"); document.documentElement.classList.remove("noVNC_disconnecting"); document.documentElement.classList.remove("noVNC_reconnecting"); - switch (event.detail.state) { + let transition_elem = document.getElementById("noVNC_transition_text"); + switch (state) { + case 'init': + break; case 'connecting': - document.getElementById("noVNC_transition_text").textContent = _("Connecting..."); + transition_elem.textContent = _("Connecting..."); document.documentElement.classList.add("noVNC_connecting"); break; case 'connected': - UI.connected = true; - UI.inhibit_reconnect = false; - UI.doneInitialResize = false; document.documentElement.classList.add("noVNC_connected"); - if (UI.getSetting('encrypt')) { - msg = _("Connected (encrypted) to ") + UI.desktopName; - } else { - msg = _("Connected (unencrypted) to ") + UI.desktopName; - } - UI.showStatus(msg); - document.getElementById('noVNC_canvas').focus(); break; case 'disconnecting': - UI.connected = false; - document.getElementById("noVNC_transition_text").textContent = _("Disconnecting..."); + transition_elem.textContent = _("Disconnecting..."); document.documentElement.classList.add("noVNC_disconnecting"); break; case 'disconnected': - UI.showStatus(_("Disconnected")); + break; + case 'reconnecting': + transition_elem.textContent = _("Reconnecting..."); + document.documentElement.classList.add("noVNC_reconnecting"); break; default: - Log.Error("Invalid visual state: " + event.detail.state); + Log.Error("Invalid visual state: " + state); UI.showStatus(_("Internal error"), 'error'); - break; + return; } - UI.updateVisualState(); - }, - - // Disable/enable controls depending on connection state - updateVisualState: function() { - //Log.Debug(">> updateVisualState"); - UI.enableDisableViewClip(); if (UI.connected) { @@ -480,8 +468,6 @@ var UI = { // State change also closes the password dialog document.getElementById('noVNC_password_dlg') .classList.remove('noVNC_open'); - - //Log.Debug("<< updateVisualState"); }, showStatus: function(text, status_type, time) { @@ -1016,6 +1002,8 @@ var UI = { UI.closeAllPanels(); UI.closeConnectPanel(); + UI.updateVisualState('connecting'); + UI.updateViewOnly(); var url; @@ -1032,7 +1020,7 @@ var UI = { { shared: UI.getSetting('shared'), repeaterID: UI.getSetting('repeaterID'), credentials: { password: password } }); - UI.rfb.addEventListener("updatestate", UI.updateState); + UI.rfb.addEventListener("connect", UI.connectFinished); UI.rfb.addEventListener("disconnect", UI.disconnectFinished); UI.rfb.addEventListener("credentialsrequired", UI.credentials); UI.rfb.addEventListener("capabilities", function () { UI.updatePowerButton(); UI.initialResize(); }); @@ -1046,9 +1034,13 @@ var UI = { UI.closeAllPanels(); UI.rfb.disconnect(); + UI.connected = false; + // Disable automatic reconnecting UI.inhibit_reconnect = true; + UI.updateVisualState('disconnecting'); + // Don't display the connection settings until we're actually disconnected }, @@ -1063,16 +1055,43 @@ var UI = { UI.connect(null, UI.reconnect_password); }, + connectFinished: function (e) { + UI.connected = true; + UI.inhibit_reconnect = false; + UI.doneInitialResize = false; + + let msg; + if (UI.getSetting('encrypt')) { + msg = _("Connected (encrypted) to ") + UI.desktopName; + } else { + msg = _("Connected (unencrypted) to ") + UI.desktopName; + } + UI.showStatus(msg); + UI.updateVisualState('connected'); + + // Do this last because it can only be used on rendered elements + document.getElementById('noVNC_canvas').focus(); + }, + disconnectFinished: function (e) { + // This variable is ideally set when disconnection starts, but + // when the disconnection isn't clean or if it is initiated by + // the server, we need to do it here as well since + // UI.disconnect() won't be used in those cases. + UI.connected = false; + if (typeof e.detail.reason !== 'undefined') { UI.showStatus(e.detail.reason, 'error'); + UI.updateVisualState('disconnected'); } else if (UI.getSetting('reconnect', false) === true && !UI.inhibit_reconnect) { - document.getElementById("noVNC_transition_text").textContent = _("Reconnecting..."); - document.documentElement.classList.add("noVNC_reconnecting"); + UI.updateVisualState('reconnecting'); var delay = parseInt(UI.getSetting('reconnect_delay')); UI.reconnect_callback = setTimeout(UI.reconnect, delay); return; + } else { + UI.updateVisualState('disconnected'); + UI.showStatus(_("Disconnected"), 'normal'); } UI.openControlbar(); @@ -1085,7 +1104,8 @@ var UI = { UI.reconnect_callback = null; } - document.documentElement.classList.remove("noVNC_reconnecting"); + UI.updateVisualState('disconnected'); + UI.openControlbar(); UI.openConnectPanel(); }, diff --git a/core/rfb.js b/core/rfb.js index a95b1326..dced6b16 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -512,8 +512,6 @@ RFB.prototype = { // State change actions this._rfb_connection_state = state; - var event = new CustomEvent("updatestate", { detail: { state: state } }); - this.dispatchEvent(event); var smsg = "New state '" + state + "', was '" + oldstate + "'."; Log.Debug(smsg); @@ -528,23 +526,15 @@ RFB.prototype = { } switch (state) { - case 'disconnected': - // Fire disconnected event after updatestate event since - // we don't know if the UI only displays the latest message - if (this._rfb_disconnect_reason !== "") { - event = new CustomEvent("disconnect", - { detail: { reason: this._rfb_disconnect_reason } }); - } else { - // No reason means clean disconnect - event = new CustomEvent("disconnect", { detail: {} }); - } - this.dispatchEvent(event); - break; - case 'connecting': this._connect(); break; + case 'connected': + var event = new CustomEvent("connect", { detail: {} }); + this.dispatchEvent(event); + break; + case 'disconnecting': this._disconnect(); @@ -553,6 +543,17 @@ RFB.prototype = { this._updateConnectionState('disconnected'); }.bind(this), DISCONNECT_TIMEOUT * 1000); break; + + case 'disconnected': + if (this._rfb_disconnect_reason !== "") { + event = new CustomEvent("disconnect", + { detail: { reason: this._rfb_disconnect_reason } }); + } else { + // No reason means clean disconnect + event = new CustomEvent("disconnect", { detail: {} }); + } + this.dispatchEvent(event); + break; } }, diff --git a/docs/API.md b/docs/API.md index 7d34b07b..e194e969 100644 --- a/docs/API.md +++ b/docs/API.md @@ -63,9 +63,9 @@ protocol stream. ### Events -[`updatestate`](#updatestate) - - The `updatestate` event is fired when the connection state of the - `RFB` object changes. +[`connect`](#connect) + - The `connect` event is fired when the `RFB` object has completed + the connection and handshaking with the server. [`disconnect`](#disconnected) - The `disconnect` event is fired when the `RFB` object disconnects. @@ -177,24 +177,11 @@ connection to a specified VNC server. - A `DOMString` specifying the ID to provide to any VNC repeater encountered. -#### updatestate +#### connect -The `updatestate` event is fired after the noVNC connection state -changes. The `detail` property is an `Object` containg the property -`state` with the new connection state. - -Here is a list of the states that are reported: - -| connection state | description -| ----------------- | ------------ -| `"connecting"` | starting to connect -| `"connected"` | connected normally -| `"disconnecting"` | starting to disconnect -| `"disconnected"` | disconnected - -Note that a `RFB` objects can not transition from the disconnected -state in any way, a new instance of the object has to be created for -new connections. +The `connect` event is fired after all the handshaking with the server +is completed and the connection is fully established. After this event +the `RFB` object is ready to recieve graphics updates and to send input. #### disconnect diff --git a/tests/test.rfb.js b/tests/test.rfb.js index a3746402..2ef61240 100644 --- a/tests/test.rfb.js +++ b/tests/test.rfb.js @@ -68,11 +68,9 @@ describe('Remote Frame Buffer Protocol Client', function() { describe('#RFB', function () { it('should set the current state to "connecting"', function () { var client = new RFB(document.createElement('canvas'), 'wss://host:8675'); - var spy = sinon.spy(); - client.addEventListener("updatestate", spy); + client._rfb_connection_state = ''; this.clock.tick(); - expect(spy).to.have.been.calledOnce; - expect(spy.args[0][0].detail.state).to.equal('connecting'); + expect(client._rfb_connection_state).to.equal('connecting'); }); it('should actually connect to the websocket', function () { @@ -90,13 +88,15 @@ describe('Remote Frame Buffer Protocol Client', function() { client = make_rfb(); }); - it('should set the current state to "disconnecting"', function () { - var spy = sinon.spy(); - client.addEventListener("updatestate", spy); + it('should go to state "disconnecting" before "disconnected"', function () { + sinon.spy(client, '_updateConnectionState'); client.disconnect(); - expect(spy).to.have.been.calledTwice; - expect(spy.args[0][0].detail.state).to.equal('disconnecting'); - expect(spy.args[1][0].detail.state).to.equal('disconnected'); + expect(client._updateConnectionState).to.have.been.calledTwice; + expect(client._updateConnectionState.getCall(0).args[0]) + .to.equal('disconnecting'); + expect(client._updateConnectionState.getCall(1).args[0]) + .to.equal('disconnected'); + expect(client._rfb_connection_state).to.equal('disconnected'); }); it('should unregister error event handler', function () { @@ -320,13 +320,6 @@ describe('Remote Frame Buffer Protocol Client', function() { expect(client._disconnTimer).to.be.null; }); - it('should call the updateState callback', function () { - var spy = sinon.spy(); - client.addEventListener("updatestate", spy); - client._updateConnectionState('disconnecting'); - expect(spy.args[0][0].detail.state).to.equal('disconnecting'); - }); - it('should set the rfb_connection_state', function () { client._rfb_connection_state = 'disconnecting'; client._updateConnectionState('disconnected'); @@ -340,16 +333,23 @@ describe('Remote Frame Buffer Protocol Client', function() { }); it('should ignore state changes to the same state', function () { - var spy = sinon.spy(); - client.addEventListener("updatestate", spy); - client._rfb_connection_state = 'connecting'; - client._updateConnectionState('connecting'); - expect(spy).to.not.have.been.called; + var connectSpy = sinon.spy(); + var disconnectSpy = sinon.spy(); + client.addEventListener("connect", connectSpy); + client.addEventListener("disconnect", disconnectSpy); + + client._rfb_connection_state = 'connected'; + client._updateConnectionState('connected'); + expect(connectSpy).to.not.have.been.called; + + client._rfb_connection_state = 'disconnected'; + client._updateConnectionState('disconnected'); + expect(disconnectSpy).to.not.have.been.called; }); it('should ignore illegal state changes', function () { var spy = sinon.spy(); - client.addEventListener("updatestate", spy); + client.addEventListener("disconnect", spy); client._rfb_connection_state = 'connected'; client._updateConnectionState('disconnected'); expect(client._rfb_connection_state).to.not.equal('disconnected'); @@ -403,6 +403,39 @@ describe('Remote Frame Buffer Protocol Client', function() { }); describe('Connection States', function () { + describe('connecting', function () { + it('should open the websocket connection', function () { + var client = new RFB(document.createElement('canvas'), + 'ws://HOST:8675/PATH'); + sinon.spy(client._sock, 'open'); + this.clock.tick(); + expect(client._sock.open).to.have.been.calledOnce; + }); + }); + + describe('connected', function () { + var client; + beforeEach(function () { + client = make_rfb(); + }); + + it('should result in a connect event if state becomes connected', function () { + var spy = sinon.spy(); + client.addEventListener("connect", spy); + client._rfb_connection_state = 'connecting'; + client._updateConnectionState('connected'); + expect(spy).to.have.been.calledOnce; + }); + + it('should not result in a connect event if the state is not "connected"', function () { + var spy = sinon.spy(); + client.addEventListener("connect", spy); + client._sock._websocket.open = function () {}; // explicitly don't call onopen + client._updateConnectionState('connecting'); + expect(spy).to.not.have.been.called; + }); + }); + describe('disconnecting', function () { var client; beforeEach(function () { @@ -465,19 +498,7 @@ describe('Remote Frame Buffer Protocol Client', function() { expect(spy).to.have.been.calledOnce; expect(spy.args[0].length).to.equal(1); }); - - it('should call the updateState callback before the disconnect callback', function () { - var updateStateSpy = sinon.spy(); - var disconnectSpy = sinon.spy(); - client.addEventListener("disconnect", disconnectSpy); - client.addEventListener("updatestate", updateStateSpy); - client._rfb_connection_state = 'disconnecting'; - client._updateConnectionState('disconnected'); - expect(updateStateSpy.calledBefore(disconnectSpy)).to.be.true; - }); }); - - // NB(directxman12): Connected does *nothing* in updateConnectionState }); describe('Protocol Initialization States', function () { diff --git a/vnc_lite.html b/vnc_lite.html index e93efcfd..b904b4a5 100644 --- a/vnc_lite.html +++ b/vnc_lite.html @@ -147,45 +147,25 @@ document.getElementById('noVNC_status_bar').className = "noVNC_status_" + level; document.getElementById('noVNC_status').textContent = text; } - function updateState(e) { - var cad = document.getElementById('sendCtrlAltDelButton'); - switch (e.detail.state) { - case 'connecting': - status("Connecting", "normal"); - break; - case 'connected': - doneInitialResize = false; - if (WebUtil.getConfigVar('encrypt', - (window.location.protocol === "https:"))) { - status("Connected (encrypted) to " + - desktopName, "normal"); - } else { - status("Connected (unencrypted) to " + - desktopName, "normal"); - } - break; - case 'disconnecting': - status("Disconnecting", "normal"); - break; - case 'disconnected': - status("Disconnected", "normal"); - break; - default: - status(e.detail.state, "warn"); - break; - } - if (e.detail.state === 'connected') { - cad.disabled = false; + function connected(e) { + document.getElementById('sendCtrlAltDelButton').disabled = false; + doneInitialResize = false; + if (WebUtil.getConfigVar('encrypt', + (window.location.protocol === "https:"))) { + status("Connected (encrypted) to " + desktopName, "normal"); } else { - cad.disabled = true; - updatePowerButtons(); + status("Connected (unencrypted) to " + desktopName, "normal"); } - } - function disconnect(e) { + + function disconnected(e) { + document.getElementById('sendCtrlAltDelButton').disabled = true; + updatePowerButtons(); if (typeof(e.detail.reason) !== 'undefined') { status(e.detail.reason, "error"); + } else { + status("Disconnected", "normal"); } } @@ -246,6 +226,8 @@ (function() { + status("Connecting", "normal"); + if ((!host) || (!port)) { status('Must specify host and port in URL', 'error'); } @@ -270,8 +252,8 @@ shared: WebUtil.getConfigVar('shared', true), credentials: { password: password } }); rfb.viewOnly = WebUtil.getConfigVar('view_only', false); - rfb.addEventListener("updatestate", updateState); - rfb.addEventListener("disconnect", disconnect); + rfb.addEventListener("connect", connected); + rfb.addEventListener("disconnect", disconnected); rfb.addEventListener("capabilities", function () { updatePowerButtons(); initialResize(); }); rfb.addEventListener("credentialsrequired", credentials); rfb.addEventListener("desktopname", updateDesktopName);