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

Update wordmarks and logos #15975

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

maureenlholland
Copy link
Collaborator

@maureenlholland maureenlholland commented Feb 4, 2025

If this changeset needs to go into the FXC codebase, please add the WMO and FXC label.

One-line summary

Brings in new refresh mozilla symbol where possible

NOTES:

Significant changes

Replaces root favicons with m24 favicons (names do not change, not sure if this is an issue). Also there are no m24 replacements for the 16x16 & 32x32 sizes (yet, we could add if needed)

Issue / Bugzilla link

#15682

Testing

new-card-image heart-with-flag new-copy-and-images new-image

@maureenlholland maureenlholland force-pushed the update-wordmarks-and-logos branch 5 times, most recently from ab502a1 to 6dc6046 Compare February 5, 2025 16:36
@maureenlholland

This comment was marked as resolved.

@@ -12,7 +12,7 @@
{% block body_class %}mzp-t-mozilla format-none{% endblock%}
{% block body_id %}privacy-landing{% endblock %}

{% block article_header_logo %}{{ static('img/trademarks/logo-mozm.png') }}{% endblock %}
{% block article_header_logo %}{{ static('img/trademarks/symbol.svg') }}{% endblock %}
Copy link
Collaborator Author

@maureenlholland maureenlholland Feb 5, 2025

Choose a reason for hiding this comment

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

It was suggested to use the green flag with black background here, but I've gone with the asset that exists in the trademarks folder for simplicity (https://www.mozilla.org/en-US/foundation/trademarks/list/). Should I copy the png favicon for more general use?
privacy-symbol

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure who the authority is on this - but I like what you've done.

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.

"BIMI requires SVG version of favicon"

I think BIMI doesn't work at all currently, or the set up was never really finished given the absence from DNS. (Or the verification at the TLS cert provider side was the issue? Maybe someone with access to https://bugzil.la/1738806 can check its fate.)

https://bimigroup.org/bimi-generator/ validator shows no records for:
email.mozilla.org
em.mozilla.org
e.mozilla.org
mozilla.org

TL;DR no need to put any extra effort into BIMI assets unless requested again I guess. As the SVG needs to use TinyPS profile therefore some manual checks/tweaks etc. it's an extra effort:(

<link rel="shortcut icon" href="{% block page_favicon %}{% if switch('m24-website-refresh') %}{{ static('img/favicons/mozilla/m24/favicon.ico') }}{% else %}{{ static('img/favicons/mozilla/favicon.ico') }}{% endif %}{% endblock %}">
<link rel="apple-touch-icon" type="image/png" sizes="180x180" href="{% block page_ios_icon %}{{ static('img/favicons/mozilla/apple-touch-icon.png') }}{% endblock %}">
<link rel="icon" type="image/png" sizes="196x196" href="{% block page_favicon_large %}{{ static('img/favicons/mozilla/favicon-196x196.png') }}{% endblock %}">
<link rel="shortcut icon" href="{% block page_favicon %}{{ static('img/favicons/mozilla/favicon.ico') }}{% endblock %}">
Copy link
Contributor

Choose a reason for hiding this comment

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

"Also there are no m24 replacements for the 16x16 & 32x32 sizes (yet, we could add if needed)"

They were only added in #7889/files wholesale in a batch, for consistency/availability — but never consumed since (or used before), as in #7916/files — so it's safe to omit them.

Copy link
Contributor

Choose a reason for hiding this comment

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

@maureenlholland So the TinyPS is pretty feasible for the flag shape — I've made a -bimi variant if you wanna update that one as well to… well… not keep an old asset around;)

logo-bimi.svg
logo-bimi

logo-bimi.svg.zip

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks for the SVG, gonna link this in a separate BIMI-specific issue

@maureenlholland
Copy link
Collaborator Author

waiting for stakeholder feedback

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not 100% sure this will work, esp. for CSP reasons, with a raster image embedded as a datauri; and something running as "a script" that might get sandboxed.

(WebKit makes it look like there's something broken: …)

Screenshot 2025-02-18 at 17 16 30

Should I look into it, to try to replace the inline PNG with some vector alternative?

Copy link
Contributor

Choose a reason for hiding this comment

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

(NB: the base64 datauri seems to be empty/invalid PNG anyways — was there something in the original asset perhaps? The original PNG had a Fx logo in white there on the mug…)

https://jaredwinick.github.io/base64-image-viewer/
data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAZ4AAAGeCAYAAACkfGcPAAAgAElEQVR4nO3db1bcRro/8FJJ

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

these might not be final images, so you can ignore for now, thanks for the info though! will keep an eye on it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

cleaned up the svg, if embedded data-based images are important to studio team, I'll re-request and explain the issue, but I think these are interim images to better match the refresh branding until a more comprehensive update is ready, so not high priority

@maureenlholland maureenlholland force-pushed the update-wordmarks-and-logos branch from edb0012 to 06c4f63 Compare February 18, 2025 17:25
@maureenlholland maureenlholland force-pushed the update-wordmarks-and-logos branch from 06c4f63 to cbc5fd7 Compare February 18, 2025 20:54
@maureenlholland maureenlholland added P3 Third level priority - Nice to have Needs Review Awaiting code review Review: XS Code review time: up to 30min Frontend HTML, CSS, JS... client side stuff and removed Do Not Merge ⚠️ labels Feb 18, 2025
@maureenlholland maureenlholland marked this pull request as ready for review February 18, 2025 21:08
@maureenlholland maureenlholland requested review from a team as code owners February 18, 2025 21:08
Copy link

codecov bot commented Feb 18, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.57%. Comparing base (eccbb39) to head (a7499f6).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #15975   +/-   ##
=======================================
  Coverage   79.57%   79.57%           
=======================================
  Files         160      160           
  Lines        8396     8396           
=======================================
  Hits         6681     6681           
  Misses       1715     1715           

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

@maureenlholland maureenlholland mentioned this pull request Feb 18, 2025
2 tasks
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 Needs Review Awaiting code review P3 Third level priority - Nice to have Review: XS Code review time: up to 30min
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants