search

mozilla / fx-private-relay

Keep your email safe from hackers and trackers. Make an email alias with 1 click, and keep your address to yourself.
https://relay.firefox.com
Other
1.44k stars 168 forks source link

MPP-3835: Workaround next/image's inline style for SVG images #4820

Closed jwhitlock closed 4 days ago

jwhitlock commented 4 days ago

Next.js's Image from next/image adds an inline style style="color:transparent" to some images, such as the logo image:

<img
  alt=""
  loading="lazy"
  width="42"
  height="44"
  decoding="async"
  data-nimg="1"
  style="color:transparent"
  src="/_next/static/media/relay-logo.cabccc81.svg"
/>

This is a known issue, with a good explanation by an issue writer at https://0xdbe.github.io/NextJS-CSP-NextImage/ .

Since we do not use image optimization, we can replace this with an <img /> element. This PR extracts the alt text and image parameters from the <Image> declaration, turns an inline style="color:transparent" into a className with an equivalent CSS rule, and returns the new <img /> element.

How to test:

To see the current behavior In main or PR #4819, run cd frontend; npm run build. Look for <img or style="color:transparent" in that document. If desired, make a copy for diff later.

In Firefox, open the developer tools, console, and load / reload https://relay.firefox.com/. See many Content-Security-Policy violations.

To see the new behavior In this branch, run cd frontend; npm run build. The string "color:transparent" should not be in that document. Some of the <img tags will have a new className entry, like Image_transparent__sZTo9. A visual diff with the old version should show that the alt= tag is still present, and the data-nimg="1" element is dropped.

jwhitlock commented 4 days ago

Wondering if we should install sharp or try and remove the warning, as we're not using Image Optimization.

I read https://nextjs.org/docs/messages/sharp-missing-in-production, but I don't see a way to remove the warning, or an existing issue. This will need some more research. It may be worth installing sharp and seeing if it changes the SVGs at all, and if it is compatible with this change. I'll spend a little time on this before merging.

jwhitlock commented 4 days ago

The output is identical with sharp installed, which is what we'd expect since we've disabled optimization. Let's ignore the warning for now, and maybe revisit disabling after the next.js 15 upgrade.