Skip to content
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

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

Open
janbrasna opened this issue Jan 14, 2025 · 9 comments · May be fixed by #15874
Open

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

janbrasna opened this issue Jan 14, 2025 · 9 comments · May be fixed by #15874
Labels
Bug 🐛 Something's not working the way it should

Comments

@janbrasna
Copy link
Contributor

Description

Key pages retain some legibility even on super old IE versions and there are styles and scripts to achieve that. The newly added dropdown panels in nav refresh probably also need tidying up from the page flow in that bundle.

Steps to reproduce

Compare pre-refresh nav (used by not uptodate locales) e.g. https://www.mozilla.org/et/ to current layout e.g. https://www.mozilla.org/en/ in old IE versions.

Expected result

The nav is somewhat tidy not to obscure the page etc.

Actual result

The nav with its h2 elements takes over much of the page:

2023 | 2024
Image

(pre-m24 // post-m24)

Environment

IE8 @ Win7

@janbrasna janbrasna added the Bug 🐛 Something's not working the way it should label Jan 14, 2025
@janbrasna
Copy link
Contributor Author

(Interestingly, the gtm-snippet failing on missing window.Mozilla comes from consent/utils checking for DNT, while most of the JS silently expected is not loaded for IE. As with most of the header scripts, maybe the GTM JS could also be skipped for legacy IE: …)

Image

@alexgibson
Copy link
Member

@janbrasna good catch on the IE8 error. Yeah I think we can likely skip GA for IE8 now too, as I think it's largely unsupported anyways.

alexgibson added a commit to alexgibson/bedrock that referenced this issue Jan 15, 2025
alexgibson added a commit to alexgibson/bedrock that referenced this issue Jan 15, 2025
@janbrasna
Copy link
Contributor Author

It seems harmless enough to ignore, but since it's pretty much happy otherwise, it may deserve the extra effort, given the whole base template tiptoes carefully around the IE conditionals already;)

@alexgibson Thanks for verifying in #15871 it's not worth stubbing out some of the checks as it won't work anyways. I wanted to try to either find a cheap place to create a mock object, or add some extra guardrails around the DNT check to be even more resilient — to try to keep GTM/GA loading and collecting stats — as I was unable to look up any explicit IE compatibility issues or limitations mentioned online, so kinda expected if it worked in IE9, it ought to work in IE8 too. So thanks for trying that first, it saved me a lot of time wrestling with attempts adapting modern JS for legacy engines;D

@alexgibson
Copy link
Member

alexgibson commented Jan 15, 2025

Thanks @janbrasna - yeah, I even see JS errors from the GA snippet in IE9 when it loads (google's code, not ours), so support there is even limited these days I think. But we already don't support things like Firefox attribution on IE8, and have relatively few visits these days from such an old browser. So I think we're fine to simple stop loading it.

@janbrasna
Copy link
Contributor Author

Best effort 🤷

Quoting https://support.google.com/tagmanager/answer/4620708?hl=en

July 2, 2024
Starting on July 15, 2024, Google Tag and Google Tag Manager will no longer support Microsoft Internet Explorer. While scripts may still function, Google will not actively test or fix issues on Internet Explorer. This aligns with Microsoft's decision to end Internet Explorer support on June 15, 2022.

Seems they've broken something since last summer;) Well, in case it resolves itself over time on their side, good call only commenting out what doesn't work on bedrock-side in iE8, and let IE9 try to load it.

👍 🚀

(Anyways. Now with the legacy nav in main, I'll rebase using the added asset, and follow up with the rest of this issue tidying up the new nav too.)

@alexgibson
Copy link
Member

(Anyways. Now with the legacy nav in main, I'll rebase using the added asset, and follow up with the rest of this issue tidying up the new nav too.)

Awesome, thanks @janbrasna 💯

@janbrasna janbrasna linked a pull request Jan 15, 2025 that will close this issue
stephaniehobson pushed a commit that referenced this issue Jan 16, 2025
* Prevent loading GA on IE8 and below (Issue #15867)

* Prevent loading newsletter JS on old IE (Issue #15867)
@janbrasna
Copy link
Contributor Author

While the actual common-old-ie_navigation-ie as per this issue look reasonably right in #15874, there is another part to this compatibility.

The product desktop & base flows (firefox/new, /mac, /win, /download/thanks etc.) augment the basic styling for such support profile (docs) by avoiding the conditionals and and serving the protocol base styles here in extrahead (docs; parts no longer true, parts missing, otherwise still applies) — that got however recently updated to also serve refresh styles as protocol 2024 base and refresh nav — that are however no longer compatible with such browsers (grids, flex, variables et al.), so this attempt to enhance the experience actually makes it only look more broken:

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

@alexgibson What would you say to make an end to attempts to "style more" of these pages, and just rely on the basics and document structure by removing this exception? (PoC/idea here: janbrasna@c2193fe … TBR to-be-refined)

@alexgibson
Copy link
Member

@janbrasna thanks for bringing this up. At this point I would say we would be OK to serve IE9 and below the very basic styles, as long as content was still readable. Firefox is no longer supported on those operating systems, so as long as they still successfully see the Firefox ESR download messaging then it should be OK imho. Does that make sense to you?

@janbrasna
Copy link
Contributor Author

👍 I think it is probably the only way forward to support all the m24 changes at the same time (besides heavily hiding all the complex styling behind some .is-modern-browser guardrails etc.) — the current CSS feature levels used are just too far ahead from what can reasonably be degraded to look at least legible on UAs from the aughts… so sticking with just the fallback bundle feels safer. It still has some of the better compatible mzp components and renders the layout a notch fancier above just B&W linear document, so it's not too bad — I'll try to document any changes thoroughly to make sure the result is actually more usable compared to the current state, not less (give or take some pure decoration).

I will however take the liberty to follow up with a broader support profile issue for visibility if you don't mind, where this change to enhancing the experience is one part, but some other changes in the browser matrix might also be necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐛 Something's not working the way it should
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants