Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Save new expected client size on resize #1801

Closed
wants to merge 2 commits into from
Closed

Conversation

und3f
Copy link

@und3f und3f commented Aug 26, 2023

Hello,

_handleResize() doesn't save new expected size if window was resized which cause a bug if window was resized to different size, and then return back to original (e.g. go fullscreen and back to windowed).
Since the expected client size wasn't changed during fullscreen resize, the _handleResize would not resize window during resize to original size as it would exit because of this._clientHasExpectedSize().

The PR add save of new expected client size to fix the issue

@und3f
Copy link
Author

und3f commented Aug 30, 2023

Windows test failed because of windows machine itself, please rerun if needed

@samhed
Copy link
Member

samhed commented Sep 1, 2023

Thanks for your contribution, good to see that you included a unit test. Could you please describe the issue you saw before applying your fix?

@samhed
Copy link
Member

samhed commented Sep 1, 2023

Resize back and forth between full screen and regular seems to work fine for me. What am I looking for?

@und3f
Copy link
Author

und3f commented Sep 7, 2023

Resize back and forth between full screen and regular seems to work fine for me. What am I looking for?

Hi, I've rechecked functionality with vnc.html and vnc_lite.html files, and it seems that resize works correctly in those.
I am working on reproducible sample, I suspect that it depends on VNC container element display or other parameters.

What I've noticed: there are two resize handlers which are executed separately: _resize and _handleResize the main difference is that _resize saves new ExpectedClientSize and _handleResize doesn't. For both vnc.html and vnc_lite.html the _resize is called on each container element resize, but in my code only _handleResize is called so the element is not resized after full screen.

@samhed
Copy link
Member

samhed commented Sep 7, 2023

The two functions have very different uses. _handleResize() is called when the browser window resizes, and _resize() is called when noVNC receives a "DesktopSize" or "ExtendedDesktopSize" message from the VNC server.

The fact that _resize() isn't called in your case indicates that you didn't receive the triggering messages. Are you using a different VNC server when testing vnc.html?

@und3f
Copy link
Author

und3f commented Sep 7, 2023

I am using same VNC server (qemu's VNC with websocket) for all tests.

Than, it is kinda weird that _resize() is executed on html element resize. _resize is calling _saveExpectedClientSize which is why vnc.html doesn't have bug with resizing back to original size).

Is it correct to assume that my test case correctly emulates expected real world RFB behavior for resizing?

@samhed
Copy link
Member

samhed commented Sep 12, 2023

Is it correct to assume that my test case correctly emulates expected real world RFB behavior for resizing?

I'm not sure what you mean. I don't know the details of your test case. You said you were working on a reproducible sample, do you have something like that?

@und3f
Copy link
Author

und3f commented Apr 22, 2024

Sorry I got busy and wasn't able to create clean example. But I've tried latest noVnc and it has the issue fixed, so we can close it

@und3f und3f closed this Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants