"CSS Embedded Images" produces html comments in the header, and these comments fail validation.

<!--[if gte IE 8]><!-->
<link type="text/css" rel="stylesheet" href=".." media="all" />
<!--<![endif]-->

Not only that, but I also found that IE7 may render fragments of those comments visible to a normal visitor.

I imagine there is some difficulty if both Drupal core and css_emimage add their conditional comments, which can result in something nested. Simply doing the obvious and fixing the comments in css_emimage did not do the trick, because those comments were nested in some <!--IE6--> whatever comments (probably caused by IE6 specific stylesheets in the theme).

Comments

chrlvclaudiu’s picture

chrlvclaudiu CreditAttribution: chrlvclaudiu commented
Status: Active » Needs review

Hi,

The html comments inserted in the header of the document has a bit different structure and declaration of a normal one.
I think this:

<!--[if gte IE 8]><!-->
<link type="text/css" rel="stylesheet" href=".." media="all" />
<!--<![endif]-->

should turn into this:

<!--[if gte IE 8]>
<link type="text/css" rel="stylesheet" href=".." media="all" />
<![endif]-->

, according to all of the resources on the internet.

You can check line 175 and 176 in css_emimage.module and you'll see the changes you can do there to reflect the proper html comment declaration.

Maybe a patch can be written in the further.

Anyway, the document validates OK with this actual status.

Thanks,
Claudiu

jcarnett’s picture

jcarnett CreditAttribution: jcarnett commented
Status: Needs review » Closed (works as designed)

Unfortunately I can only say this is by design. Because the module supports IE < 8 it must use a downlevel-revealed conditional comment to only send the CSS with embedded images to IE >= 8 or non-IE browsers. That "or non-IE browsers" is the reason this particular conditional comment syntax is used:

<!--[if gte IE 8]><!-->
   ...stuff for IE >= 8 or non-IE browsers...
<!--<![endif]-->

This syntax was, at one point, officially recommended by Microsoft as a solution to the need to target both specific versions of IE and non-IE browsers at the same time. It's also valid HTML 4 and 5.

If you're seeing the generated HTML being broken or nested inside another comment, please reopen this issue and include the broken HTML you're seeing or at least enough detail to recreate it.

donquixote’s picture

donquixote CreditAttribution: donquixote commented

http://stackoverflow.com/questions/1073605/how-to-write-conditional-comm...
I can confirm what is said in #2, this is the "correct" syntax to use to target everything except IE6/IE7.

The problem is if this gets nested or mixed with something else.
Unfortunately I don't have anything available to test atm, so I cannot give more detailed feedback. I will keep my eyes open, and post next time I notice this problem.