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

Moments page for Root Certification expiration (1 of 6) #15212

Merged
merged 3 commits into from
Oct 7, 2024

Conversation

reemhamz
Copy link
Contributor

@reemhamz reemhamz commented Sep 24, 2024

One-line summary

Moments page made for letting users know to update their Firefox browsers so they aren't affected by the Root Certification expiration.

This is 1 of 6 moments pages to be developed for the Root Certification info. More to come in a few months

📆 Launch date: October 9th 2024 📆

Note

This is hard-coded in 15 languages, enjoy the review! 😅

Issue / Bugzilla link

#15162

Testing

Localhost

Demo servers

@reemhamz reemhamz added WIP 🚧 Pull request is still work in progress Frontend HTML, CSS, JS... client side stuff labels Sep 24, 2024
@reemhamz reemhamz marked this pull request as draft September 24, 2024 07:41
Copy link
Contributor

@janbrasna janbrasna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👋 Enjoyed! ;)

(BTW, what is "addon-geddan" in the commit message?)

bedrock/firefox/templates/firefox/welcome/page19.html Outdated Show resolved Hide resolved
bedrock/firefox/templates/firefox/welcome/page19.html Outdated Show resolved Hide resolved
bedrock/firefox/templates/firefox/welcome/page19.html Outdated Show resolved Hide resolved
bedrock/firefox/templates/firefox/welcome/page19.html Outdated Show resolved Hide resolved
bedrock/firefox/templates/firefox/welcome/base.html Outdated Show resolved Hide resolved
bedrock/firefox/templates/firefox/welcome/page19.html Outdated Show resolved Hide resolved
bedrock/firefox/templates/firefox/welcome/page18.html Outdated Show resolved Hide resolved
bedrock/firefox/urls.py Outdated Show resolved Hide resolved
@reemhamz reemhamz changed the title Moments page for Root Certification expiration Moments page for Root Certification expiration (1 of 6) Sep 25, 2024
@reemhamz reemhamz added Needs Review Awaiting code review Review: XS Code review time: up to 30min and removed WIP 🚧 Pull request is still work in progress labels Sep 25, 2024
@reemhamz reemhamz marked this pull request as ready for review September 25, 2024 11:09
Copy link
Contributor

@janbrasna janbrasna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following up with a quick linting fixup, and a broken logotype on page16-page18 when switching dark/light modes:

bedrock/firefox/urls.py Outdated Show resolved Hide resolved
bedrock/firefox/templates/firefox/welcome/base.html Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

attn: welcome/16 and /18 both reference the file, so better not remove that from those…

(i have a wip in my backlog to clean these welcome xrefs between versions where possible, and also dedupe the assets if they're already somewhere more canonical… so i'll tackle that at some point later separately.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those pages are mentioning page../firefox-wordmark-light.svg, not this particular file path?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At one point this image was moved here from page16 assets so I figured it would be missing there:

Screen Shot 2024-09-26 at 05 34 43

Now that 16–18 are updated too to link the canonical location instead it should be all hunky-dory. (Just wanted to point out it touches more welcome pages' both light and dark appearances so to make sure they're also included in testing they still render as expected…)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

d'oh, i forgot that i moved it, i thought I just pasted it! I was looking for that file during this fix 😅 thanks for pointing it out to me. I also just went ahead and used the global image you pointed out in a previous comment to replace all these individual page-specific assets, so it'll be more uniform going forward :)

Copy link
Contributor

@janbrasna janbrasna Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏 Praise! Thanks, lovely. 💟 Those assets linked between page versions that have nothing common are always in risk being inadvertently removed breaking something unrelated (here, here, me, me…) so this was def on my backlog to make more universal, thanks for taking care of it going few versions back (and getting the correct dimensions! I know the local assets were not the exact right size all the time…)

(Also: nice svgo cleanup job on all the remaining assets! 🚀)

bedrock/firefox/templates/firefox/welcome/base.html Outdated Show resolved Hide resolved
Copy link
Member

@craigcook craigcook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks good to me, nice work! r+ but it sounds like they want to review on a demo server before we merge? Though I'm not sure what needs review, we can't really make copy changes since it's all translated already... but we can merge as soon as we get signoff.

@reemhamz
Copy link
Contributor Author

Can do -- I'll set up demos now :) !

@reemhamz reemhamz removed the Needs Review Awaiting code review label Oct 2, 2024
Copy link
Member

@alexgibson alexgibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive by note:

If they want to measure the number of actual installs this page generates (which I'm assuming they do), then we probably want to load the stub attribution script on welcome pages, by removing this override:

https://github.com/mozilla/bedrock/blob/main/bedrock/firefox/templates/firefox/welcome/base.html#L59-L60

@alexgibson
Copy link
Member

Sound like you can ignore my comment above, they’re just gonna rely on Firefox version update metrics instead 👍

@craigcook craigcook merged commit b720f8e into main Oct 7, 2024
7 checks passed
@craigcook craigcook deleted the moments-root-one branch October 7, 2024 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Frontend HTML, CSS, JS... client side stuff Review: XS Code review time: up to 30min
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants