Hani/ Fix test mixed content download https

[sv-hyacoub]

Relevant Links

Bugzilla: 2064075
TestRail: 1756722

Description of Code / Doc Changes

• Fix Fix tests/downloads/test_mixed_content_download_via_https.py

Process Changes Required

Mark the relevant boxes, delete irrelevant lines.

• Adds a dependency (rerun uv sync)
• Modifies a git hook (rerun ./devsetup.sh)
• Changes the BasePage
• Changes or creates a BOM/POM (name the object model): _
• Changes CI flow
• Changes scheduled Beta / DevEdition / RC
• Changes Autofill L10n harness

Screenshots or Explanations

N/A

Comments or Future Work

N/A

Workflow Checklist

• Reviewers have been requested.
• Code has been linted and formatted.
• If this is an unblocker, a message has been posted to #dte-automation in Slack.

Thank you!

[github-actions]

The changes look good overall. Two minor observations:

1. Logic ordering: The new nav.click_download_button() call opens the downloads panel before waiting for the download to appear in it. This is the correct fix — previously the test waited for the download element without ever opening the panel, so the wait would time out. The fix properly sequences: trigger download → open panel → verify download entry → verify name/completion.

2. Unstable comment removed: Removing the comment field in key.yaml is fine since the bug is being fixed, but if the Bugzilla link is useful for future reference, consider keeping it as a # inline comment in the YAML instead (e.g. result: pass # https://bugzilla.mozilla.org/show_bug.cgi?id=2064075). Not a blocker.

Overall the fix is correct and the approach is sound.

[github-actions]

The fix looks correct. The old code had a problematic double-wait pattern: web_page.wait.until(lambda _: nav.element_visible(...)) called element_visible (which already does its own self.expect() wait internally), making the outer wait.until redundant and confusing.

The new approach — explicitly clicking the downloads button to open the panel before checking for download-target-element — is more reliable since download-target-element lives inside the downloads panel and requires the panel to be open.

One minor thing to verify: if Firefox auto-opens the downloads panel when the download starts (some versions do this), a subsequent click_download_button() could toggle it closed rather than open it, causing element_visible to then time out. If you've confirmed this isn't an issue in practice (or if Firefox no longer auto-shows the panel for this download type), the fix is good to go.

[github-actions]

Good fix overall. The old pattern web_page.wait.until(lambda _: nav.element_visible(...)) was broken — element_visible already blocks internally via expect, so wrapping it in wait.until produced unpredictable behavior. The new flow of explicitly opening the downloads panel before checking for download-target-element is correct for a URL-navigation-triggered download (unlike link-click downloads which show a warning panel that opens the panel as a side effect).

One minor note: nav.element_visible("downloads-button") on line 33 is a no-op since that button is always present in the toolbar — it won't confirm a download has started. Consider removing it or replacing with a wait for the button's animation/attention state to confirm the download actually began before opening the panel.

[github-actions]

Overall this is a good fix — opening the download panel before checking for download-target-element correctly addresses the root cause of the instability (the panel item wasn't visible because the panel hadn't been opened).

One minor issue inline, but nothing blocking.

[github-actions]

The core fix is correct — opening the downloads panel before checking for download-target-element addresses the root cause. However, the new code reproduces the exact anti-pattern it replaced:

# Before (broken)
web_page.wait.until(lambda _: nav.element_visible("download-target-element"))

# After (same issue, different element)
web_page.wait.until(lambda _: nav.element_visible("downloads-button"))

element_visible calls self.expect() internally, which already blocks and raises TimeoutException on failure. Wrapping it in wait.until doesn't add retry semantics — if the inner expect times out, the exception propagates directly through the lambda and wait.until re-raises rather than retries. The wait.until wrapper is effectively a no-op here.

Since element_visible already waits, the correct usage is:

nav.element_visible("downloads-button")
nav.click_download_button()
nav.element_visible("download-target-element")

If you want to use wait.until, use a method that returns bool without blocking, such as nav.is_download_button_visible() (already exists at line 1554 of browser_object_navigation.py).

[github-actions]

The core fix (opening the downloads panel before checking for download-target-element) is correct and addresses the root cause of the instability.

Two issues worth addressing before marking this as stable:

1. Line 36 — nav.element_visible("downloads-button") is a no-op: the downloads button is always in the toolbar, so this returns immediately and doesn't confirm a download has started. Either remove it or replace it with something that actually waits on download state (e.g., nav.wait_for_download_animation_finish()).

2. Line 37 — If Firefox auto-opens the downloads panel when the download begins (possible depending on build/prefs), calling click_download_button() will toggle the panel closed, and the subsequent element_visible("download-target-element") will time out. This is a potential source of continued intermittent failure. Consider checking or ensuring panel state before clicking.

[github-actions]

Review

The core fix is correct — the old web_page.wait.until(lambda _: nav.element_visible(...)) was broken because element_visible already calls self.wait.until internally; if that inner wait timed out, the exception would propagate through the lambda and wait.until would re-raise it rather than retry. The new code is cleaner.

One concern worth checking before landing:

Similar tests in this suite (e.g., test_download_apk_and_check_extension.py, test_download_epub_shows_extension_in_downloads_panel.py) use the pref ("browser.download.alwaysOpenPanel", True) via add_to_prefs_list so the panel opens automatically, then call element_visible("download-target-element") directly without manually clicking the button.

This test instead manually calls nav.click_download_button() without that pref. If Firefox auto-opens the downloads panel when the download starts (which can happen depending on build defaults), click_download_button() will toggle the panel closed, and the subsequent element_visible("download-target-element") will time out — reproducing the original instability.

Recommend either:

• Adding add_to_prefs_list with ("browser.download.alwaysOpenPanel", True) and removing the manual click_download_button() call (consistent with suite pattern), or
• Keeping the manual click but confirming that the panel does not auto-open for this URL/download type.

[github-actions]

Review

The fix looks correct. Setting browser.download.alwaysOpenPanel to True ensures the downloads panel appears automatically when the download triggers, which is what allows element_visible("download-target-element") to succeed. This is the right approach to stabilize the test.

One concern: The test downloads from an external third-party URL (file-examples.com). The previous instability comment suggested CI flakiness — if any of that was network-related (slow/unresponsive external host) rather than the panel not showing, the test may still be intermittently unreliable. Worth monitoring after merging before fully removing the unstable label in the manifest.

Otherwise the changes are clean and the simplification of the wait logic is correct.

[github-actions]

The fix is correct and consistent with the suite pattern. Using add_to_prefs_list to set browser.download.alwaysOpenPanel: True ensures the panel auto-opens on download start, so element_visible("download-target-element") correctly waits for the entry without needing a manual panel click. This is the same approach used by other tests in the downloads suite.

Two minor issues inline.

[github-actions]

The fix is correct and consistent with the suite pattern — using add_to_prefs_list to set browser.download.alwaysOpenPanel: True ensures the panel auto-opens on download start, making element_visible("download-target-element") reliable without needing a manual click.

One unused constant: MAX_CHECKS = 30 on line 21 is no longer referenced anywhere in the file and should be removed.

[github-actions]

Good fix overall. The root cause is correctly addressed by adding browser.helperApps.neverAsk.saveToDisk — this prevents intermittent save dialogs from blocking the test flow, which was the likely source of instability.

One note on the wait simplification: the original web_page.wait.until(lambda _: nav.element_visible(...)) was actually double-waiting — element_visible already polls internally via self.expect(). The simplified call is correct. The comment # Wait for the downloads button to appear before interacting with it is slightly misleading since element_visible does wait (not just check once), but this is minor.

[github-actions]

Review summary

The prefs fixture approach is the right fix. Two issues worth addressing:

Bug: delete_files_regex_string won't clean up the downloaded file

The existing regex r"\bdownload\b" (line 15) will never match file-sample_100kB.odt. re.match() anchors at the start of the string, and the word download does not appear in that filename at all, so the file leaks after the test. Compare to other tests in the suite (e.g. test_add_mime_type_doc.py uses r"sample.*\.doc"). Should be something like r"file-sample_100kB.*\.odt".

Minor: LibreOffice may auto-open the .odt on Linux

The close_external_apps fixture in tests/downloads/conftest.py was added specifically to kill LibreOffice/soffice processes that can auto-launch after an .odt download. The test does not use it, which could pollute subsequent tests on Linux CI.

Everything else (prefs fixture, element_visible call, manifest update, ruff reformats) looks good.

[github-actions]

Two concrete issues to fix:

1. delete_files_regex_string won't clean up the downloaded file

Line 15 currently returns r"\bdownload\b". re.match(r"\bdownload\b", "file-sample_100kB.odt") returns None, so the downloaded file leaks after every test run. Should be:

return r"file-sample_100kB.*\.odt"

2. Missing close_external_apps fixture

The test downloads a .odt file. The close_external_apps fixture in tests/downloads/conftest.py exists specifically to kill LibreOffice/soffice processes on Linux that auto-launch after opening an .odt download. Without it, stale LibreOffice processes can pollute subsequent CI tests. Add it to the test signature:

def test_mixed_content_download_via_https(driver: Firefox, delete_files, close_external_apps):

[github-actions]

The core fix is correct and clean. A few observations:

What's fixed: Setting browser.download.alwaysOpenPanel = True via add_to_prefs_list ensures the downloads panel auto-opens when the download starts, so download-target-element is present when verify_download_name calls get_element (which uses Selenium's implicit wait). The simplified test flow is better.

Lambda changes in page_object_prefs.py / test_c3298824: These are purely cosmetic — adding explicit parentheses around the lambda body. Python already parsed the original form as lambda _: (execute_script() == state), so there's no functional difference. Good for readability though.

Pre-existing issue (out of scope, but worth noting): delete_files_regex_string returns r"\bdownload\b", which won't match the downloaded filename file-sample_100kB.odt. The cleanup fixture won't actually delete the test artifact. This predates this PR, but could cause flakiness if the file already exists from a prior run.