Skip to content

Docker: Fluxbox not rendering Chinese characters via VNC view #2817

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

Merged
merged 2 commits into from
May 3, 2025

Conversation

VietND96
Copy link
Member

@VietND96 VietND96 commented May 2, 2025

Thanks for contributing to the Docker-Selenium project!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines, applied for this repository.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Motivation and Context

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link

qodo-merge-pro bot commented May 2, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅

2509 - Fully compliant

Compliant requirements:

• Fix Chinese error messages displayed by Chrome browser in noVNC
• Ensure proper rendering of Chinese characters in the browser when viewed through noVNC

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Font Modification

The PR modifies the Fluxbox style to use WenQuanYi Zen Hei font instead of Ubuntu font. This change should be validated to ensure it doesn't negatively impact other language displays while fixing Chinese character rendering.

# For Fluxbox style, use fonts-wqy-zenhei which has a large international language coverage
&& sed -i 's/Ubuntu-/WenQuanYi Zen Hei-/g' /usr/share/fluxbox/styles/ubuntu-light \

Copy link

qodo-merge-pro bot commented May 2, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Security
Fix security vulnerability

Setting FB_NOAUTH=true disables authentication for the file browser, creating a
security risk as anyone can access the video recordings. Consider enabling
authentication or restricting network access to this service.

tests/docker-compose-v3-get-started-arm64.yml [49-59]

 # File browser to manage the videos from local volume
 file_browser:
   image: filebrowser/filebrowser:latest
   container_name: file_browser
   restart: always
   ports:
-    - "8081:80"
+    - "127.0.0.1:8081:80"  # Only accessible from localhost
   volumes:
     - /tmp/videos:/srv
-  environment:
-    - FB_NOAUTH=true
+  # Remove FB_NOAUTH=true to enable default authentication
  • Apply this suggestion
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that FB_NOAUTH=true disables authentication, which is a security concern. Although this is an example file likely intended for local use, promoting secure defaults by removing this flag or restricting access (like binding to localhost as suggested in the improved_code) is a valuable improvement.

Medium
General
Reduce excessive wait time

The hardcoded sleep of 100 seconds is excessive and could cause tests to run
unnecessarily long. Consider using a shorter timeout or implementing a more
dynamic wait mechanism that exits once the required operation completes.

tests/get_started.py [36]

-time.sleep(100)
+time.sleep(10)  # Shorter wait time for demonstration purposes
  • Apply this suggestion
Suggestion importance[1-10]: 4

__

Why: The suggestion correctly identifies a potentially long wait time in a demonstration script. While not critical, reducing the sleep duration improves the script's efficiency for quick checks, although the long wait might be intentional for manual observation.

Low
  • Update

Copy link

qodo-merge-pro bot commented May 2, 2025

CI Feedback 🧐

(Feedback updated until commit 37554a2)

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: Rerun workflow when failure

Failed stage: Authenticate GitHub CLI for PR [❌]

Failure summary:

The action failed because the GitHub CLI authentication failed with the error: "missing required
scope 'read:org'". The token provided in the GH_CLI_TOKEN_PR environment variable does not have the
necessary permissions to perform the required operations. Specifically, it's missing the 'read:org'
scope which is needed for the GitHub CLI to authenticate properly.

Relevant error logs:
1:  ##[group]Operating System
2:  Ubuntu
...

22:  Issues: write
23:  Metadata: read
24:  Models: read
25:  Packages: write
26:  Pages: write
27:  PullRequests: write
28:  RepositoryProjects: write
29:  SecurityEvents: write
30:  Statuses: write
31:  ##[endgroup]
32:  Secret source: Actions
33:  Prepare workflow directory
34:  Prepare all required actions
35:  Getting action download info
36:  Download action repository 'actions/checkout@main' (SHA:85e6279cec87321a52edac9c87bce653a07cf6c2)
37:  Complete job name: Rerun workflow when failure
38:  ##[group]Run actions/checkout@main
...

42:  ssh-strict: true
43:  ssh-user: git
44:  persist-credentials: true
45:  clean: true
46:  sparse-checkout-cone-mode: true
47:  fetch-depth: 1
48:  fetch-tags: false
49:  show-progress: true
50:  lfs: false
51:  submodules: false
52:  set-safe-directory: true
53:  env:
54:  GH_CLI_TOKEN: ***
55:  GH_CLI_TOKEN_PR: ***
56:  RUN_ID: 14804617613
57:  RERUN_FAILED_ONLY: true
58:  RUN_ATTEMPT: 1
...

113:  Or undo this operation with:
114:  git switch -
115:  Turn off this advice by setting config variable advice.detachedHead to false
116:  HEAD is now at 5a9d1dd Merge 37554a277eaedc8f2cb595f38fb05886e8fafffa into 78ff7f681ceb9f3092f62d57529f04475e776a83
117:  ##[endgroup]
118:  [command]/usr/bin/git log -1 --format=%H
119:  5a9d1dda35dc93183a15a0d26051733a0d36c54a
120:  ##[group]Run sudo apt update
121:  �[36;1msudo apt update�[0m
122:  �[36;1msudo apt install gh�[0m
123:  shell: /usr/bin/bash -e {0}
124:  env:
125:  GH_CLI_TOKEN: ***
126:  GH_CLI_TOKEN_PR: ***
127:  RUN_ID: 14804617613
128:  RERUN_FAILED_ONLY: true
129:  RUN_ATTEMPT: 1
...

165:  Reading state information...
166:  40 packages can be upgraded. Run 'apt list --upgradable' to see them.
167:  WARNING: apt does not have a stable CLI interface. Use with caution in scripts.
168:  Reading package lists...
169:  Building dependency tree...
170:  Reading state information...
171:  gh is already the newest version (2.71.2).
172:  0 upgraded, 0 newly installed, 0 to remove and 40 not upgraded.
173:  ##[group]Run echo "$GH_CLI_TOKEN_PR" | gh auth login --with-token
174:  �[36;1mecho "$GH_CLI_TOKEN_PR" | gh auth login --with-token�[0m
175:  shell: /usr/bin/bash -e {0}
176:  env:
177:  GH_CLI_TOKEN: ***
178:  GH_CLI_TOKEN_PR: ***
179:  RUN_ID: 14804617613
180:  RERUN_FAILED_ONLY: true
181:  RUN_ATTEMPT: 1
182:  ##[endgroup]
183:  error validating token: missing required scope 'read:org'
184:  ##[error]Process completed with exit code 1.
185:  Post job cleanup.

@VietND96 VietND96 force-pushed the novnc-font branch 2 times, most recently from 9269020 to fd8781a Compare May 2, 2025 22:44
@VietND96 VietND96 merged commit 03b34b6 into trunk May 3, 2025
6 of 28 checks passed
@VietND96 VietND96 deleted the novnc-font branch May 3, 2025 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[🐛 Bug]: Chinese error displayed by Chrome browser in noVNC
1 participant