I tried to add inline CSS styles to <head> section using following PHP code:
$dynamic_style = ".wrapper { width: 90%;} /* This is sample comment */"
drupal_add_css($data = $dynamic_styles, $options['type'] = 'inline', $options['preprocess'] = FALSE);
Unfortunately Drupal ignores anything between /* and */. This might look like a minor issue (comments for inline styles are not very useful), but real problems start when I wrap styles with CDATA section:
$dynamic_styles = "<!--/*--><![CDATA[/*><!--*/ .wrapper { width: 100%; } /*]]>*/-->"
drupal_add_css($data = $dynamic_styles, $options['type'] = 'inline', $options['preprocess'] = FALSE);
In this case, the output will be:
<style>
<!-- .wrapper { width: 100%; } -->
</style>
instead of:
<style>
<!--/*--><![CDATA[/*><!--*/ .wrapper { width: 100%; } /*]]>*/-->
</style>
which will result in missing styles and invalid markup.
Documentation says that if 'preprocess' option is set to TRUE, then inline styles "will be compressed when being output on the page". But it appears to me that styles are being compressed no matter whether $options['preprocess'] is set to TRUE or FALSE.
| Comment | File | Size | Author |
|---|---|---|---|
| #23 | 672954-23.patch | 2.77 KB | jacine |
| #13 | css_cdata-672954-13.patch | 2.79 KB | jacine |
| #7 | css_cdata-672954-7.patch | 2.75 KB | effulgentsia |
Comments
Comment #1
jarek foksa commentedComment #2
damien tournoud commentedOutputting the CDATA part is not your job, it's the job of the page template.
Comment #3
damien tournoud commentedComment #4
jarek foksa commentedCould you precise how to do this in template file? All styles added via drupal_add_css() are outputted by
<?php print $styles; ?>in html.tpl.php. Note that CDATA must be put inside<styles></styles>tags, not outside them.Comment #5
damien tournoud commentedYou don't have to use CDATA at all. The parameter to drupal_add_css() is a plain CSS snippet. Why would you want to add CDATA in there?
Comment #6
damien tournoud commentedActually, we probably want to wrap those inline CSS (new in Drupal 7) in explicit CDATA, for consistency with inline javascript. Requalifying as a bug.
Comment #7
effulgentsia commentedThis patch includes the prefix and suffix used in the issue overview, but I have no idea if it's correct. Someone who understands that gibberish, please comment as to whether that is the correct way to comment out the CDATA for HTML4.
Comment #8
effulgentsia commented"markup" component maintainers are the best candidates to review whether the way in which the CDATA wrapping and commenting of CDATA tags for HTML4 is correct.
Comment #9
sunUnfortunately, I've lost the original source we used for the JS equivalent. However, according to http://www.webdevout.net/articles/escaping-style-and-script-data, this seems to be correct.
Comment #10
realityloop commented#7: css_cdata-672954-7.patch queued for re-testing.
Comment #12
jacineHere's another good source on this: http://hixie.ch/advocacy/xhtml
Comment #13
jacineHere's a re-roll of @effulgentsia's patch.
Comment #14
Jeff Burnz commentedThis appears to work as expected.
Comment #15
sun#13: css_cdata-672954-13.patch queued for re-testing.
Comment #17
jacine#13: css_cdata-672954-13.patch queued for re-testing.
Comment #18
jacineSomething must have been up with the bot when @sun retested. This is still green, so setting back to RTBC.
Comment #19
webchickI think this makes sense, since it brings CSS inline with the way JS behaves in D7.
But there are tests in at least filter.test that are checking for this behaviour with scripts. We need ones that check it for CSS as well.
Comment #20
sunThe patch already contains a change for the actually affected functionality that's covered by tests. User input and/or filter.test has nothing to do with this functionality.
Comment #21
tom_o_t commented#13: css_cdata-672954-13.patch queued for re-testing.
Comment #23
jacineHere's a straight re-roll since #13 doesn't apply anymore.
Comment #24
jacineSetting back to RTBC.
Comment #25
webchickWeird. Not sure how I missed that .test hunk before.
Committed to HEAD. Thanks!