Opened 2 years ago
Last modified 5 months ago
#16118 new enhancement
Support for wp_enqueue_style with negative conditional comments
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Future Release |
| Component: | General | Version: | 3.0.4 |
| Severity: | normal | Keywords: | has-patch needs-testing dev-feedback 3.6-early |
| Cc: | obenland@… |
Description (last modified by nacin)
Please refer to #10891. It refers to the support for conditional comments using the global variable, wp_styles.
I have noticed that if you pass a negative conditional comment, however, this breaks. E.g. Let's say you have a lot of CSS3 rules, which don't apply to IE. You would not include that CSS:
<!--[if !IE]>--> <link rel="stylesheet" id="my-handle-css" href="http://my.url.com/css3.css" type="text/css" media="all"/> <!--<![endif]-->
I know that IE9 supports CSS3, but I am using the above for illustrative purposes. One would expect that to include the conditional comment above you would do this between the register and the enqueue commands:
$GLOBALS['wp_styles']->add_data('my-handle', 'conditional', '!IE');
If you add a conditional tag to wp_styles, however, the generated markup is incorrect:
<!--[if !IE]> <link rel='stylesheet' id='my-handle-css' href='http://my.url.com/css3.css' type='text/css' media='all' /> <![endif]-->
Note the missing --> after [if !IE]>, and <!-- before <![endif]. This has the effect of hiding the CSS from all browsers, which wasn't the original intention. So probably a separate handling approach is required for "show" type of conditional comments than for "hide".
Attachments (7)
Change History (20)
- Description modified (diff)
- Keywords needs-patch added
- Milestone changed from Awaiting Review to Future Release
- Type changed from defect (bug) to enhancement
SergeyBiryukov — 2 years ago
comment:2
SergeyBiryukov — 2 years ago
- Keywords has-patch added; needs-patch removed
- Keywords needs-testing added
In case the condition if !IE 8 is specified, the valid code should be:
<!--[if !IE 8]><!-->HTML<!--<![endif]-->
not:
<!--[if !IE 8]>-->HTML<!--<![endif]-->
Otherwise, in IE 7 or 6, for example, this will be displayed:
-->HTML
My patch addresses this edge case.
comment:5
sanchothefat — 13 months ago
- Keywords dev-feedback added
- Version changed from 3.0.4 to 3.4
There's one extra tweak you need for this to work properly that's in my attached patch (sorry - didn't name the first one correctly) As soon as there is an instance of '!IE' anywhere in the conditional we need to leave the contents of the CC uncommented.
Take for example a more complex CC:
<!--[if (!IE)|(gte IE 9)]><!--> <link ... /> <!--<![endif]-->
The not IE logic can appear anywhere within the CC. Complex CCs will become more useful with the ability to target IEMobile too.
I'd love to see this in core, not just for the sake of completeness but it makes writing stylesheets for plugins that need to separate support for older IEs and the newest much much better.
I've tested my modification of Gary's patch with the above example and various others and it works fine.
To test it work through the examples listed on this page: http://msdn.microsoft.com/en-us/library/ms537512(v=vs.85).aspx
Note:
In cases like !(IE 7) the contents of the CC should still be commented. Only !IE should cause the contents to be uncommented.
- Version changed from 3.4 to 3.0.4
Version number indicates when the enhancement was initially suggested.
comment:7
in reply to:
↑ 6
sanchothefat — 13 months ago
Replying to helenyhou:
Version number indicates when the enhancement was initially suggested.
Apologies, didn't realise that. Thought the ticket had been left behind at 3.0.4 :/
All of the attached patches seem to test for '!' or '!IE' which means you get a false positive when testing for '!IEMobile'.
comment:10
Umbercode — 11 months ago
I had some time so I thougth some more on this "problem". As I see it we have the following possibilities:
- !IE
- (!IE)
- !IE x (where x is a version number)
- (!IE x)
- !IEMobile
- (!IEMobile)
- !IEy (where y is any string that microsoft in the future might add)
So short of a regex I think we just need to decide which one triggers the requested behaviour.
Since !IE x and !IEMobile can mean just targeting any IE or IE derivative we can safely assume that we want this behaviour only on !IE or (!IE), if needed we can implicitly add it (e.g. if we also want to add all non-IE browsers to <!--[if !IE 7]>--> we can "force" this by using <!--[if (!IE 7) | (!IE)]>-->
As such I propose we designate (!IE) as the trigger, this eliminates the possibility for false positives like !IE 8, !IEMobile, !IE4 etc.
I attached a new patch for this.
comment:11
kobenland — 11 months ago
- Cc obenland@… added
comment:12
SergeyBiryukov — 8 months ago
#22122 was marked as a duplicate.
comment:13
helenyhou — 5 months ago
- Keywords 3.6-early added
Want to remember to look at this. Preliminary question: is it a matter of making specific things use the full comment around the start and end of the conditional, or is it a matter of making specific things use the comment around the conditional statement as a whole?

oops my patch is wrong.
Sergey's patch looks good