Problem/Motivation

The polyfill added in #1077878: Add HTML5shiv to core is no longer needed by any of the browsers supported by Drupal core.

Proposed resolution

Remove the usage of html5shiv in core and mark the asset library deprecated in core.libraries.yml.

Alternatively, just mark the html5shiv asset library deprecated and continue the work in #1993334: Add HTML5shiv to Stable and Classy only

Remaining tasks

  1. Decide on approach
  2. Write patch

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#7 interdiff_2-7.txt7.37 KBzrpnr
#7 3086379-7.patch1.52 KBzrpnr
#2 3086379-2.patch7.71 KBzrpnr

Comments

zrpnr created an issue. See original summary.

zrpnr’s picture

zrpnr
he/him
Primary language English
Location UTC-7
commented
Status: Active » Needs review
StatusFileSize
new 3086379-2.patch7.71 KB

This patch marks html5shiv deprecated and removes it from core. The deprecation message points to the draft change record.
It's similar to the work in #1993334: Add HTML5shiv to Stable and Classy only but without adding anything to core themes, as well as some changes to the AjaxPageStateTest. I chose different libraries to test for since domready should be removed in #3086375: Deprecate domready and remove usages in core.

The assertions there are also updated to not use deprecated methods and I moved the non-functional second arguments to inline comments.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

zrpnr’s picture

zrpnr
he/him
Primary language English
Location UTC-7
commented

I added a contrib module to replace this library, on recommendation from @xjm and @lauriii
https://www.drupal.org/project/html5shiv

This adds the exact library that is removed from core in this patch for any sites that still need it.

johnwebdev’s picture

johnwebdev commented

I added a contrib module to replace this library

Anything blocking there to be released as stable?

It would be nice to see the change record updated, pointing to the contrib and some simple instructions what you need to do.

Code looks good!

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

The module looks good. Let's release a stable version of that 👏

zrpnr’s picture

zrpnr
he/him
Primary language English
Location UTC-7
commented
Version: 8.9.x-dev » 8.8.x-dev
StatusFileSize
new 3086379-7.patch1.52 KB
new interdiff_2-7.txt7.37 KB

Anything blocking there to be released as stable?

I just was hoping to have some other eyes on it before I made a release. Thanks for reviewing!
I'll update the CR to point to the contrib module as well.

After talking with @lauriii again here is a new patch which leaves html5shiv in place in core but simply marks the library deprecated and suppresses the warning. Then it can be more safely removed in 9.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs review » Reviewed & tested by the community

Reviewed code in the module and tested it manually. There are some very unlikely edge cases where the module might not work as expected, most notably if someone is not rendering the html render element but they depended on the core/html5shiv library, they wouldn't get the polyfill at all. However, given how unlikely that would be, I'd just keep the code as it is.

The patch looks good as well and will remove the usages of the deprecated library in Drupal 9. 🚢

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +Needs release manager review

  • xjm committed df9fd06 on 9.0.x 
    Issue #3086379 by zrpnr, lauriii, johndevman, xjm: Deprecate html5shiv...

  • xjm committed e20ff5e on 8.9.x 
    Issue #3086379 by zrpnr, lauriii, johndevman, xjm: Deprecate html5shiv...

  • xjm committed 58a2d2d on 8.8.x 
    Issue #3086379 by zrpnr, lauriii, johndevman, xjm: Deprecate html5shiv...
xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs release manager review +8.8.0 release notes

Committed and pushed to 9.0.x, 8.9.x, and 8.8.x. Thanks!

xjm’s picture

xjm
she/her
Primary language English
commented

Published the CR.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.