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

Unnecessary CSS is included in IE8 stylesheet #3231

Closed
Tracked by #2621
querkmachine opened this issue Jan 31, 2023 · 2 comments
Closed
Tracked by #2621

Unnecessary CSS is included in IE8 stylesheet #3231

querkmachine opened this issue Jan 31, 2023 · 2 comments
Labels
awaiting triage Needs triaging by team tech debt

Comments

@querkmachine
Copy link
Member

querkmachine commented Jan 31, 2023

Cause

Unnecessary CSS, such as repeated style rules and features that are not supported by Internet Explorer 8, is still being included in the IE8-specific stylesheet, despite efforts to automatically remove these features.

Noticed during the 4.5.0 release process.

Repeated rules

Rules are repeated in several places where media queries have been removed. postcss-unmq should be removing the duplicates and only including the rules relevant to the configured screen dimensions (by default, 1024×768)—however this doesn't appear to be happening.

The presence of both rem and px unit versions is due to node-pixrem. Having the rem unit versions is unnecessary in the IE8 stylesheet, too.

.govuk-\!-font-size-19 {
    font-size: 16px!important;
    font-size: 1rem!important;
    line-height: 1.25!important;
    font-size: 19px!important;
    font-size: 1.1875rem!important;
    line-height: 1.31579!important;
}

Unsupported rules

Unsupported CSS rules are still included, such as the flexbox-related rules here.

.govuk-button-group {
    margin-bottom: 15px;
    display: flex;
    flex-direction: column;
    align-items: center;
    margin-right: -15px;
    flex-direction: row;
    flex-wrap: wrap;
    align-items: baseline;
}

And box-shadow and box-decoration-break rules here:

.js-enabled .govuk-accordion__show-all:focus {
    color: #0b0c0c;
    background-color: #fd0;
    box-shadow: 0 -2px #fd0,0 4px #0b0c0c;
    text-decoration: none;
    box-decoration-break: clone;
}

Unsupported selectors

Selectors incorporating the :not selector are included in the stylesheet, despite not being supported by IE8.

.govuk-link--no-underline:not(:hover):not(:active) {
    text-decoration: none;
}

Same for :last-child and :last-of-type here.

.govuk-checkboxes__item:last-child,
.govuk-checkboxes__item:last-of-type {
    margin-bottom: 0;
}

Unsupported functions

Several CSS functions not supported by IE8 are still included. In this example, none of calc(), env() or max() are supported in IE8.

@supports (margin:max(calc(0px))) {
    .govuk-width-container {
        margin-right: max(15px,calc(15px + env(safe-area-inset-right)));
        margin-left: max(15px,calc(15px + env(safe-area-inset-left)));
        margin-right: max(30px,calc(15px + env(safe-area-inset-right)));
        margin-left: max(30px,calc(15px + env(safe-area-inset-left)));
    }
}

Third-party browser prefixes

Some browser-prefixed properties and values are still included, despite not being relevant to IE8.

.govuk-link {
    font-family: GDS Transport,arial,sans-serif;
    -webkit-font-smoothing: antialiased;
    -moz-osx-font-smoothing: grayscale;
    text-decoration: underline;
}

@supports at-rule

@supports at-rules are included in the stylesheet, despite IE8 not supporting them.

@supports (content-visibility:hidden) {
    .js-enabled .govuk-accordion__section-content[hidden] {
        content-visibility: hidden;
        display: inherit;
    }
}

Consequences

We ship significantly more CSS to IE8 than the browser is capable of using. This may have an exacerbated effect on performance as any contemporary IE8 users are probably running very old hardware and software.

Impact of debt

Low

Reason

Considered as low because:

  • There are very, very few IE8 users around nowadays.
  • We intend to drop IE8 support in the near future.
  • Unsupported CSS features should be ignored by IE8, though there is no guarantee that the rules fail gracefully, especially compound selectors where only part of the selector is supported.

Effort to pay down

High

Reason

Some of these issues could be resolved manually by wrapping the offending code in the govuk-not-ie8 mixin, so that it's not included in the IE8 stylesheet.

For others: We currently use the Oldie plugin to automatically remove unsupported features in the IE8 stylesheet. Oldie appears to be unmaintained, with issues and pull requests not being monitored. We would likely have to fork the plugin or its dependencies if we wanted to make updates to how it works.

Despite us planning to drop support for IE8 going forward, we will still be supporting it on a security and major bugfix basis for at least a year following the release of Frontend v5. We may want to pay down some of this debt so that things are in a better place before we reduce and drop IE8 support completely.

Overall rating

Low

@querkmachine querkmachine added awaiting triage Needs triaging by team tech debt labels Jan 31, 2023
@36degrees
Copy link
Contributor

I think there are a couple of things that are going on in our codebase that are worth pointing out as they are somewhat hidden / confusing:

Rules are repeated in several places where media queries have been removed. postcss-unmq should be removing the duplicates and only including the rules relevant to the configured screen dimensions (by default, 1024×768)—however this doesn't appear to be happening.

We're using a vendored version of sass-mq under the hood – wherever you see the govuk-media-query mixin being used, under the hood that's calling sass-mq:

@mixin govuk-media-query($args...) {
@include mq($args...) {
@content;
}
}

One of the features of sass-mq is that it 'rasterizes' media queries – so wherever govuk-media-query is used and the media query matches our desktop breakpoint, it'll be outputted without being wrapped in a media query:

// Responsive support is disabled, rasterize the output outside @media blocks
// The browser will rely on the cascade itself.
@if $responsive == false {
$static-breakpoint-width: mq-get-breakpoint-width($static-breakpoint, $breakpoints);
$target-width: mq-px2em($static-breakpoint-width);
// Output only rules that start at or span our target width
@if (
$and == false
and $min-width <= $target-width
and (
$until == false or $max-width >= $target-width
)
and $media-type != 'print'
) {
@content;
}
}

There's nothing intelligent going on to remove duplicates and – because this happens as part of the Sass compilation step – there's no media query left for unmq to do anything with.

We do have a few other places in the codebase where we use @media rules directly instead of calling govuk-media-query (not for breakpoints – typically used for things like @media (forced-colors: active) or @media (hover: none). Because these don't go through the mixin, they are included in the CSS and this is where unmq does help as it removes them for us.

The presence of both rem and px unit versions is due to node-pixrem. Having the rem unit versions is unnecessary in the IE8 stylesheet, too.

.govuk-\!-font-size-19 {
    font-size: 16px!important;
    font-size: 1rem!important;
    line-height: 1.25!important;
    font-size: 19px!important;
    font-size: 1.1875rem!important;
    line-height: 1.31579!important;
}

To the best of my knowledge, we've never used node-pixrem for this. Our typography scale is defined in px, and our typography helpers convert them to rem and output both font-size declarations:

font-size: $font-size;
@if $govuk-typography-use-rem {
font-size: $font-size-rem;
}

There's a flag to enable this ($govuk-typography-use-rem). If we wanted to, we could update the govuk-typography-responsive mixin to consider both flags (@if $govuk-typography-use-rem and not $govuk-is-ie8 {) or consider $govuk-is-ie8 when setting the default value for $govuk-typography-use-rem.

@querkmachine
Copy link
Member Author

Closing as we're now starting work on GOV.UK Frontend 5, which drops support for IE8.

@querkmachine querkmachine closed this as not planned Won't fix, can't repro, duplicate, stale Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting triage Needs triaging by team tech debt
Projects
None yet
Development

No branches or pull requests

2 participants