Simplify new nav for old IE basic styling by janbrasna · Pull Request #15874 · mozilla/bedrock

janbrasna · 2025-01-15T13:43:51Z

If this changeset needs to go into the FXC codebase, please add the WMO and FXC label. 🤷 (yes, this will need re-applying to fxc 🔥🦊 with different paths)

One-line summary

Streamlines the experience for IE9- and polishes a bit more in IE8- to make the site usable with recent (mostly m24) styling additions.

Significant changes and points to review

Constrains refreshed header & footer to just top level nav items without any interaction, for better usability and scanability of the rest of pages.

Improves some basic alignment and adds alternatives for key SVG affordances (to not look broken in places where it is used for navigation/interaction, removing large objects with rendering issues like header flag or footer wordmark, augmenting download steps with plaintext pseudo content to allow navigation etc., while keeping product logos in menus for IE versions that can support that).

Removes fallback styling overrides ("enhancements") that try to inject full modern bundles (incl. current base m24 styles) for otherwise base-less legacy-only pages like fx/desktop, as it was only making the experience worse, not better. Being covered by the common-IE bundle like everywhere else, with the added extra desktop/ie* still for enhanced styling of the components there, seems to serve such clients better, also being more stable and predictable (=frozen in time, instead of blindly serving full modern bundles' changes over time…)

Also cleans up some superfluous meta and duplicate hyphenation for ?xv=basic e.g. fx/mac or fx/linux for everyone while at it… SERPs strip what's after the hyphen anyway, or use the shorter opengraph content…

Tweaks the ESR/unsupported CTAs as these are now the default for such platforms+UAs.
(+tiny RTL fixes.)

(Also, with the switch to ESR and ESR download button macro magic, these UAs skip the download/thanks flow anyways and request direct bouncer links, so no need to keep enhancing paths not being taken any longer.)

Removes custom IE8-9 styles that no longer need to be styled, being covered by the common-ie base protocol tweaks sufficiently.

NB: Updates browser compatibility info in docs with some more precise datapoints.

Issue / Bugzilla link

Fixes #15867

Testing

http://localhost:8000/en/
http://localhost:8000/fy/firefox/new/
http://localhost:8000/en/firefox/new/?xv=basic
http://localhost:8000/cy/firefox/windows/
http://localhost:8000/en/firefox/download/thanks/?xv=basic
http://localhost:8000/sco/firefox/download/thanks/
http://localhost:8000/he/firefox/all/desktop-esr/win/
http://localhost:8000/ar/about/
http://localhost:8000/el/ (og home)
http://localhost:8000/pl/ (og nav)

Screen.Recording.2025-01-22.at.23.36.17.mov

Screen.Recording.2025-01-22.at.2.28.36.mov

Screen.Recording.2025-01-22.at.2.19.15.mov

Some corner cases:

Screen.Recording.2025-01-20.at.23.18.08.mov

Screen.Recording.2025-01-20.at.23.18.09.mov

codecov · 2025-01-23T00:51:26Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.71%. Comparing base (3a7a8c3) to head (810a14a).
Report is 11 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #15874      +/-   ##
==========================================
+ Coverage   79.55%   79.71%   +0.16%     
==========================================
  Files         160      160              
  Lines        8436     8518      +82     
==========================================
+ Hits         6711     6790      +79     
- Misses       1725     1728       +3

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

maureenlholland

Really great improvement on the IE styles, thank you! 🎉

I have a few questions on some of the changes, but I don't think there's anything stopping this from merging as-is. r+wc

bedrock/firefox/templates/firefox/new/basic/base.html

maureenlholland · 2025-03-31T14:10:31Z

media/css/protocol/components/_navigation-ie.scss

+    li {
+        @include bidi(((margin-right, $spacing-md, margin-left, 0),));
+        display: inline-block;
+        *display: inline;


question What is *display? My initial thought was a typo, but is it some kind of Sass thing?

IE6-IE7 hack 🚀 ;) I know it's unsupported, but for consistency… (and SASS doesn't complain and happily outputs it, so I kept it).

TIL, thanks! - would love a comment to help remember that in future

(Yes I'm old;D) Basically I followed what's historically in the file — would you like a note added to all occurrences, even preexisting? (There are ca half a dozen throughout this file over the years, along with other IE7/8 hacks with slashed values and similar, that I'm not sure I'd be able to correctly infer these days.)

(Why I'm mentioning it is… after opening the file for the first time it became obvious that some familiarity with CSS hacks from the 00s is necessary to understand the reasoning behind some of the selectors, rules and values; I even had to double check the effect of some of the fancier ones.)

Nonetheless a very good point is, that all the documentation mentions IE8-9, however there are tweaks even for IE6-7 still present, even though long removed from support profile. I'd keep them in as they're harmless, but at the same time keep a note of the fact that we have some tweaks even for versions older than we declare otherwise.

I'd keep them in as they're harmless

In terms of helping render for older browsers and not disrupting newer ones, yes. But in terms of code maintenance, no. They are confusing and will take time to research for future devs. A note in the docs or one comment at top of file would be helpful in my opinion.

But I plan to merge this PR at end of my day in any case! It's great work and, as you mention, the hacks are pre-existing

(I perhaps forgot to add that actually combing over the legacy styles and removing hacks for UAs no longer even in the degraded support profile might make sense to cut down on the weird constructs; however that's a separate task I'm unable to support within the scope of this PR — mainly because I'm cargo-culting some of the decisions from older code, and while happy to remove, I'd personally like to see the impact for those older-than-old UAs to be sure what I'm committing in — for which I'd need a handful of extra VMs to test I did not need to have ready for the scope of this PR, so I'm making a note to come back to such IE7- code leftovers one day, and clean that up in an isolated change…)

They were authored in times when their use and meaning was perhaps more ubiquitous, and only aged well with how these conditional styles lost their importance over time.

But truth is, as can be seen with the last-minute commit, that even harmless dart-sass bump may cause that legacy code to actually compile in a way no longer working the way it was intended (in case of the recent bug it started transpiling transparent keyword as an rgba() set completely breaking that rule with any other values following it also ignored in such old UAs…)

I'll try to add some good introduction to what's important in the file, to help future spelunkers;) — only need to think about a good resource to link (as I prefer this https://gist.github.com/ricardozea/5549389 list, but not sure if that's a good 101 material to explain the concept itself…)

maureenlholland · 2025-04-01T10:28:20Z

bedrock/firefox/templates/firefox/new/basic/base.html

-{% block page_og_desc %}{{ ftl('firefox-new-desc', fallback='firefox-new-faster-page-loading-less-memory') }}{% endblock %}
-
-{# All stylesheets are loaded in extrahead to serve legacy IE basic styles #}
-{% block extrahead %}


media/css/protocol/components/_navigation-ie.scss

maureenlholland · 2025-04-01T10:51:14Z

media/css/protocol/components/_fx-unsupported-ie.scss

+// too little space to display all the ESR messaging.
 .c-navigation .c-navigation-shoulder .c-button-download-thanks {
-    display: none !important;
+    p {


note (non-blocking) we can end up with a double CTA on legacy home (not an issue on new-home)

Good catch. I think this PR doesn't change the logic of the macro that decides what to show in the shoulder, so this was duplicated earlier before being hidden for ESR.

Normally I'd say let's hide the support explainers and just keep the CTA, however over time the navigation-v2 lost its importance, as I actually do think there's not a prod locale now that would show anything older than m24-navigation AND anything on home at the same time; it's either at least activated for m24 header and footer, or the language doesn't activate any home variant at all, and just fall back to default language wholesale.

(Few months ago this might have been a corner case for a handful of locales, but since then Polish or Hebrew got the m24 nav translated, so they no longer show nav-v2; and those who still are on nav-v2 don't have the homepage threshold high enough to enable at prod at all, like Estonian, Latvian, Lithuanian demonstrated above in dev.)

So while a valid wrinkle, the current state of locale activation metadata effectively works around it in prod.

janbrasna

Thanks for such a thorough review @maureenlholland, I realize this is an unorthodox changeset with unusual testing surface;)

I have suggested a few explainers, so feel free to accept these if that makes any difference, or amend as you see fit to better explain the impact:

media/css/protocol/components/_navigation-ie.scss

media/css/protocol/common-old-ie.scss

Co-authored-by: Jan Brasna <1784648+janbrasna@users.noreply.github.com>

maureenlholland

r+ 💯 thanks again for the thoughtful changes @janbrasna

* Simplify new nav for IE9- * Make better use of space * Show nav icons in IE9+ * Degrade styling of fx product flow in IE9- * Update browser-support.rst * Download improvements * Download ESR tweaks in shoulder * Update bundle usage note * Simplify fx/new ie8-9 tweaks * Lint colons like it's 1999 * Remove fx/platform ie bundles * Update browser-support.rst exceptions * Update browser-support.md * Fixup logo compile compatibility * Add comment on .c-product-info Co-authored-by: Jan Brasna <1784648+janbrasna@users.noreply.github.com> * Add comment on older IE hacks Co-authored-by: Jan Brasna <1784648+janbrasna@users.noreply.github.com> --------- Co-authored-by: maureenlholland <19650432+maureenlholland@users.noreply.github.com>

janbrasna added 4 commits January 15, 2025 14:12

Simplify new nav for IE9-

0eb6b1a

Make better use of space

831a499

Show nav icons in IE9+

777151d

Degrade styling of fx product flow in IE9-

c2193fe

janbrasna mentioned this pull request Jan 20, 2025

Nav refresh not hidden by common-old-ie #15867

Closed

janbrasna added 5 commits January 20, 2025 21:09

Update browser-support.rst

0bf591d

Download improvements

cd7c3be

Download ESR tweaks in shoulder

7c1b191

Update bundle usage note

134113f

Simplify fx/new ie8-9 tweaks

f246ea3

Lint colons like it's 1999

3aa8180

janbrasna changed the title ~~Simplify new nav in old IE bundle~~ Simplify new nav for old IE styling Jan 24, 2025

janbrasna added 2 commits January 24, 2025 20:42

Remove fx/platform ie bundles

246eb49

Update browser-support.rst exceptions

d5c86eb

janbrasna marked this pull request as ready for review February 16, 2025 19:01

janbrasna requested a review from a team as a code owner February 16, 2025 19:01

janbrasna changed the title ~~Simplify new nav for old IE styling~~ Simplify new nav for old IE basic styling Feb 16, 2025

Merge branch 'main' into add/nav-refresh-ie-support

25c1ff9

maureenlholland added Frontend HTML, CSS, JS... client side stuff Needs Review Awaiting code review labels Mar 19, 2025

janbrasna added 3 commits March 27, 2025 13:21

Merge branch 'main' into add/nav-refresh-ie-support

98fc227

Update browser-support.md

a8f762d

Fixup logo compile compatibility

fa61c82

maureenlholland self-assigned this Mar 31, 2025

maureenlholland added the WMO and FXC Code relevant to both mozilla/bedrock (www.mozilla.org) and mozmeao/springfield (www.firefox.com) label Mar 31, 2025

maureenlholland approved these changes Apr 1, 2025

View reviewed changes

janbrasna commented Apr 1, 2025

View reviewed changes

media/css/protocol/components/_navigation-ie.scss Show resolved Hide resolved

media/css/protocol/common-old-ie.scss Show resolved Hide resolved

maureenlholland and others added 2 commits April 1, 2025 17:20

Add comment on .c-product-info

505b2cf

Co-authored-by: Jan Brasna <1784648+janbrasna@users.noreply.github.com>

Add comment on older IE hacks

810a14a

Co-authored-by: Jan Brasna <1784648+janbrasna@users.noreply.github.com>

maureenlholland approved these changes Apr 1, 2025

View reviewed changes

maureenlholland merged commit 145ff28 into mozilla:main Apr 1, 2025
6 checks passed

janbrasna deleted the add/nav-refresh-ie-support branch April 1, 2025 16:36

Conversation

janbrasna commented Jan 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

One-line summary

Significant changes and points to review

Issue / Bugzilla link

Testing

Uh oh!

codecov bot commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

maureenlholland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

janbrasna left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

maureenlholland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

janbrasna commented Jan 15, 2025 •

edited

Loading

codecov bot commented Jan 23, 2025 •

edited

Loading