WT-841 Show correct Windows download link

[bluewave41]

One-line summary

Windows users are shown the 32 bit download link on https://www.mozilla.org/firefox/installer-help/?channel=release regardless of their current architecture.

Issue / Bugzilla link

https://bugzilla.mozilla.org/show_bug.cgi?id=1564333

Testing

Before each step remember to change the HTML class OS to "windows"
The x64 doesn't matter as it doesn't affect the shown download button. Only "win" is ever displayed

• Checkout this PR
• Visit http://localhost:8000/en-US/download/installer-help/?channel=release
• Remove the x64 class from HTML if it exists
• The download link should be https://download.mozilla.org/?product=firefox-latest-ssl&os=win&lang=en-US
• Add the x64 class back to HTML
• The download link should still be https://download.mozilla.org/?product=firefox-latest-ssl&os=win&lang=en-US
• Visit http://localhost:8000/en-US/download/installer-help/?channel=release&installer_arch=1
• The download link should be https://download.mozilla.org/?product=firefox-latest-ssl&os=win&lang=en-US
• Visit http://localhost:8000/en-US/download/installer-help/?channel=release&installer_arch=2
• The download link should be https://download.mozilla.org/?product=firefox-latest-ssl&os=win64&lang=en-US
• Visit http://localhost:8000/en-US/download/installer-help/?channel=release&installer_arch=3
• The download link should be https://download.mozilla.org/?product=firefox-latest-ssl&os=win64-aarch64&lang=en-US

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.00%. Comparing base (3679819) to head (04d7e72).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1152   +/-   ##
=======================================
  Coverage   79.00%   79.00%           
=======================================
  Files         132      132           
  Lines        8236     8236           
=======================================
  Hits         6507     6507           
  Misses       1729     1729           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[janbrasna]

Just proxying Hector's note from mozilla/bedrock#8635 (comment) for visibility.

The TL;DR is the true x64 bitness is not provided by all user agents and is therefore not reliable. There should be a new query parameter available now locking the actual arch used for the installation that pointed back to this page.

[maureenlholland]

@janbrasna do you know how we can trigger the installer_arch param for testing? (Otherwise, I guess we just assume it will be there and use test urls that already include the installer_arch param)

[janbrasna]

I believe it comes from within in-product flow, either from the stub installer unable to install, or the in-product updater not able to update(?) — @l-hedgehog Would you know how to trigger that for testing? (or who to ask on the installer team? TY)

For the NSIS patch linked it's this: https://github.com/mozilla-firefox/firefox/blob/c1e060fc/browser/installer/windows/nsis/stub.nsh#L98-L101 so IIUC the archs are passed as literal installer_arch=1 installer_arch=2 installer_arch=3 values in that URL?

[l-hedgehog]

@janbrasna Essentially you'll have to somehow interrupt the stub installer while it's downloading the full installer in the background. I guess you could try just disabling the network during that phase, or if the download finishes too quick to test, maybe adding an entry to hosts file and point downloads.mozilla.org to localhost.

[maureenlholland]

IIUC the archs are passed as literal installer_arch=1 installer_arch=2 installer_arch=3 values in that URL?

That's my understanding too. Thanks both for the tips.

@bluewave41 for development I think the easiest route will be testing directly with the installer arch params:

• 192.168.x.x:8000/download/installer-help/?channel=release&installer_arch=1 (32-bit)
• 192.168.x.x:8000/download/installer-help/?channel=release&installer_arch=2 (64-bit)
• 192.168.x.x:8000/download/installer-help/?channel=release&installer_arch=3 (AARCH64)

We can try triggering from interrupting the stub installer to confirm these urls but I think it's safe to assume we can rely on the expected params being there

[bluewave41]

Ahh I get the full scope now.

Blocking /etc/hosts on Windows with: 127.0.0.1 download.mozilla.org

Brings you to this URL once pressing the OK/continue button when asked to retry.

www.firefox.com/en-US/download/installer-help/?redirect_source=mozilla-org&channel=release&installer_lang=en-US&installer_arch=x

[janbrasna]

"This feels a little bit like cheating as the page has elements for win, win64 and win64-aarch divs specifically but I don't see an easy way to target the aarch64 div as the classes on HTML only support nothing and x64. I don't think it's worth going through and adding support for also showing aarch64 when it's most likely only going to be used on this page."

OK that's kind of a good point. Let's cheat differently. Could you turn the extra query arg into a new body/html class (wherever is easier to reach from the jinja context you'll be touching) — say "force-win64" based on the arg — and then just hide all .force-win64 .os_* combinations via specificity, and only show .force-win64 .os_win64 back? (That could theoretically even override the platform user agent sniffing to make sure it always positively selects from windows buttons if the URL specifies coming from windows stub;D) — That way you create your own namespace for the classes so you can define e.g. .force-win64arm .os_win64-aarch64 to show just based on what you put there already in the template based on the URL query, even though the current Client JS body classes don't expose that per se, and you won't need it to that way.

[bluewave41]

"This feels a little bit like cheating as the page has elements for win, win64 and win64-aarch divs specifically but I don't see an easy way to target the aarch64 div as the classes on HTML only support nothing and x64. I don't think it's worth going through and adding support for also showing aarch64 when it's most likely only going to be used on this page."

OK that's kind of a good point. Let's cheat differently. Could you turn the extra query arg into a new body/html class (wherever is easier to reach from the jinja context you'll be touching) — say "force-win64" based on the arg — and then just hide all .force-win64 .os_* combinations via specificity, and only show .force-win64 .os_win64 back? (That could theoretically even override the platform user agent sniffing to make sure it always positively selects from windows buttons if the URL specifies coming from windows stub;D) — That way you create your own namespace for the classes so you can define e.g. .force-win64arm .os_win64-aarch64 to show just based on what you put there already in the template based on the URL query, even though the current Client JS body classes don't expose that per se, and you won't need it to that way.

That was my first thought but it seemed like I was touching a larger scope than I wanted to for something that will only be used on a single page.

I replaced my code with a commit that adds an aarch64 class that gets applied in place of x64 based on installer_arch since I think it makes sense to consider it a different architecture. With this we can just hide the os_win div and instead show the os_win64-aarch64 one which feels less cheaty.

I think with this we should be able to also use user agent sniffing to apply the aarch64 class correctly as a quick Google search says they display like Mozilla/5.0 (Windows NT 10.0; ARM64; RM-1096) AppleWebKit/537.36 (KHTML like Gecko) Chrome/51.0.2704.79 Safari/537.36 Edge/14.14393

[l-hedgehog]

That was my first thought but it seemed like I was touching a larger scope than I wanted to for something that will only be used on a single page.

I think you're now offering win64 installers not only for installer_arch=2, but also when .windows.x64 is applied by site.js. I believe the latter was deliberately not the case and should stay that way.

[bluewave41]

That was my first thought but it seemed like I was touching a larger scope than I wanted to for something that will only be used on a single page.

I think you're now offering win64 installers not only for installer_arch=2, but also when .windows.x64 is applied by site.js. I believe the latter was deliberately not the case and should stay that way.

Gotcha so.

No installer_arch = win32 always
installer_arch=1 = win32
installer_arch=2 = win64
installer_arch=3 = win64-aarch64

[maureenlholland]

the installer arch work looks good to me 🎉

one blocking issue on Nightly download button for http://localhost:8000/en-US/channel/desktop/

We should confirm this PR passes existing integration tests before merge: https://mozmeao.github.io/platform-docs/operations/pipeline/#pushing-to-the-integration-tests-branch
(^ this may require some permissions you don't yet have, if you are blocked here, ask in www slack for permissions)

With any new download functionality, we should also add new tests

• jasmine: https://mozmeao.github.io/platform-docs/testing/jasmine/
• playwright: https://mozmeao.github.io/platform-docs/testing/playwright/

Jasmine is probably the best for the new logic in site.js, but we might need to adjust or update Playwright to confirm the channel page download functionality too

[maureenlholland]

issue (blocking): we've lost the nightly button on channels pages for 64-bit. It doesn't show up in the download list either, so it's not a CSS display issue.

Beta & dev for 64-bit are fine. No issues with aarch64 or 32-bit on any of these channels either.

[maureenlholland]

initial integration tests run: https://github.com/mozmeao/springfield/actions/runs/23549818496/job/68560367753

[janbrasna]

Could you turn the extra query arg into a new body/html class (wherever is easier to reach from the jinja context you'll be touching) — say "force-win64" based on the arg — and then just hide all .force-win64 .os_* combinations via specificity, and only show .force-win64 .os_win64 back? […] — That way you create your own namespace for the classes […]

That was my first thought but it seemed like I was touching a larger scope than I wanted to for something that will only be used on a single page.

I perhaps forgot to mention only adding that template class override only for that one specific page where it's expected to be parsing the query string. Haven't checked actual feasibility in terms of what view handles that and how easy is to add the request args to the render context etc., so that was mainly thinking out loud, how to contain the change within the targeted page as much as possible.

I replaced my code with a commit that adds an aarch64 class that gets applied in place of x64 based on installer_arch since I think it makes sense to consider it a different architecture.

That however sounds like this could affect any page running site.js, if a query param is added to any such arbitrary page?

(Also, the issues surfaced by integration tests on other pages not subject to this change per se means the potential to change CTAs behavior is currently site-wide.)

Why I'm mentioning it is, without deeper understanding of when the stub vs. latest installer is chosen for any given CTA, in most cases the ideal version served for os_win is the small 32bit stub installer, that then itself picks the right arch once run on the target system — which normally is the preferred even for x64 and arm64 — until it fails, and then points towards the /installer-help page — which should probably be the only place that ought to enforce an arch, compared to other CTA buttons around the site. That's just a comment regarding touching site-wide functionality, compared to just something contained in plain querystring → template class → style/visibility overrides over any Client/site.js classes applied just for this one specific template. (But I'll defer to reviewer for advice.)

I think with this we should be able to also use user agent sniffing to apply the aarch64

It was not included in the Client/site.js because it is not exposed and most user agent strings realistically just freeze the values, or, was not deemed reliable even when using the JS navigator data. It might work for some browsers using client-hints, but e.g. will definitely not work if opened in Firefox. The code used to be there, but was never used: mozilla/bedrock#8454 (comment)