Skip to content

[py] Use XWayland for internal Python Firefox tests #15601

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 12 commits into from
Apr 10, 2025

Conversation

cgoldberg
Copy link
Contributor

@cgoldberg cgoldberg commented Apr 9, 2025

User description

🔗 Related Issues

Fixes #15584

💥 What does this PR do?

There are several issues related to setting window size/position when running Firefox under Wayland display server (Linux). This PR sets the MOZ_ENABLE_WAYLAND=0 environment variable in our PyTest configuration, so Firefox uses XWayland instead.

(This should fix several CI failures)

🔄 Types of changes

  • Bug fix (backwards compatible)

PR Type

Bug fix


Description

  • Set MOZ_ENABLE_WAYLAND=0 for Firefox in tests to use XWayland.

  • Fixes issues with window size/position under Wayland on Linux.

  • Resolves CI failures related to Firefox on Wayland.


Changes walkthrough 📝

Relevant files
Bug fix
conftest.py
Configure Firefox to use XWayland in tests                             

py/conftest.py

  • Added environment variable MOZ_ENABLE_WAYLAND=0 for Firefox.
  • Ensures Firefox uses XWayland instead of Wayland.
  • Fixes issues with window size/position under Wayland.
  • +3/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • Copy link
    Contributor

    qodo-merge-pro bot commented Apr 9, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No major issues detected

    @selenium-ci selenium-ci added the C-py Python Bindings label Apr 9, 2025
    Copy link
    Contributor

    qodo-merge-pro bot commented Apr 9, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Ensure environment variable timing
    Suggestion Impact:The commit partially addressed the suggestion's concern about environment variable timing by moving the MOZ_ENABLE_WAYLAND setting to the Remote driver section, ensuring it's set before Firefox is launched in that context. However, it didn't implement the conditional check to avoid overriding existing values.

    code diff:

    -            os.environ['MOZ_ENABLE_WAYLAND'] = '0'
    +            os.environ["MOZ_ENABLE_WAYLAND"] = "0"
             if driver_class == "Chrome":
                 options = get_options(driver_class, request.config)
             if driver_class == "Edge":
                 options = get_options(driver_class, request.config)
             if driver_class == "WebKitGTK":
                 options = get_options(driver_class, request.config)
    -        if driver_class.lower() == "WPEWebKit":
    +        if driver_class == "WPEWebKit":
                 options = get_options(driver_class, request.config)
             if driver_class == "Remote":
                 options = get_options("Firefox", request.config) or webdriver.FirefoxOptions()
                 options.set_capability("moz:firefoxOptions", {})
                 options.enable_downloads = True
    +            # There are issues with window size/position when running Firefox
    +            # under Wayland, so we use XWayland instead.
    +            os.environ["MOZ_ENABLE_WAYLAND"] = "0"

    Setting environment variables after the process has started may not affect
    Firefox's behavior. Environment variables should be set before Firefox is
    launched. Consider checking if this environment variable is being set early
    enough to affect Firefox's startup behavior.

    py/conftest.py [157-159]

     # There are issues with window size/position when running Firefox
     # under Wayland, so we use XWayland instead.
    -os.environ['MOZ_ENABLE_WAYLAND'] = '0'
    +# Set this before Firefox is launched to ensure it takes effect
    +if 'MOZ_ENABLE_WAYLAND' not in os.environ:
    +    os.environ['MOZ_ENABLE_WAYLAND'] = '0'

    [Suggestion has been applied]

    Suggestion importance[1-10]: 7

    __

    Why: The suggestion correctly identifies a potential issue with environment variable timing. Setting environment variables after process initialization may not affect Firefox's behavior, and the improved code adds a check to avoid overriding existing values, which is a good practice for environment configuration.

    Medium
    • Update

    @Delta456
    Copy link
    Member

    Delta456 commented Apr 9, 2025

    Is there a way to also test this environment variable in test files? It would be a good addition.

    @cgoldberg
    Copy link
    Contributor Author

    @Delta456 you mean add tests that verify it works with this env variable set and broken without? I can add those.

    @Delta456
    Copy link
    Member

    Delta456 commented Apr 9, 2025

    Yes those. They will be very useful

    @cgoldberg
    Copy link
    Contributor Author

    @Delta456 I added some tests. Can you give it a review?

    @Delta456
    Copy link
    Member

    Looks good to me though the Lint CI didn't start correctly. Can you restart that?

    @cgoldberg
    Copy link
    Contributor Author

    That was actually a legitimate linting error. I just fixed it and it's running now.

    @cgoldberg
    Copy link
    Contributor Author

    CI is failing because of #15608 (unrelated to this PR)

    @cgoldberg cgoldberg merged commit 7b530cd into SeleniumHQ:trunk Apr 10, 2025
    2 of 4 checks passed
    @cgoldberg cgoldberg deleted the py-xwayland-firefox-tests branch April 10, 2025 17:16
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    [🐛 Bug]: [py] Unable to set window position in Firefox
    3 participants