-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Upgrade noVNC from 1.4.0 to 1.6.0 #11119
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
Conversation
|
@blueorangutan package |
|
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #11119 +/- ##
============================================
- Coverage 16.58% 16.58% -0.01%
Complexity 13991 13991
============================================
Files 5745 5745
Lines 510757 510757
Branches 62144 62144
============================================
- Hits 84690 84689 -1
- Misses 416598 416599 +1
Partials 9469 9469
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13981 |
|
@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
UI build: ✔️ |
|
@blueorangutan test keepEnv |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-13664)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Upgrades noVNC from version 1.4.0 to 1.6.0, replacing low-level send/receive calls with buffered queue methods and adding modern codec support, along with UI embedding improvements and dependency bumps.
- Bumped noVNC version to 1.6.0 and added empty
defaults.json/mandatory.jsonstubs - Replaced direct
.send/.sendStringcalls withsQpush*and.flush()in Websock and RFB - Introduced Zlib and H264 decoding support, refactored UI initialization in
vnc.html
Reviewed Changes
Copilot reviewed 73 out of 73 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| systemvm/agent/noVNC/package.json | Bump version to 1.6.0, update devDependencies |
| systemvm/agent/noVNC/defaults.json | Add empty defaults placeholder |
| systemvm/agent/noVNC/mandatory.json | Add empty mandatory placeholder |
| systemvm/agent/noVNC/vnc_lite.html | Minor text casing/style tweaks |
| systemvm/agent/noVNC/vnc.html | Refactor UI startup to dynamic imports and fetch settings |
| systemvm/agent/noVNC/utils/convert.js | Use destructured program import from commander |
| systemvm/agent/noVNC/core/websock.js | Migrate to sQpush*/flush API for send queue buffering |
| systemvm/agent/noVNC/core/rfb.js | Enhanced resize logic, new Zlib/H264 encodings, pointer events |
| systemvm/agent/noVNC/core/decoders/*.js | Add ZlibDecoder and H264Decoder, update existing decoders |
| systemvm/agent/noVNC/core/util/*.js | Standardize copyright casing, correct utility methods |
| systemvm/agent/noVNC/docs/novnc_proxy.1 | Update --listen synopsis to support [HOST:]PORT syntax |
Comments suppressed due to low confidence (2)
systemvm/agent/noVNC/core/decoders/h264.js:1
- [nitpick] The new H264 decoder logic is substantial but lacks dedicated unit tests; consider adding tests to validate parsing, key-frame handling, and decoder configuration.
/*
systemvm/agent/noVNC/core/rfb.js:2560
- [nitpick]
replaceAllis not supported in some older browsers; consider usingtextData = textData.replace(/\r\n/g, "\n");for wider compatibility.
textData = textData.replaceAll("\r\n", "\n");
|
Changes doesn't seem to be working and require some investigation. |
|
@weizhouapache @nvazquez let's also review if this creates any regression for keystore/keyboard related changes we had done. |
95ceffe to
895a84a
Compare
895a84a to
a4cbfde
Compare
|
@blueorangutan package |
|
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14184 |
|
@rohityadavcloud a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
looks good, will do some extra testing |
|
I've public rpm repo here for anyone wanting to try/test this https://build.yadav.cloud/cloudstack/pr/11119/el8/4.21/ |
|
Tested noVNC 1.6.0 by manually applying the file changes to a ACS 4.20.1 env CPVM and restarting cloud service. Post this, the noVNC 1.6 based console appeared slightly faster: (with VNC/TLS disabled) novnc-1.6-upgrade.movAlso tested the same env with noVNC 1.6 based console & VNC/TLS enabled: novnc-1.6-with-tls.mov |
|
I think we should go ahead with this PR but,
If I don't touch i can reverse the shift-lock by closing the connection and creating a new one. The rest of the console will keep working but shiftlock will be reversed. |
|
[SF] Trillian test result (tid-13772)
|
DaanHoogland
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clgtm
|
@nvazquez @weizhouapache can you please review? |
|
@blueorangutan package |
|
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✖️ el8 ✖️ el9 ✔️ debian ✖️ suse15. SL-JID 14420 |
|
@blueorangutan package |
|
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14423 |
|
@blueorangutan test |
|
@borisstoyanov a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
borisstoyanov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, manually checked it could not find any regressions
|
@vishesh92 @borisstoyanov @sureshanaparti , I marked this |
|
[SF] Trillian test result (tid-13955)
|
Description
This PR upgrades noVNC from 1.4.0 to 1.6.0. Followed the same steps as in #7281. This should fix issues like #9940
noVNC release notes: https://github.com/novnc/noVNC/releases
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
How Has This Been Tested?
How did you try to break this feature and the system with this change?