Skip to content

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

Merged
merged 1 commit into from
Feb 16, 2023

Conversation

adrianhj
Copy link
Contributor

@adrianhj adrianhj commented Feb 13, 2023

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 to LANGUAGE at runtime, not build-time.

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.

@CLAassistant
Copy link

CLAassistant commented Feb 13, 2023

CLA assistant check
All committers have signed the CLA.

@adrianhj
Copy link
Contributor Author

Output of the generated script after make chrome:

$ 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 "$@"

@jamesmortensen
Copy link
Member

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:

docker run --rm -it -p 4444:4444 -p 7900:7900 --shm-size 2g -e LANGUAGE=zh_CN.UTF-8 selenium/standalone-chrome:latest

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! :)

@adrianhj
Copy link
Contributor Author

adrianhj commented Feb 15, 2023

@jamesmortensen LANGUAGE is indeed the correct way as per How to set your browser's locale > Linux, and is also what we were relying on and what seems to have regressed.

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 --lang flag that is used by Windows/OSX to change locale to the LANGUAGE environment variable that is used by Linux (see other sections in earlier link).

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 4.8.0-20230123, the version prior to #1778 being released, where this behaviour works fine using your example for zh_CN:

  1. Launch selenium/standalone-chrome:4.8.0-20230123 with LANGUAGE set to zh_CN.UTF-8
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
  1. Accessing the image via noVNC on :7900, launch a shell and validate that the language is as expected via env | grep -i language:

  2. From the same shell, launch via google-chrome and inspect the language of Google Chrome's UI + navigator.language in the JS console

Output from above steps:

Screenshot 2023-02-15 at 09 52 04

Now repeat the above but with selenium/standalone-chrome:latest, below now shows this having broken:

image

The issue stems from the wrapper generation scripts that generated the wrapped google-chrome entries having been incorrectly implemented. You can see this by issuing e.g. cat $(which google-chrome) to review the script that was generated:

  • On 4.8.0-20230123:
$ 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 "$@"
  • On latest:
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 make standalone_chrome, the generated google-chrome wrapper script in the produced image is as per my earlier comment, and will now use LANGUAGE from its outer environment if no --lang is passed:

Screenshot 2023-02-15 at 10 08 50

Language can also be overridden via --lang, which I believe was the original intention of #1778:

Screenshot 2023-02-15 at 10 10 49

If there is any more information that would be of use please let me know.

@jamesmortensen
Copy link
Member

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.)

@jamesmortensen
Copy link
Member

I validated that this is working using the following examples:

First, I built and started the container image:

git pull https://github.com/adrianhj/docker-selenium.git
cd docker-selenium && make standalone_chrome
docker run -it -p 4444:4444 -p 7900:7900 selenium/standalone-chrome:4.5.0-20230216

Then I ran a test Python script (using a shell script to create a virtualenv):

run.sh:

#!/usr/bin/env bash
# cd tests || true

if [ "${CI:-false}" = "false" ]; then
  pip3 install virtualenv | grep -v 'Requirement already satisfied'
  virtualenv selenium-webdriver-demo-python
  source selenium-webdriver-demo-python/bin/activate
fi

python -m pip install selenium==4.3.0 \
                      | grep -v 'Requirement already satisfied'

python remote-demo.py
ret_code=$?

if [ "${CI:-false}" = "false" ]; then
  deactivate
fi

exit $ret_code

remote-demo.py:

from selenium import webdriver
import time

chrome_options = webdriver.ChromeOptions()
#chrome_options.add_argument('--lang=zh_CN.UTF-8')
chrome_options.add_argument('--lang=es_US.UTF-8')
driver = webdriver.Remote(command_executor='http://localhost:4444', options=chrome_options)
driver.maximize_window()
driver.get("http://www.google.com")
time.sleep(5)
driver.quit()

I executed with both Chinese and Spanish, and both times I visually validated that the language changed in the browser.

Screenshot 2023-02-16 at 10 52 49 AM

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:

docker run -it -p 4444:4444 -p 7900:7900 -e LANGUAGE=fr_CA.UTF-8 selenium/standalone-chrome:20230216

Then I ran the same python code, but without setting the language in the code. The language was still French.

Screenshot 2023-02-16 at 10 50 47 AM

Additionally, if I set the language in the code as Chinese, then it overrides the default as expected.

Screenshot 2023-02-16 at 10 50 23 AM

@jamesmortensen jamesmortensen merged commit c6df1ab into SeleniumHQ:trunk Feb 16, 2023
@jamesmortensen
Copy link
Member

Thank you! @adrianhj

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.

3 participants