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

Change fallback method for SVG images #39

Merged
merged 2 commits into from
May 7, 2021
Merged

Conversation

tombye
Copy link
Contributor

@tombye tombye commented May 6, 2021

Since adding a Content Security Policy, we're getting violation errors due to the use of data URIs in our page. This is because of the fallback method we use for our SVG images.

Content Security Policy: The page's settings blocked the loading of a resource at data:, ('default-src').

The current fallback method works like this:

  • we put a <image> tag in the SVG that uses an empty data URI for the image data and the SVG attribute of display="none" so it doesn't show up in browsers that support SVG
  • the <image> tag includes a src attribute, pointing to a fallback image

Because browsers interpret <image> as <img>, those that don't support SVG:

  • ignore all the SVG tags
  • interpret <image> as an <img> tag and display the image its src points to

The problem with this is that using data URIs violates our Content Security Policy.

The new fallback method is to just wrap SVGs in conditional comments that ensure it is only delivered to Internet Explorer (IE) 8 and above, as well as all other browsers. This does mean that any browsers that don't support SVG but aren't IE will no
longer get the fallback but this seems limited to quite old versions of Android's native browser:

https://caniuse.com/svg

The latest one of these was released in 2010 so I think this risk is reasonable.

These changes extend the interface of the alerts_icon component to allow the conditional comments to be optional. This is because we nest it in the example component, which is wrapped in conditional comments itself so any child SVGs it
includes don't need them.

Note: The very latest GOVUK Frontend also contains this issue but it has been raised with the team and is scheduled to be worked on soon:

alphagov/govuk-frontend#2203

We don't use this version yet so can hold off until they roll out a solution (which we may yet copy).

@tombye tombye force-pushed the fix-data-uri-csp-violations branch from 78f54f9 to 0efbce9 Compare May 6, 2021 10:41
The current fallback method works like this:
- we put a `<image>` tag in the SVG that uses an
  empty data URI for the image data and the SVG
  attribute of `display="none"` so it doesn't
  show up in browsers that support SVG
- the `<image>` tag includes a `src` attribute,
  pointing to a fallback image

Because browsers interpret `<image>` as `<img>`,
those that don't support SVG:
- ignore all the SVG tags
- interpret `<image>` as an `<img>` tag and
  display the image its `src` points to

The problem with this is that using data URIs
violates our Content Security Policy.

The new fallback method is to just wrap SVGs in
conditional comments that ensure it is only
delivered to Internet Explorer (IE) 8 and above, as well as all
other browsers. This does mean that any browsers
that don't support SVG but aren't IE will no
longer get the fallback but this seems limited to
quite old versions of Android's native browser:

https://caniuse.com/svg

The latest one of these was released in 2010 so I
think this risk is reasonable.

These changes extend the interface of the
alerts_icon component to allow the conditional
comments to be optional. This is because we nest
it in the example component, which is wrapped in
conditional comments itself so any child SVGs it
includes don't need them.

Note: The very latest GOVUK Frontend also contains
this issue but it has been raised with the team
and is scheduled to be worked on soon:

alphagov/govuk-frontend#2203

We don't use this version yet so can hold off
until they roll out a solution (which we may yet
copy).
@tombye tombye force-pushed the fix-data-uri-csp-violations branch from 0efbce9 to a68f402 Compare May 6, 2021 10:43
Copy link
Contributor

@benthorner benthorner left a comment

Choose a reason for hiding this comment

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

Great explanation - just when I think HTML can't get any whackier it does 😜.

I'm confused why we have/had the alerts-icon__fallback class set on the example.png image, since it's not an icon. I can't actually see where that class is defined to understand what it's doing. I expected it to be in alert_icon.scss. On the other hand, it was already there, so not a big deal. What does it do?

The class doesn't match any CSS declarations so
it can only add confusion so this removes it.
@tombye
Copy link
Contributor Author

tombye commented May 6, 2021

I'm confused why we have/had the alerts-icon__fallback class set on the example.png image, since it's not an icon. I can't actually see where that class is defined to understand what it's doing. I expected it to be in alert_icon.scss.

This was actually a great spot. I added it a while ago as part of building the alerts icon component but it ended up, as you noticed, not being used. Being redundant it only adds confusion so I removed it. I assume this doesn't change your approval so please add a comment if it does.

@benthorner
Copy link
Contributor

Thanks, I thought it might be some strange CSS technique I wasn't aware of. Glad it was useful, feel free to merge 👍.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants