jcarr
/
noVNC
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #1617 from novnc/resizeObserverScrollbars 

Fix issue with ResizeObservers and scrollbars
This commit is contained in:
Samuel Mannehed 2021-12-15 16:01:36 +01:00 committed by GitHub
parent 78eda3c040 0ff0844a14
commit 679b45fa3b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 159 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
core/rfb.js
View File

@ -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);
@ -703,12 +731,13 @@ export default class RFB extends EventTargetMixin {
}
_fixScrollbars() {
// This is a hack because Chrome screws up the calculation
// for when scrollbars are needed. So to fix it we temporarily
// toggle them off and on.
// This is a hack because Safari on macOS screws up the calculation
// for when scrollbars are needed. We get scrollbars when making the
// browser smaller, despite remote resize being enabled. So to fix it
// we temporarily toggle them off and on.
const orig = this._screen.style.overflow;
this._screen.style.overflow = 'hidden';
// Force Chrome to recalculate the layout by asking for
// Force Safari to recalculate the layout by asking for
// an element's dimensions
this._screen.getBoundingClientRect();
this._screen.style.overflow = orig;
@ -2506,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) {

150
tests/test.rfb.js
View File

@ -74,6 +74,9 @@ describe('Remote Frame Buffer Protocol Client', function () {
let fakeResizeObserver = null;
const realObserver = window.ResizeObserver;
// Since we are using fake timers we don't actually want
// to wait for the browser to observe the size change,
// that's why we use a fake ResizeObserver
class FakeResizeObserver {
constructor(handler) {
this.fire = handler;
@ -520,7 +523,7 @@ describe('Remote Frame Buffer Protocol Client', function () {
container.style.width = '40px';
container.style.height = '50px';
fakeResizeObserver.fire();
clock.tick();
clock.tick(1000);
expect(client._display.viewportChangeSize).to.have.been.calledOnce;
expect(client._display.viewportChangeSize).to.have.been.calledWith(40, 50);
@ -537,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.
@ -550,9 +557,8 @@ describe('Remote Frame Buffer Protocol Client', function () {
container.style.width = '40px';
container.style.height = '50px';
const event = new UIEvent('resize');
window.dispatchEvent(event);
clock.tick();
fakeResizeObserver.fire();
clock.tick(1000);
expect(client._display.viewportChangeSize).to.not.have.been.called;
});
@ -563,13 +569,38 @@ describe('Remote Frame Buffer Protocol Client', function () {
container.style.width = '40px';
container.style.height = '50px';
const event = new UIEvent('resize');
window.dispatchEvent(event);
clock.tick();
fakeResizeObserver.fire();
clock.tick(1000);
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;
@ -716,7 +747,7 @@ describe('Remote Frame Buffer Protocol Client', function () {
container.style.width = '40px';
container.style.height = '50px';
fakeResizeObserver.fire();
clock.tick();
clock.tick(1000);
expect(client._display.autoscale).to.have.been.calledOnce;
expect(client._display.autoscale).to.have.been.calledWith(40, 50);
@ -733,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);
@ -745,9 +780,8 @@ describe('Remote Frame Buffer Protocol Client', function () {
container.style.width = '40px';
container.style.height = '50px';
const event = new UIEvent('resize');
window.dispatchEvent(event);
clock.tick();
fakeResizeObserver.fire();
clock.tick(1000);
expect(client._display.autoscale).to.not.have.been.called;
});
@ -777,20 +811,39 @@ describe('Remote Frame Buffer Protocol Client', function () {
it('should request a resize when initially connecting', function () {
// Simple ExtendedDesktopSize FBU message
const incoming = [ 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00,
0x00, 0x04, 0x00, 0x04, 0xff, 0xff, 0xfe, 0xcc,
0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x04,
0x00, 0x00, 0x00, 0x00 ];
const incoming = [ 0x00, // msg-type=FBU
0x00, // padding
0x00, 0x01, // number of rects = 1
0x00, 0x00, // reason = server initialized
0x00, 0x00, // status = no error
0x00, 0x04, // new width = 4
0x00, 0x04, // new height = 4
0xff, 0xff,
0xfe, 0xcc, // enc = (-308) ExtendedDesktopSize
0x01, // number of screens = 1
0x00, 0x00,
0x00, // padding
0x00, 0x00,
0x00, 0x00, // screen id = 0
0x00, 0x00, // screen x = 0
0x00, 0x00, // screen y = 0
0x00, 0x04, // screen width = 4
0x00, 0x04, // screen height = 4
0x00, 0x00,
0x00, 0x00]; // screen flags
// This property is indirectly used as a marker for the first update
client._supportsSetDesktopSize = false;
// First message should trigger a resize
client._supportsSetDesktopSize = false;
client._sock._websocket._receiveData(new Uint8Array(incoming));
// It should match the current size of the container,
// not the reported size from the server
expect(RFB.messages.setDesktopSize).to.have.been.calledOnce;
expect(RFB.messages.setDesktopSize).to.have.been.calledWith(sinon.match.object, 70, 80, 0, 0);
expect(RFB.messages.setDesktopSize).to.have.been.calledWith(
sinon.match.object, 70, 80, 0, 0);
RFB.messages.setDesktopSize.resetHistory();
@ -811,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';
@ -837,8 +919,7 @@ describe('Remote Frame Buffer Protocol Client', function () {
container.style.width = '40px';
container.style.height = '50px';
const event = new UIEvent('resize');
window.dispatchEvent(event);
fakeResizeObserver.fire();
clock.tick(1000);
expect(RFB.messages.setDesktopSize).to.not.have.been.called;
@ -849,8 +930,7 @@ describe('Remote Frame Buffer Protocol Client', function () {
container.style.width = '40px';
container.style.height = '50px';
const event = new UIEvent('resize');
window.dispatchEvent(event);
fakeResizeObserver.fire();
clock.tick(1000);
expect(RFB.messages.setDesktopSize).to.not.have.been.called;
@ -861,24 +941,40 @@ describe('Remote Frame Buffer Protocol Client', function () {
container.style.width = '40px';
container.style.height = '50px';
const event = new UIEvent('resize');
window.dispatchEvent(event);
fakeResizeObserver.fire();
clock.tick(1000);
expect(RFB.messages.setDesktopSize).to.not.have.been.called;
});
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);
});
});