From 0ff0844a142c086128cce8771a3dad21a499f02b Mon Sep 17 00:00:00 2001 From: Samuel Mannehed Date: Fri, 10 Dec 2021 20:11:05 +0100 Subject: Ignore resize observation caused by server resizes If we increase the remote screen size from the server in such a way that it no longer fits the browser window, the browser will probably want to show scrollbars. The same happens if you enable 'clipping' while the remote is larger than the browser window. These scrollbars do, in turn, decrease the available space in the browser window. This causes our ResizeObserver to trigger. If the resize observation triggers a requestRemoteResize() we will overwrite the size and request a new one just because scrollbars have appeared. We don't want that. We can save the expected client size after resizing, and then compare the current client size with the expected one. If there is no change compared to the expected size, we shouldn't send the request. Fixes issue #1616. --- core/rfb.js | 31 ++++++++++++++++++++ tests/test.rfb.js | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 113 insertions(+), 2 deletions(-) diff --git a/core/rfb.js b/core/rfb.js index fe9939f..970e03f 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -244,6 +244,8 @@ export default class RFB extends EventTargetMixin { this._sock.on('message', this._handleMessage.bind(this)); this._sock.on('error', this._socketError.bind(this)); + this._expectedClientWidth = null; + this._expectedClientHeight = null; this._resizeObserver = new ResizeObserver(this._eventHandlers.handleResize); // All prepared, kick off the connection @@ -623,7 +625,26 @@ export default class RFB extends EventTargetMixin { { detail: { name: this._fbName } })); } + _saveExpectedClientSize() { + this._expectedClientWidth = this._screen.clientWidth; + this._expectedClientHeight = this._screen.clientHeight; + } + + _currentClientSize() { + return [this._screen.clientWidth, this._screen.clientHeight]; + } + + _clientHasExpectedSize() { + const [currentWidth, currentHeight] = this._currentClientSize(); + return currentWidth == this._expectedClientWidth && + currentHeight == this._expectedClientHeight; + } + _handleResize() { + // Don't change anything if the client size is already as expected + if (this._clientHasExpectedSize()) { + return; + } // If the window resized then our screen element might have // as well. Update the viewport dimensions. window.requestAnimationFrame(() => { @@ -664,6 +685,12 @@ export default class RFB extends EventTargetMixin { this._display.viewportChangeSize(size.w, size.h); this._fixScrollbars(); } + + // When changing clipping we might show or hide scrollbars. + // This causes the expected client dimensions to change. + if (curClip !== newClip) { + this._saveExpectedClientSize(); + } } _updateScale() { @@ -688,6 +715,7 @@ export default class RFB extends EventTargetMixin { } const size = this._screenSize(); + RFB.messages.setDesktopSize(this._sock, Math.floor(size.w), Math.floor(size.h), this._screenID, this._screenFlags); @@ -2507,6 +2535,9 @@ export default class RFB extends EventTargetMixin { this._updateScale(); this._updateContinuousUpdates(); + + // Keep this size until browser client size changes + this._saveExpectedClientSize(); } _xvpOp(ver, op) { diff --git a/tests/test.rfb.js b/tests/test.rfb.js index 1d806e7..c3aa74e 100644 --- a/tests/test.rfb.js +++ b/tests/test.rfb.js @@ -540,6 +540,10 @@ describe('Remote Frame Buffer Protocol Client', function () { sinon.spy(client._display, "viewportChangeSize"); client._sock._websocket._receiveData(new Uint8Array(incoming)); + // The resize will cause scrollbars on the container, this causes a + // resize observation in the browsers + fakeResizeObserver.fire(); + clock.tick(1000); // FIXME: Display implicitly calls viewportChangeSize() when // resizing the framebuffer, hence calledTwice. @@ -571,6 +575,32 @@ describe('Remote Frame Buffer Protocol Client', function () { expect(client._display.viewportChangeSize).to.not.have.been.called; }); + describe('Clipping and remote resize', function () { + beforeEach(function () { + // Given a remote (100, 100) larger than the container (70x80), + client._resize(100, 100); + client._supportsSetDesktopSize = true; + client.resizeSession = true; + sinon.spy(RFB.messages, "setDesktopSize"); + }); + afterEach(function () { + RFB.messages.setDesktopSize.restore(); + }); + it('should not change remote size when changing clipping', function () { + // When changing clipping the scrollbars of the container + // will appear and disappear and thus trigger resize observations + client.clipViewport = false; + fakeResizeObserver.fire(); + clock.tick(1000); + client.clipViewport = true; + fakeResizeObserver.fire(); + clock.tick(1000); + + // Then no resize requests should be sent + expect(RFB.messages.setDesktopSize).to.not.have.been.called; + }); + }); + describe('Dragging', function () { beforeEach(function () { client.dragViewport = true; @@ -734,6 +764,10 @@ describe('Remote Frame Buffer Protocol Client', function () { sinon.spy(client._display, "autoscale"); client._sock._websocket._receiveData(new Uint8Array(incoming)); + // The resize will cause scrollbars on the container, this causes a + // resize observation in the browsers + fakeResizeObserver.fire(); + clock.tick(1000); expect(client._display.autoscale).to.have.been.calledOnce; expect(client._display.autoscale).to.have.been.calledWith(70, 80); @@ -830,6 +864,35 @@ describe('Remote Frame Buffer Protocol Client', function () { expect(RFB.messages.setDesktopSize).to.have.been.calledWith(sinon.match.object, 40, 50, 0, 0); }); + it('should not request the same size twice', function () { + container.style.width = '40px'; + container.style.height = '50px'; + fakeResizeObserver.fire(); + clock.tick(1000); + + expect(RFB.messages.setDesktopSize).to.have.been.calledOnce; + expect(RFB.messages.setDesktopSize).to.have.been.calledWith( + sinon.match.object, 40, 50, 0, 0); + + // Server responds with the requested size 40x50 + const incoming = [ 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, + 0x00, 0x28, 0x00, 0x32, 0xff, 0xff, 0xfe, 0xcc, + 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x28, 0x00, 0x32, + 0x00, 0x00, 0x00, 0x00]; + + client._sock._websocket._receiveData(new Uint8Array(incoming)); + clock.tick(1000); + + RFB.messages.setDesktopSize.resetHistory(); + + // size is still 40x50 + fakeResizeObserver.fire(); + clock.tick(1000); + + expect(RFB.messages.setDesktopSize).to.not.have.been.called; + }); + it('should not resize until the container size is stable', function () { container.style.width = '20px'; container.style.height = '30px'; @@ -885,16 +948,33 @@ describe('Remote Frame Buffer Protocol Client', function () { }); it('should not try to override a server resize', function () { - // Simple ExtendedDesktopSize FBU message + // Simple ExtendedDesktopSize FBU message, new size: 100x100 const incoming = [ 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x04, 0x00, 0x04, 0xff, 0xff, 0xfe, 0xcc, + 0x00, 0x64, 0x00, 0x64, 0xff, 0xff, 0xfe, 0xcc, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00 ]; + // Note that this will cause the browser to display scrollbars + // since the framebuffer is 100x100 and the container is 70x80. + // The usable space (clientWidth/clientHeight) will be even smaller + // due to the scrollbars taking up space. client._sock._websocket._receiveData(new Uint8Array(incoming)); + // The scrollbars cause the ResizeObserver to fire + fakeResizeObserver.fire(); + clock.tick(1000); expect(RFB.messages.setDesktopSize).to.not.have.been.called; + + // An actual size change must not be ignored afterwards + container.style.width = '120px'; + container.style.height = '130px'; + fakeResizeObserver.fire(); + clock.tick(1000); + + expect(RFB.messages.setDesktopSize).to.have.been.calledOnce; + expect(RFB.messages.setDesktopSize.firstCall.args[1]).to.equal(120); + expect(RFB.messages.setDesktopSize.firstCall.args[2]).to.equal(130); }); }); -- cgit v1.2.1