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.

Sign up for GitHub

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

Jump to bottom

Documented xlink:href on SVG fallback image is… missing… #1299

Closed
36degrees opened this issue Apr 26, 2019 · 1 comment · Fixed by #1337
Closed

Documented xlink:href on SVG fallback image is… missing… #1299

36degrees opened this issue Apr 26, 2019 · 1 comment · Fixed by #1337
Assignees
@36degrees
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation) ⚠️ high priority Used by the team when triaging 🕔 hours A well understood issue which we expect to take less than a day to resolve.

Comments

@36degrees
Copy link
Contributor

In the header component, we talk about the importance of the (empty) xlink:href attribute in the fallback <image> tag:

govuk-frontend/src/components/header/template.njk

Lines 35 to 39 in 8370f97

The <image> element is a valid SVG element. In SVG, you would specify
the URL of the image file with the xlink:href – as we don't reference an
image it has no effect. It's important to include the empty xlink:href
attribute as this prevents versions of IE which support SVG from
downloading the fallback image when they don't need to.

However, we don't actually include that attribute:

govuk-frontend/src/components/header/template.njk

Line 43 in 8370f97

<image src="{{ params.assetsPath | default('/assets/images') }}/govuk-logotype-crown.png" class="govuk-header__logotype-crown-fallback-image"></image>

🤔

We do include the xlink:href attribute in the Design System where the header component originated from – it looks like it was omitted when the component was copied in #695.

We should double check whether this attribute actually does have the effect we say it does (https://css-tricks.com/a-complete-guide-to-svg-fallbacks/ suggests it does), and then either reintroduce the missing attribute or update the documentation.
@36degrees 36degrees added awaiting triage Needs triaging by team 🐛 bug Something isn't working the way it should (including incorrect wording in documentation) labels Apr 26, 2019
@NickColley
Copy link
Contributor

NickColley commented Apr 29, 2019 •
edited
Loading

The article suggests that out current implementation does work, but without it, it will download the logo unnecessarily:

"In most browsers, therefore, it is sufficient to include an tag with a src attribute (pointing to your fallback image) inside your inline SVG: the old browsers will download the fallback, the new browsers won't. Except for Internet Explorer, which downloads the fallback image even when it doesn't display it. The solution is to put a null xlink:href attribute on the element. The IE developer tools still show it requesting the fallback, but it aborts almost immediately (<1ms in IE11 or IE10/IE9 emulation mode), before downloading anything. "

@timpaul timpaul added 🕔 hours A well understood issue which we expect to take less than a day to resolve. ⚠️ high priority Used by the team when triaging and removed awaiting triage Needs triaging by team labels May 8, 2019
36degrees added a commit that referenced this issue May 13, 2019
@36degrees
Set xlink:href on crown PNG fallback
84485f5 
When the header component was originally created in the Design System, we included the xlink:href attribute [1] to ensure that Internet Explorer did not download both the SVG and the PNG:

> "In most browsers, therefore, it is sufficient to include an <image> tag with a src attribute (pointing to your fallback image) inside your inline SVG: the old browsers will download the fallback, the new browsers won't. Except for Internet Explorer, which downloads the fallback image even when it doesn't display it. The solution is to put a null xlink:href attribute on the element. The IE developer tools still show it requesting the fallback, but it aborts almost immediately (<1ms in IE11 or IE10/IE9 emulation mode), before downloading anything." [2]

When the component was moved to GOV.UK Frontend, this attribute was omitted[3]. This adds it back again.

Fixes #1299

[1]: https://github.com/alphagov/govuk-design-system/blob/ffdb3e7bdd5ce2c42dfe95979f9faa308d75a0d9/views/partials/_header.njk#L31
[2]: https://css-tricks.com/a-complete-guide-to-svg-fallbacks/#fallback-inline-svg-imgtag
[3]: 8da2cb3
@36degrees 36degrees mentioned this issue May 13, 2019
Merged
@36degrees 36degrees self-assigned this May 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@36degrees 36degrees

Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation) ⚠️ high priority Used by the team when triaging 🕔 hours A well understood issue which we expect to take less than a day to resolve.
Projects
GOV.UK Design System cycle board
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Set xlink:href on crown PNG fallback alphagov/govuk-frontend
3 participants
@36degrees @timpaul @NickColley