Make WordPress Core

Opened 19 months ago

Closed 10 months ago

Last modified 3 months ago

#56699 closed enhancement (fixed)

Remove IE specific checks from default themes

Reported by: desrosj's profile desrosj Owned by: audrasjb's profile audrasjb
Milestone: 6.3 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch has-dev-note
Focuses: Cc:

Description

In several default themes, there are a number of different things targeting old browsers:

  • stylesheets targeting specific versions of IE
  • an HTML5 shiv targeting IE 6-9, Safari 4.x, and Firefox 3.x.

Support for IE < 11 was dropped in WordPress 4.8 (2017). Safari 4.x and Firefox 3.x has not been supported for quite a bit longer (the auto-prefixing in Grunt.js was introduced almost 10 years ago in [27078] with support for Firefox >= 17 and Safari >= 6.0).

While it can be argued that themes were designed to support a specific set of browsers and that should not change as time goes on, the following data about browser usage should make it clear why this continuing to include these files is unnecessary:

Themes affected:

  • Twenty Eleven
  • Twenty Twelve
  • Twenty Thirteen
  • Twenty Fourteen
  • Twenty Fifteen
  • Twenty Sixteen
  • Twenty Seventeen

Attachments (3)

twentyelevenchild.zip (415.3 KB) - added by sabernhardt 15 months ago.
child theme to test Twenty Eleven
56699.twentyseventeen.patch (711 bytes) - added by sabernhardt 10 months ago.
use wp_register_script in Twenty Seventeen
child-themes-13-15-17.zip (602.3 KB) - added by sabernhardt 9 months ago.
Child themes used to test Twenty Thirteen, Twenty Fifteen and Twenty Seventeen

Download all attachments as: .zip

Change History (51)

#1 @sabernhardt
19 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 6.2

Related: #46015

#2 @desrosj
19 months ago

One thing I forgot to mention.

It's possible that child themes are registering their own styles and scripts with these as dependencies. Removing these handles will result in those no longer being enqueued.

There are two approaches that can be taken here:

  • Continue to enqueue those scripts/styles in the default theme without specifying a source. This will make the script/style handle an alias, and the scripts/styles will continue to load.
  • Just remove the handles entirely. The scripts and styles outside of the default themes will no longer be loaded. But this could be OK because they are targeting old browsers anyway.

A __doing_it_wrong() or deprecated notice could be used here in some way.

There's also the possibility that themes are referencing these files directly. Removing them could result in 404 errors. I think this is likely a lower risk than the first issue I detailed, but still worth considering.

#3 follow-up: @sabernhardt
19 months ago

Option 3: Switch the wp_enqueue_ functions to wp_register_ for any IE-related files.

  • Maintains the handles, source references, etc.
  • Keeps all IE-specific files within the theme package to avoid errors.
  • Requires site admins to opt in to loading them in the theme.

This ticket was mentioned in PR #3474 on WordPress/wordpress-develop by @neychok.


19 months ago
#4

  • Keywords has-patch added; needs-patch removed

Removed CSS, JS and other code related to IE<10

Trac ticket: https://core.trac.wordpress.org/ticket/56699

#5 @neychok
19 months ago

Hey guys, I checked the TwentyEleven theme and we don't actually enqueue any IE-related assets (or at least I didn't see such) rather we directly add it with a script tag in the header.

I wasn't sure what to do in this case so completely removed all IE-related code from the theme in the PR I just submitted.

I am willing to work on this task but I am not entirely sure how to proceed.

Should I:

  • Completely remove all IE code
  • Just change wp_enqueue_ to wp_register_ where possible and leave the rest, like in the case with TwentyEleven
  • Remove all IE code except where we have wp_enqueue_ that's going to be changed to wp_register_ instead.

The PR is just an example.

Last edited 19 months ago by neychok (previous) (diff)

This ticket was mentioned in Slack in #accessibility by sabernhardt. View the logs.


18 months ago

#7 in reply to: ↑ 3 @audrasjb
16 months ago

  • Keywords needs-patch added; has-patch removed

Replying to sabernhardt:

Option 3: Switch the wp_enqueue_ functions to wp_register_ for any IE-related files.

  • Maintains the handles, source references, etc.
  • Keeps all IE-specific files within the theme package to avoid errors.
  • Requires site admins to opt in to loading them in the theme.

Yes this is definitely our best option yet 👍
This way, we don't introduce any serious backwards compatibility issue to existing child themes.

@neychok thanks for the T11 PR!

Re-adding needs-patch as we'll need a patch for each bundled theme :)

#8 @sabernhardt
15 months ago

  • Milestone changed from 6.2 to Future Release

#57031 suggested the 6.3 cycle for removing other IE browser support, and I would like both changes announced in the same news post and/or dev note.

For the Twenty Eleven patch:

  1. If we only make the changes to Twenty Eleven's header.php template file, that should minimize what breaks. If some people really want to maintain support in their sites, then they could do that with a different header template.
  2. We could be somewhat selective with the proposed CSS revisions. WordPress admin has not supported older IE versions for years, so editor-style.css should not need to accommodate them. Also, the gradients' solid color fallbacks would make their absence very minor. And the negative padding values probably never worked (#46771).
  3. Removing the other IE-related CSS could reduce the front-end stylesheets by about 1 KB. The difference is small enough to consider keeping those for possible continued use in child themes (which I expect would be very rare).
  4. To avoid any 404 errors, please _keep a file_ at js/html5.js. The script is not particularly large at about 2.4 KB, so it could stay intact. If it is worth removing from the package, however, its contents could be replaced with a single-line comment such as "This theme does not support old versions of Microsoft Internet Explorer anymore."

@sabernhardt
15 months ago

child theme to test Twenty Eleven

#9 @poena
13 months ago

  • Keywords needs-testing added
  • Milestone changed from Future Release to 6.3

This ticket was mentioned in Slack in #slackhelp by ramsesdelr. View the logs.


12 months ago

This ticket was mentioned in Slack in #core by oglekler. View the logs.


10 months ago

#12 @oglekler
10 months ago

This ticket was discussed in the recent bug scrub.

Hi @neychok this is an enhancement, and it needs to have separate patches to all supported bundled themes. Because you have already done the patch, it will be easier for you to make other patches as well. We are close to the Beta 1 (12 days left) and after it, no enhancement can be committed. So, please consider making this patches, or possibly you know another contributor who can follow your footsteps.

Thank you 🙏

Note for testers: child themes need to be created to each of these parent themes.

Additional props to: @hareesh-pillai

Last edited 10 months ago by sabernhardt (previous) (diff)

This ticket was mentioned in PR #4642 on WordPress/wordpress-develop by @metodiew.


10 months ago
#13

  • Keywords has-patch added; needs-patch removed

Removing IE specific checks from Twenty Twelve default theme. Keeping the js/html5.js as per the note from this comment

Working with @Neychok on the ticket, so we can split the work on the default themes.

Trac ticket: 56699

@metodiew commented on PR #4642:


10 months ago
#14

Thanks for the feedback @sabernhardt ! Applied the notes and changes, can you take a look is there anything else? if not, I'll open a new PR for the other themes, following the ticket updates. Thanks in advance!

This ticket was mentioned in PR #4644 on WordPress/wordpress-develop by @metodiew.


10 months ago
#15

Remove IE specific checks from default themes in Twenty Fourteen

Trac ticket: 56699

This ticket was mentioned in PR #4658 on WordPress/wordpress-develop by @metodiew.


10 months ago
#16

Remove IE specific checks from default themes in Twenty Sixteen

Trac ticket: 56699

This ticket was mentioned in PR #4659 on WordPress/wordpress-develop by @neychok.


10 months ago
#17

Removed IE code from header.php
Removed IE code from editor-style.css
Removed negative paddings and gradient IE code from style.css and dark.css

This ticket was mentioned in PR #4660 on WordPress/wordpress-develop by @neychok.


10 months ago
#19

Replaced html5.js with a comment.
De-enqueued ie.css
Removed IE specific code from header.php

This ticket was mentioned in PR #4661 on WordPress/wordpress-develop by @neychok.


10 months ago
#20

Removed IE specific code from Customizer, header.php and custom-header.php
Dequeued IE css.
Replaced HTML5.js

This ticket was mentioned in PR #4662 on WordPress/wordpress-develop by @neychok.


10 months ago
#21

Removed IE specific code from style.css
Dequeued IE specific CSS files.
Replaced html5.js with a comment.

#22 @neychok
10 months ago

I think we are ready with the patches.

I've been selective with removing CSS as mentioned in this comment https://core.trac.wordpress.org/ticket/56699#comment:8

#23 @audrasjb
10 months ago

  • Keywords needs-dev-note added
  • Owner set to audrasjb
  • Status changed from new to reviewing

Thanks @neychok for all the patches!
I'll review (and hopefully) commit each of them individually.

Adding needs-dev-note so we can tell people that IE stylesheets are registered but not enqueued anymore.

#24 @audrasjb
10 months ago

In 55980:

Twenty Seventeen: Remove IE specific resources.

This changeset switches the wp_enqueue_* functions to wp_register_* for IE-related resources, which maintains handles, source references, etc., keeps all
IE-specific files within the theme package to avoid errors, and requires site admins to opt in to loading them in the theme.

It also replace the content of html5.js shiv with a comment (to avoid 404s) and removes IE-specific code in general stylesheets.

Props desrosj, sabernhardt, audrasjb, neychok, oglekler.
See #56699.

#26 @audrasjb
10 months ago

In 55981:

Twenty Fifteen: Remove IE specific resources.

This changeset switches the wp_enqueue_* functions to wp_register_* for IE-related resources, which maintains handles, source references, etc., keeps all
IE-specific files within the theme package to avoid errors, and requires site admins to opt in to loading them in the theme.

It also replaces the content of html5.js shiv with a comment (to avoid 404s), removes IE-specific code in general stylesheets, and removes IE specific code
from Customizer, header.php and custom-header.php.

Props desrosj, sabernhardt, audrasjb, neychok, oglekler.
See #56699.

#28 @audrasjb
10 months ago

In 55982:

Twenty Thirteen: Remove IE specific resources.

This changeset switches the wp_enqueue_* functions to wp_register_* for IE-related resources, which maintains handles, source references, etc., keeps all
IE-specific files within the theme package to avoid errors, and requires site admins to opt in to loading them in the theme.

It also replaces the content of html5.js shiv with a comment (to avoid 404s), and removes IE specific code from header.php.

Props desrosj, sabernhardt, audrasjb, neychok, oglekler.
See #56699.

#30 @audrasjb
10 months ago

@sabernhardt Looks like I missed your comment on the first PRs:

If replacing the code in html5.js is good, then all the IE-specific CSS could be removed. It would be highly difficult for someone to restore old IE support, if anyone feels the need, though I do not expect people would go through extra effort for that.

So you recommend to restore the content of html5.js for all themes, right?

#31 @sabernhardt
10 months ago

Maybe we should restore the shiv code, plus the credit in Twenty Seventeen's readme file.

Editing html5.js seems appropriate enough for the themes that are already modified, though any sites that might "opt in" to load it with the stylesheet(s) would need the code. The ticket summary says to remove the checks, not necessarily all the related code.

#54421 purposely allowed sites to restore the skip link script; however, with this ticket, restoring the older IE code could be more difficult and less valuable for most sites.

I had started a PR with limited changes yesterday to consider that option before making a decision. To maintain or restore IE support with those sets of changes, site owners would need to:

  1. Keep the conditional code from Twenty Eleven/Twelve/Thirteen/Fourteen, most likely with their own header.php file in a child theme. (If they already have the template, this part would be completed.)
  2. Enqueue any registered stylesheet(s) and/or script that had been enqueued earlier. (Theoretically, sites could have added the styles and script separate from the parent, either by printing directly or by enqueuing another stylesheet with the IE styles as a dependency.)

The Twenty Eleven child theme example would have no difference (on the front end) with only the limited changes for that theme. Removing styles and/or the script code from themes would require finding what was removed and adding it somewhere just to continue supporting the old browsers at the same level.

On the other hand, the files would be lighter and/or cleaner without the old code. I do not expect IE<10 support to be anyone's priority, and I even think Twenty Fifteen through Twenty Seventeen had lost IE8 support with the jQuery update three years ago. [55981] dropped legacy support for custom colors and [55980] removed gradient filters, which I think are acceptable changes for the one visit out of maybe more than 1,500.

For the Twenty Eleven commit: changes on this ticket should also fix #46771.

#32 @azaozz
10 months ago

Just a note that this enhancement is committed already. Code can be fixed or refined as needed during beta.

This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.


10 months ago

#34 @audrasjb
10 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

Marking as fixed then reopening, just to facilitate triage.

#35 @audrasjb
10 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#36 @metodiew
10 months ago

hey @audrasjb and team, let us know if there is anything we have to do for the remaining open pull requests. If there are any other changes we have to apply, happy do so, so we can finalize the ticket.

#37 @audrasjb
10 months ago

In 56147:

Twenty Fifteen: Reintroduce HTML5 Shiv.

This reintroduces HTML5 Shiv script that was removed in [55981], to ensure backward compatibility for websites that opted-in to IE support.

Follow-up to [55981]

Props audrasjb, sabernhardt.
See #56699.

#38 @audrasjb
10 months ago

In 56148:

Twenty Thirteen: Reintroduce HTML5 Shiv.

This reintroduces HTML5 Shiv script that was removed in [55982], to ensure backward compatibility for websites that opted-in to IE support.

Follow-up to [55982].

Props sabernhardt.
See #56699.

#39 @audrasjb
10 months ago

In 56149:

Twenty Seventeen: Reintroduce HTML5 Shiv.

This reintroduces HTML5 Shiv script that was removed in [55980], to ensure backward compatibility for websites that opted-in to IE support. Also reintroduces
HTML5 credits.

Follow-up to [55980].

Props sabernhardt.
See #56699.

#40 @audrasjb
10 months ago

  • Keywords needs-testing removed

@sabernhardt HTML5 Shiv code has been reintroduced in the above commits. Anything else needed for milestone 6.3?

@sabernhardt
10 months ago

use wp_register_script in Twenty Seventeen

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


10 months ago

#42 @audrasjb
10 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 56188:

Twenty Seventeen: Use wp_register_script() to register HTML5 Shiv script.

This replaces wp_register_style() with wp_register_script().

Props sabernhardt.
Fixes #56699.

#43 @sabernhardt
9 months ago

If anyone would like to review a dev note that mentions the three themes edited for this release cycle, I started a post draft (also in a Google doc if you prefer that).

#44 @audrasjb
9 months ago

Thanks for the dev note! It looks 100% ok to me 👍

#45 @metodiew
9 months ago

Looking at the latest comments and the movements, I would assume my patches for TwentyTwelve, TwentyFourteen, and TwentySixteen won't go with this release?

#46 @sabernhardt
9 months ago

Keep the PRs open. We can create a follow-up ticket for the other themes in the next cycle.

Version 0, edited 9 months ago by sabernhardt (next)

@sabernhardt
9 months ago

Child themes used to test Twenty Thirteen, Twenty Fifteen and Twenty Seventeen

#47 @sabernhardt
9 months ago

To share the child themes I used for testing these changes, this set includes four variations for each parent theme:

  • Using a custom header.php template that nearly matches the parent template (Twenty Thirteen's has a valuable edit but the others simply remove the XFN link).
  • Calling scripts and styles as dependencies of other resources.
  • Enqueuing or printing the IE scripts and styles with the parent theme's URLs.
  • Trying to restore support by enqueuing the scripts and styles and/or including IE conditional classes.
Note: See TracTickets for help on using tickets.