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

Stop IE8 from downloading GDS Transport font #1559

Merged
merged 3 commits into from
Sep 9, 2019
Merged

Conversation

hannalaakso
Copy link
Member

@hannalaakso hannalaakso commented Sep 6, 2019

Internet Explorer 8 was making unsuccessful requests for the new font files because 3.0.0 no longer postfixes them with ?#iefix as we did in v2.

3.0.0 dropped support for rendering GDS Transport for IE8. This PR removes the font-face declarations from the IE8 stylesheet to stop IE8 trying to unsuccessfully to download the font files. This means that IE8 will fall back to system fonts (Arial by default). Our previous approach of postfixing the paths with ?#iefix wouldn't work here as IE8 would still download the redundant WOFF files.

The downside of this new approach is that GDS Transport is still shown in font-family in the IE8 stylesheet.

I've also tried an alternative approach that sets font-family for IE8 on a higher level and removes the above issue, but has other downsides.

This also imports the IE8 tools for govuk-typography-common tests, and tests that font-face is not declared for IE8.

Fixes #1550

The font paths were breaking in IE8 as we since v3 we're no longer postfixing
them with `?#iefix`.

As we removed the EOT files in v3 to stop serving GDS Transport font files to IE8 (see ),
this commit removes the font faces from the IE8 stylesheet. This means that IE8 will fall
back to system fonts, in this case Arial or sans-serif.

Our previous approach of postfixing `?#iefix` wouldn't work here as IE8 would end up
downloading the WOFF files which it cannot render.

The downside of this approach is that the IE8 stylesheet still says
```
font-family: "GDS Transport", Arial, sans-serif;
```
which could be confusing.

An alternative would have been define a new font family for IE8 at the `settings` level.
See https://github.com/alphagov/govuk-frontend/compare/fix-ie8-fonts-alt-approach

This also imports the IE8 tools for testing `govuk-typography-common`.
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1559 September 6, 2019 15:15 Inactive
@@ -130,6 +131,28 @@ describe('@mixin govuk-typography-common', () => {
expect(resultsString).toContain(`font-family: "GDS Transport"`)
expect(resultsString).toContain(`font-family: "GDS Transport"`)
})
it('should not output a @font-face declaration when the browser is IE8', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice test 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

💯

Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

Seems like a sensible approach to me – top stuff @hannalaakso 👍

@hannalaakso hannalaakso marked this pull request as ready for review September 9, 2019 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

IE8 causes 404 errors when using Frontend 3.0
4 participants