-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: ensure language is not hardcoded in generated wrapper scripts #1789
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
Output of the generated script after $ docker run -it --rm selenium/node-chrome:4.1.0-20230213 cat /usr/bin/google-chrome
#!/bin/bash
# umask 002 ensures default permissions of files are 664 (rw-rw-r--) and directories are 775 (rwxrwxr-x).
umask 002
# Debian/Ubuntu seems to not respect --lang, it instead needs to be a LANGUAGE environment var
# See: https://stackoverflow.com/a/41893197/359999
for var in "$@"; do
if [[ $var == --lang=* ]]; then
LANGUAGE=${var//--lang=}
fi
done
# Set language environment variable
export LANGUAGE="$LANGUAGE"
# Note: exec -a below is a bashism.
exec -a "$0" "/opt/google/chrome/google-chrome-base" --no-sandbox "$@" |
Hi @adrianhj Thanks for the PR. I'm attempting to review the changes, but I'm not sure how to validate them... Can you help me understand the difference between what we have now and what the changes do? Here's some context: Back in September, someone had asked how to change language at run-time: #1671 (comment) For example, I could do this to have the browser show text in Chinese:
Would you please be able to help me understand what the script changes are doing and how I can validate the difference between the current revision and the changes, perhaps with an example with --lang? Thank you! :) |
@jamesmortensen This broke with 4.8.0-20230131 due to #1778 (original issue #1750). There is not a huge amount of information on the original #1750 issue, but it seems that the outcome in #1778 was to provide some behaviour that attempts to adapt the Whether this is the right design choice or not for selenium-standalone I'm not sure, the original issue author and reviewer may be better off to elaborate here (SeleniumHQ/selenium#11629 involving same pair seem to be opened since attempting another fix elsewhere with commentary suggesting the one in #1778 was unsuccesful). In terms of the issue I raised in #1788, this can be validated by first running
docker run --rm -it -p 4444:4444 -p 7900:7900 --shm-size 2g -e LANGUAGE=zh_CN.UTF-8 selenium/standalone-chrome:4.8.0-20230123
Output from above steps: Now repeat the above but with The issue stems from the wrapper generation scripts that generated the wrapped
$ seluser@43848d76539d:/$ cat $(which google-chrome)
#!/bin/bash
# umask 002 ensures default permissions of files are 664 (rw-rw-r--) and directories are 775 (rwxrwxr-x).
umask 002
# Note: exec -a below is a bashism.
exec -a "$0" "/opt/google/chrome/google-chrome-base" --no-sandbox "$@"
seluser@d89cc1f17477:/$ cat $(which google-chrome)
#!/bin/bash
# umask 002 ensures default permissions of files are 664 (rw-rw-r--) and directories are 775 (rwxrwxr-x).
umask 002
# Set language environment variable
export LANGUAGE="en_US.UTF-8"
# Note: exec -a below is a bashism.
exec -a "$0" "/opt/google/chrome/google-chrome-base" --no-sandbox "$@" Language is now always hardcoded to en_US.UTF8 as an outcome of #1778. In terms of validating this PR, after applying this PR and building via Language can also be overridden via If there is any more information that would be of use please let me know. |
Thanks so much for the details. I'll try over the weekend or in the evening with the latest so I can reproduce. (my "latest" was about 4 months old.) |
I validated that this is working using the following examples: First, I built and started the container image:
Then I ran a test Python script (using a shell script to create a virtualenv): run.sh:
remote-demo.py:
I executed with both Chinese and Spanish, and both times I visually validated that the language changed in the browser. Additionally, I also validated that the existing behavior, setting the language by default using the LANGUAGE environment variable, from September, still works as expected. I used French as the default:
Then I ran the same python code, but without setting the language in the code. The language was still French. Additionally, if I set the language in the code as Chinese, then it overrides the default as expected. |
Thank you! @adrianhj |
Description
Ensures LANGUAGE is not hardcoded in standalone images by moving logic into the generated wrapper script.
Motivation and Context
See #1788 for related issue.
I have interpreted the original intention in #1778 to attempt to translate
--lang
toLANGUAGE
at runtime, not build-time.Types of changes
Checklist