Deprecate IE conditional comments support

IE 9 & 10 support was already dropped in 8.4 #2842298: [policy, no patch] Drop IE9 and IE10 support from Drupal 8.4.x, and this is probably something that the IE9 Compatibility module could maintain support for.

Maybe best to deprecate in D8, and drop in D9?

I think the errors are because classList and/or html5shiv (which use the browsers attribute, but are already deprecated) are included on almost every page. Hopefully adding to the skipped deprecation messages should improve the test failures.

I've created #3101620: Remove IE conditional comments support in Drupal 10 to handle the removal in D9.

I'm not sure why testPreRenderHtmlTag() called preRenderConditionalComments() - I think that can just be removed.
Marked testPreRenderConditionalComments() as legacy, which I think should resolve that last failure.

This looks good to me, there is no point keeping this code around if we don't support any of the browsers that can actually use it.

Created a change record https://www.drupal.org/node/3102997

----

The documentation page for adding CSS / JS to modules doesn't mention the browsers property, and the page for adding CSS / JS to themes already includes a note about conditional comments not being supported by IE10+.

Good catch! We have already passed the deadline to introduce deprecations for removal from Drupal 9. Even though we don't support IE 9 anymore, this is probably something that would make it more difficult for someone else to support IE 9. For that reason, I assume we have to deal with this just like with any other deprecations at this point and target them for removal in Drupal 10. I tagged this for release manager review since they might have thoughts on this.

1. +++ b/core/lib/Drupal/Core/Render/Element/HtmlTag.php
   @@ -112,6 +112,9 @@ public static function preRenderHtmlTag($element) {
   +   * @see https://www.drupal.org/project/drupal/issues/3095113
   

   Nit: There should be extra line change before this line.

2. +++ b/core/lib/Drupal/Core/Render/Element/HtmlTag.php
   @@ -112,6 +112,9 @@ public static function preRenderHtmlTag($element) {
   +   * @see https://www.drupal.org/project/drupal/issues/3095113
   
   @@ -150,6 +153,8 @@ public static function preRenderConditionalComments($element) {
   +    @trigger_error('Support for IE Conditional Comments is deprecated in drupal:8.8.1 and is removed from drupal:9.0.0. See https://www.drupal.org/project/drupal/issues/3095113', E_USER_DEPRECATED);
   

   I think we usually link to the change record rather than the issue.

Please review the patch. I followed #14

The failures for the patches in #15 and #16 are because they didn't update the corresponding message in the skipped deprecations list.

I bumped the version in the patch to 8.8.2 for now, in the optimistic hope that this doesn't need to be delayed.

- Themes which want to use conditional comments to support IE <=9 can still add assets directly in the html.html.twig template, or insert generated markup into html_head (similar to what's needed for inline JavaScript), they just can't rely on the library dependency resolution (which isn't needed for things like html5shiv or classList polyfill).
- For the IE9 Compatibility module to maintain / restore support, my understanding is that it just needs to duplicate the current #pre_render callback.

Could we add #20 with some examples to the change record? That would address some of the concerns raised on #14.

Talked with @catch and he had a recommended that this would be deprecated from Drupal 10. Unfortunately, it seems like we don't have a decision on how we should do that yet #3088246: [policy, no patch] How to handle Drupal 8.9.x deprecations.

As per #3088246: [policy, no patch] How to handle Drupal 8.9.x deprecations this is non critical and unfortunately needs to be deprecated in 9.1 for removal in 10.

Working on this.

Here I have added reroll of patch #19 and addressed comment #23.

Thanks for updating the version numbers, this looks good to go now.

+++ b/core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php
@@ -200,6 +200,7 @@ public static function getSkippedDeprecations() {
       'AssertLegacyTrait::buildXPathQuery() is deprecated in drupal:8.2.0 and is removed from drupal:10.0.0. Use $this->assertSession()->buildXPathQuery() instead. See https://www.drupal.org/node/3129738',
       'AssertLegacyTrait::constructFieldXpath() is deprecated in drupal:8.5.0 and is removed from drupal:10.0.0. Use $this->getSession()->getPage()->findField() instead. See https://www.drupal.org/node/3129738',
+      'Support for IE Conditional Comments is deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. See https://www.drupal.org/node/3102997',
     ];
   }

What do we need to do to un-suppress the deprecation prior to Drupal 10?

The suppression was added because of classList and html5shiv, so I think it's no longer needed in 9.x

OK let's try to remove it here if possible.

Marking a test of JS assets being wrapped in conditional comments as legacy.

oops

We should be able to test the deprecation by adding this to one of the tests:

   * @expectedDeprecation Support for IE Conditional Comments is deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. See https://www.drupal.org/node/3102997

Hi @longwave
Updated patch please review.

@Deepak Goyal
You applied the @expectedDeprecation to the prerender method - it should added to the relevant test method(s) instead.

@expectedDeprecation annotation is deprecated, ExpectDeprecationTrait::expectDeprecation() method should now be used instead

https://www.drupal.org/node/3176667

Addressed #40.

Tested #42, changes addressed in #40 is visible in #42

Random fail.

Committed/pushed to 9.2.x and cherry-picked to 9.1.x, thanks!

Published the change record.