Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
Over in #1322794: Make Stark use a responsive layout, we discussed the best way to feed a desktop-oriented layout to IE8. We currently have the funky drupal_add_css() API to add a separate IE-conditional stylesheet.
The problem with conditional stylesheets is that they are hard to maintain because the styles are separated from the rest of the normal browser styles.
And most people these days are in favor of using conditional classes instead of conditional stylesheets.
The best overview of the pros and cons of conditional classes vs. conditional stylesheets is at http://mathiasbynens.be/notes/safe-css-hacks
Proposed resolution
Replace all the IE-conditional stylesheets with inline styles using the IE-conditional classes. Since we've dropped support for IE6 and IE7, we'd need to change the html tag in our html.tpl.php to read:
<!DOCTYPE html>
<!--[if IE 8]><html <?php print $html_attributes; ?> class="ie8"><![endif]-->
<!--[if (gte IE 9)|(IEMobile)]><!--><html<?php print $html_attributes; ?>><!--<![endif]-->
That would allow us to add IE8-specific styles using CSS that is inline with the rest of our styles using a the ".ie8" class. For example:
.thingie { property: normal; }
.ie8 .thingie { property: overridden-just-for-ie8 ; }
If we use conditional classes on the html tag, we'll also need to resolve this issue: #1468104: conditional classes on html tag cause IE to ignore meta X-UA-Compatible tag and show compatibility mode button by altering our .htaccess file to include the X-UA-Compatible header. See #1203112: Add X-UA-Compatible HTTP header
Remaining tasks
Bikeshed about what the classname to use for the IE8 class.
Write the patch, fix the tests, etc.
User interface changes
None.
API changes
The API in drupal_add_css() to use IE-conditional stylesheets is convoluted and non-obvious. We won't change it in this issue. We could remove it in a separate follow-up issue. It'd simplify the code, which would surely improve its performance.
| Comment | File | Size | Author |
|---|---|---|---|
| #47 | ie8-desktop-fallback-layout.47.patch | 1.5 KB | effulgentsia |
| #41 | ie-conditional-classes-1471382-41.patch | 574 bytes | dcmouyard |
| #2 | ie-conditional-classes-1471382-2.patch | 555 bytes | dcmouyard |
Comments
Comment #1
JohnAlbinAdding mobile tag
Comment #1.0
JohnAlbinAdd code examples.
Comment #1.1
JohnAlbinFix conditional comment example.
Comment #2
dcmouyard CreditAttribution: dcmouyard commentedThis patch uses IE-conditional comments to put IE-conditional classes on the html tag.
Comment #4
dcmouyard CreditAttribution: dcmouyard commentedComment #5
dcmouyard CreditAttribution: dcmouyard commented#2: ie-conditional-classes-1471382-2.patch queued for re-testing.
Comment #7
dcmouyard CreditAttribution: dcmouyard commented#2: ie-conditional-classes-1471382-2.patch queued for re-testing.
Comment #8
JacineThis issue makes me sad. Do we really want to force this technique on everyone? I'm not going to argue against it if everyone wants it, but I am just going on record to say that I really hate the idea. I'm aware of the plusses and minuses, but still have never used this technique nor do I have any plans to. I really believe that using this technique is a decision that a themer should make, especially since IE8 usage is currently 18% and dropping and whatever decisions we make today will be with us for a long time.
This I don't understand:
We worked hard for months to get this in to (a) be consistent for drupal_add_css() and (b) add the HTML5 Shiv. Whether you are proposing to deal with this in a separate issue or not, how do you plan on loading the HTML5 Shiv? I realize that IE "conditional comments" are going out of style with web developers, but there is still no better way to handle this that I know of. I've never seen anyone recommend anything other than conditional comments to load the shiv.
Comment #8.0
JacineAdd link to #1203112
Comment #9
JohnAlbinDoh! That's a typo on my part. I meant drupal_add_css(). I've fixed that in the issue summary. Sorry about that. What I intended to say was that adding conditional stylesheets is a PITA.
You're absolutely right that we'll use conditional comments to load the HTML5 shiv. Man, that was a bad typo on my part.
That's not really an argument for or against this technique. That's an argument against having any IE8 support in core. Whether we use conditional stylesheets or conditional classes, IE8 support is a part of core. And we have to support it some way.
I stopped using conditional stylesheets when Drupal 7 came out without the ability to add them via .info files. The syntax for adding one with drupal_add_css() is not one I feel comfortable teaching to new Drupal themers. Its an embarrassment.
That's why I started using conditional classes.
Comment #10
dcmouyard CreditAttribution: dcmouyard commentedI don’t think adding conditional classes “forces” themers to use this method; they can still use conditional comments to include IE-specific style sheets. It does, however, make it easier for themers to use conditional classes if that’s their preference.
The only downside I've come across with wrapping the
<html>element in conditional comments is that it causes IE to display a compat view icon, but that won’t happen if #1203112: Add X-UA-Compatible HTTP header gets into core.Comment #11
JohnAlbinActually, there shouldn't be any spaces between
<htmland<?php print $html_attributes; ?>So that need's fixing.In Drupal7, the RDF variables were separate from the $html_attributes. That makes it easy (in a D7 contrib theme) to not add the RDF variables to the IE8 version of the
<html>tag. (RDF meta data only needs to live on the last<html>tag since that's the one seen by normal HTML parsers.) Since D7 themes are likely to need to support IE7 (or worse), that means they need to use more conditional html tags. That can be dangerous since the<meta charset="utf-8" />element “must be serialized completely within the first 1024 bytes of the document.” See http://www.whatwg.org/specs/web-apps/current-work/multipage/semantics.ht...However, since we only need one additional
<html>tag for IE8 including the RDF meta data twice still allows the<meta charset="utf-8" />to fall within the first 887 bytes of the document.Comment #12
JacineIt imposes the markup into all Drupal themes by default. If this is done, it's what all contrib developers will also use, so, themers will not exactly have a choice without cost. In order to undo what contrib does in this respect the themer will have to own the module's CSS entirely, which makes the upgrade path difficult at best. This is the major thing we are trying to avoid by separating CSS files in the system.base.css and system.theme.css. It's a tough problem. This is how it is... Core sets the example, and contrib follows. Sure it can be overridden, but taking over entire CSS files provided by modules is at a huge cost.
No, it's not an argument for dropping IE8 support. Suggesting we need to use conditional classes to support IE8 properly is what I disagree with. Regarding the point about the current usage percentage, it's not about core. The point is that if this code is in html.tpl.php, it's being imposed on everyone by default as THE way to do it, especially with regard to contrib. It's possible many of us will not even have to support IE8 at all in a few years, yet we'd all be stuck with the choice to keep the extra baggage or override the files to remove it, which is incredibly annoying and inefficient IMO.
Maybe we can meet halfway and store these styles in separate CSS files? Like.... module.ie.css? At least that way, there is still a way to get rid of the styles when they are not needed without having to override everything.
I agree 100% that the syntax DOES suck, but I think it would be better to improve it instead of dumping the functionality entirely. I haven't stopped using them. Regardless of the syntax sucking or not, it's been great to have the IE code separate, and to have access to it via hook_css_alter(). It has come in pretty handy.
Hopefully that makes sense. I realize that I'm not going to change anyone's mind here, and I'm not trying to. I'm worried about the impact this will have when it spreads to contrib and generally don't think Drupal core should have this much of a say in how this should be handled.
Comment #13
effulgentsia CreditAttribution: effulgentsia commentedI'd really like more front-end developers to comment on this issue. To me, it really does come down to what is accepted best practice. If loading IE8 CSS with conditional comments is really considered bad practice these days, then sure, let's switch to a more modern approach. However, if there's others like Jacine who still think it's an appropriate technique, then I think we can make the API for doing so better, including re-opening #522006: Conditional Styles in .info files, since drupal_add_css has it.
Comment #14
dead_armI'm a designer and themer and I use conditional IE classes in my current workflow. I find that if I need to change something for IE 7 or 8, I can do it right where the original style is coming from. Since there isn't usually a need for massive changes in IE, it seems to be much more efficient than working in a new sheet.
Comment #15
mason@thecodingdesigner.com CreditAttribution: mason@thecodingdesigner.com commentedI really prefer the conditional class approach. Regardless of how easy or hard it is to add conditional css files I just think this technique is much easier to use, manage and teach. Similar to @dead_arm, I like to add my IE fix right next to whatever other styling is applying to the element in question.
I'd consider using conditional files if my ie fixes were piling up to the point where they were so heavy that it was affecting performance. However I haven't run into that situation in the two years since I was introduced to the conditional classes technique, and anyway I think css that heavy would be a sign that there were bigger fish to fry.
Comment #16
ruplThe patch in #2 is too strict with the conditionals. Sure, we're not supporting below IE8, but the patch as written would cause no
<html>tag to be parsed in oldIEs. Not good.We need to use LT or LTE conditional comment at least once to cover all the really oldIEs:
or maybe even
Comment #17
ruplFWIW, I am a frontend developer, and I prefer conditional classes over conditionally commented requests.
I don't think the ability to use conditional comments should be removed from drupal_add_js() or drupal_add_css(). We should have the ability to use those regardless of the workflow we decide on.
Comment #18
Jeff Burnz CreditAttribution: Jeff Burnz commentedHmmm, thorny issue with pros and cons. I have used this in my themes and I think I first saw this in HTML5 Boilerplate, then in Zen and decided to give it a go, so I have some experience actually using this and where I think it can be useful.
I have to say I think I am with Jacine on this one, I don't really like the idea of this in our core template. Point being that Drupal is used for many things, but this assumes we are building a public website that will potentially have IE usage, isnt that a pretty big assumption? I think it is.
IMHO I think the core templates should be very simple and make as fewer assumptions as possible, so bare bones and not trying to solve peoples IE styling issues. This puts a cost on everyone, not just legacy IE, that's the kicker here. So, I hate to be a downer on this, but I for one will say no, this is not the solution for core.
Stark can load its IE8 stylesheet, its a theme, it can do that, but for core html.tpl.php please don't do this, leave it clean and best practice only.
OK, so veering very slightly off topic, but, this is the reasoning for adding these classes so I will at least address this to show the idea is actually flawed and introduces a wtf moment for normal front enders, you know, people who just do stuff and expect it to work, like they should?
There is the obvious permutation that no one is talking about here, the elephant in the room so to speak... respond.js
What happens when respond.js is loaded, because you know, some people will load this, either copy/paste in or use some module that adds it. I just worked though these issues and a hundred more and can tell you these classes are a fragile way to support a fall back layout in a generic system, just think through the permutations a bit more and you will see I am right.
When respond.js loads you have to unset the fall back layout because those classes are powerful, they are not free and have high specificity. They will bork the layout in IE8 when respond.js is on.
.lt-ie9 .sidebar {}trumps.sidebar {}, period, every time. So your lovely responding layout, now being supported in IE8, via respond.js, is overridden by your IE8 only layout - except its not just IE8 any more, its IE8+media queries. So what solution now - add the layout twice in @media {}, so now we punish everyone 3 times, instead of just twice?Call me old fashioned but I think that sucks a wee bit, end user surely should be able to use respond.js with core theme and it works? Desktop first has a lot of mileage left in it, especially for retrofitted sites and cost constrained projects, it can make a lot of sense, so while the mobile first idea is great, I use it, it rocks, we cant throw the baby out with the bathwater. People should have flexibility without having to undo stuff, imo, of course, this is a challenge, I know very well, and the solution can be complex, or very simple - do nothing.
Comment #19
dcmouyard CreditAttribution: dcmouyard commentedI prefer to use conditional classes and I think it's currently a front-end best practice, but I have no qualms about leaving it out of the core html.tpl.php template. The benefits of using conditional classes for Stark are minimized due to the limited styling it needs. Other core themes can add conditional classes on a case-by-base basis.
I do, however, feel strongly about using a mobile-first approach to media queries rather than a desktop-first approach.
Comment #20
Jeff Burnz CreditAttribution: Jeff Burnz commentedThis is not a best practice, lets not be too insular in our thinking, the great unwashed have never heard of this, and the powers that be do not endorse it. It may become a best practice one day, until then, for want of a better word, its fashionable, although also extremely useful. Another word that springs to mind is hack.
dcmouyard, this is not a discussion about approach (mobile or desktop first), its a discussion about methodology, as in how to achieve it. Both are important and have use cases, we need to be careful about precluding one or the other, so I am not falling either way on any issue at this stage, but being weary of things that might cost everyone where that cost is not needed, or has insufficient ROI.
Comment #21
tim.plunkett#20, maybe rephrase your first sentence? Especially who you mean by "the powers that be".
Comment #22
ruplThis fashionable hack seems to work for some of the most popular JavaScript libraries on the web. They employ
<html>classes as a means of allowing features to be reported consistently between CSS and JS. Dojo, Modernizr, TypeKit, Responsive Images... There are tons of examples with varying purpose (i.e. not limited to UA strings) and it is nowhere close to being a fringe technique. In my opinion there's rarely such a thing as a best practice in frontend development, but this one's well-tested, versatile, and adaptable for the future.Comment #23
effulgentsia CreditAttribution: effulgentsia commentedSpeaking of JavaScript, would it make sense to add the IE class(es) with JavaScript rather than conditional comments in the html.tpl.php file? If IE users who disable JS don't get the IE styles, is that a serious problem?
Comment #24
Jeff Burnz CreditAttribution: Jeff Burnz commentedChris, I am not saying its a fringe technique, it has uses, yes, however its not really our concern what others are doing because it does not address the issues that we are having, maybe it solved some issue they had, but what are our issues, these are more pertinent. We can rarely have the pleasure of allowing others to sway our decisions, that is too easy, good things are hard to find, and take hard work, right? Review element-invisible discussions for an example. Here we set the standard, a new standard, and it took 9 months to do, but look, now everyone follows, even Compass has proposed this.
Tim, W3C, although I have not checked that for a while, last time I looked I found nothing alluding to this.
effulgentsia, I think the argument we will run into with that one is "not more js to support layout", since we are talking about this principally in an issue in regards to layout. Frankly I have no issue with it. I am still thinking over this stark vs core thing, seems to me the route ahead is for stark to do more, and core to do less.
Comment #25
tim.plunkettThe logic in the first and second paragraphs of #24 are directly contradictory.
Are we leading the way by adopting a newer technique, or waiting for the W3C to tell us we can try something new?
Comment #26
catchDoesn't this mean that IE styles get loaded for all browsers, instead of just the ones affected?
Comment #27
Jeff Burnz CreditAttribution: Jeff Burnz commentedTim, please, solutions, picking holes in one small point is not the way forward. We need a solution not nit picking. I have raised serious and real points against this, lets focus on those. My apologies for countering that part of the argument (is this a best practice and who endorses it), it will always be fraught with subtle contradictions and other such messiness, however, if there are contradictions its because they exist, we cannot avoid them, simply look at them and weigh them up. This is what I am doing.
Catch: yes, exactly, that is a one part of my argument against this.
Comment #28
Jeff Burnz CreditAttribution: Jeff Burnz commentedeffulgentsia made two important comments earlier on in #13
It depends really, if there are a small amount of overrides, and imo these never or rarely change (for example some sort of fix for rounding errors), then why incur the additional http request? Its better to load a little bit more CSS for all than incur the http cost. But, if there are many overrides, or the context or styles can change, then its a more complex situation. Not sure anyone can, yet, deprecate conditionally loaded stylesheets as a best-practice, it "just depends".
That should happen anyway, the current situation is less than ideal for designers.
I alluded to something above that probably fell under the radar as I was too subtle, I actually don't think its off the table to do nothing here, I mean really nothing, just don't provide any layout at all. Is that so crazy? Could we not provide instead documentation, examples etc, with notes on mobile first, IE8 issues and so on?
Doing nothing would satisfy the mobile first requirement and send clear signals: look ma, no layout, we're mobile first by default. Secondly we're simply removing shit, not adding shit. Removing shit is usually good. Thirdly we get to keep the default-layout-can-o-worms shut, as soon as you open this you have to deal with the consequences, and the complexity ramps up very, very fast. Fourthly it removes the layout use case from this issue and allows us to analyse this from a general front end development practice perspective.
Cons - it might make it microscopically harder for total complete noob to get started - so tell them to please read some docs, you are doing web development, learn something about it, here's a clean slate without (many) assumptions. They have to read docs anyway, so the content is merely different and we avoid the messy issues created by making (too many) assumptions.
Its quite plausible the do-nothing approach has merit, is this worth considering?
Comment #29
katherinedWhile I am used to using conditional stylesheets, right now I like the conditional classes method. Rupl's number 1 point is what seals it for me, that "fixes sit right next to the "problem selector" in the source files."
Just thought I'd raise my hand as long as we're lending opinions, but please also realize that the site I work on most is visited by so few IE users, that I stopped caring THAT much about IE a while ago. :)
Comment #30
tim.plunkett#27 keeps getting edited with no mention of what's getting changed. Hard to respond to that.
Not to insinuate that it comes down to a vote, but here are the opinions so far:
In favor of conditional classes:
JohnAlbin
dcmouyard
dead_arm
canaryMason
rupl
tim.plunkett
katherined
In favor of conditional stylesheets:
Jacine
Jeff Burnz
To me there have been no strong pros or cons, just preferences (as requested by effulgentsia in #13).
Can someone hash out the pros and cons in the issue summary?
Comment #31
ruplThere's a measurable advantage of conditional classes that no one has addressed while arguing in favor of conditional stylesheets, and that is front-end performance. In my comment above:
My other points were workflow/preference. I think there is a significant workflow improvement by centralizing progressive enhancements around the
<html>classes, but offhand I can't think of a way to measure that cleanly :)Comment #32
effulgentsia CreditAttribution: effulgentsia commentedI think the performance argument could go either way. With conditional classes, you reduce an http request in IE, but you force bigger CSS files on everyone else.
I wonder if we can break this up further:
Comment #33
JacineSo, look... I would just like to clarify that I do NOT have a problem with people deciding to use the technique. I don't even have a problem with core themes implementing this technique. I haven't used it, because I mostly have not had a need to, and I even rarely need to target IE8 at all.
What I have a problem with is Drupal making this decision for everyone by putting this in system/html.tpl.php. All I want here is to be able to make the choice on my own, as to what is best for my projects.
I also think it's sad that just because a newer technique comes along that a lot of people use that we want to completely remove the ability to even use conditional stylesheets.
Right now, it may well be better from a performance perspective for those that are using it because contrib doesn't have access to these classes, and themers have 100% control. But once contrib starts abusing it, and you better believe they will, then we will see what is better from a performance perspective and a workflow perspective, especially when you no longer have to support lte IE8 in your projects.
Clearly, everyone wants that and doesn't think there will be a problem, so I'm not going to continue commenting, but I just wanted to clarify my position on it.
Comment #34
ruplJacine, I don't think *anyone* is suggesting we remove the ability to use conditional stylesheets or JavaScript. That would be silly.
effulgentsia, the amount of IE-specific CSS would have to take longer to download than the ~100ms an extra request would incur. Even without data in front of me, I strongly suspect the test will be in favor of combining assets. However, it might not be worth the effort to profile, because resolving #784626: Default all JS to the footer, allow asset libraries to force their JS to the header would be a *massive* performance improvement compared to this issue — probably by several orders of magnitude.
Comment #35
Jacine@rupl, the JS was a typo originally, but issue summary states it for CSS as a follow-up task:
Comment #36
effulgentsia CreditAttribution: effulgentsia commentedOk, I've been thinking about this more. So, the situation is we have 3 core themes, and at the least we want Stark responsive, but we might end up wanting Seven and Bartik to be responsive too. And because they're core themes, they must not look totally crappy on IE8. So seems like our options are:
My current preference is #4, but I'm an amateur when it comes to this stuff. What do all you experts think?
Comment #37
Jeff Burnz CreditAttribution: Jeff Burnz commented#30 is a straw man. Additionally I support neither, rather a do-nothing approach, which is something different again and not dependent on this either way.
rupl, yes this is true, I addressed this (performance issue), however, to my knowledge there is no IE8 specific CSS in core modules, if there is its opaque and could well be re-factored and/or removed.
Bartik has some IE8 related stuff, embedded in regular styles - its opaque, merely leveraging rgba non-supporting browsers to ignore unknown property value. Does not need abstraction.
Seven has a couple of lines to work around its handling of fieldset legends. Could be re-factored and potentially remove an entire stylesheet.
This (conditional classes) does in part remove ability to use conditional stylesheets properly, due to specificity issues. I already went into much detail as to why this is and why this idea is flawed in specific use cases. Again, its one thing to add the classes, another thing to actually us them.
For Bartik, Seven, Stark it all depends on the mobile first approach taken, as previously discussed, i.e. major issues with this and not easily solved using either approach (classes or CC stylesheets), which is why I am on the do-nothing bench, which does not preclude not adding such classes to the template.
Comment #38
Jeff Burnz CreditAttribution: Jeff Burnz commented@36:
#1 has the big con I presented earlier, specificity issues with requirement to delete part of Starks layout if doing desktop first with respondjs, as I alluded to earlier this could be docs issues only.
no-js IE8 is a non issue, I have 20k users relying on repsondjs and not a single issue posted on this point. respondjs is not an option so precludes #3 imo, there are just way too many resulting issues from using this script (I found this out the hard way, via 20k users relying on respondjs...).
So, #4 is the most interesting solution and worth further consideration - very nice idea and in line with the modern practice of feature detection.
Comment #39
ruplI also like #4 with one modification: the HTML should be initially delivered with
no-mqand have it removed via JavaScript, exactly the same way that ano-jsclass is handled. This way, we're starting with fewer assumptions about browser capability, and requiring a positive test for the feature.Part of me wonders how many other features we'll be wanting to detect after the first test is in there.
Comment #40
effulgentsia CreditAttribution: effulgentsia commentedThat means users who disable JS on media query supporting browsers receive CSS that potentially conflicts with the media queries. Personally, I'd rather prioritize the standards-compliant-browser-with-nojs experience ahead of IE8-nojs, but I admit to being biased that way.
Comment #41
dcmouyard CreditAttribution: dcmouyard commentedrupl made a good point in #16 to use lt or lte in the conditional comment so old IE browsers will get an
<html>element.For this patch, I went with the following:
I added !IEMobile in the first condition so IE 7 mobile won't get two
<html>tags, and I only used two conditions rather than three because of the issue JohnAlbin brought up in #11, where it's important that<meta charset="utf-8" />is within the first 1024 bytes.We could add a third condition for IE browsers older than IE8, but then we should separate out the RDF data from
$html_attributesand only add it to the final<html>element.Regarding the four options effulgentsia mentioned in #36, I also like the idea of #4, but that doesn't address the issue of still needing conditional classes for non-layout IE8 fixes.
I think we should pursue adding media query feature detection in drupal.js in another issue.
Comment #42
effulgentsia CreditAttribution: effulgentsia commentedWhat I'm suggesting though is that we don't want core providing a conditional class for miscellaneous IE fixes (as per #36.1), and that any non-layout IE fixes be done with conditional stylesheets in the core themes (#37 suggests we might be able to fix the core themes to not need them anyway), and contrib themes can override html.tpl.php to add IE classes if they want them.
However, would it be crazy to do:
instead of JS-based feature detection? It feels a little wrong to me, and as I said, I don't particularly care about supporting IE8-nojs, but if others do care about that, maybe it's not so bad?
Comment #43
dcmouyard CreditAttribution: dcmouyard commentedUnless the IE fixes are substantial, then using conditional classes is better for performance than using conditional style sheets. As rupl mentioned in #34, "the amount of IE-specific CSS would have to take longer to download than the ~100ms an extra request would incur."
And I don't forsee us needing more than a few IE8 fixes, if any...
I'm still up in the air about using js feature detection for media queries.
If we do mobile-first responsive layouts, then IE8 is the only browser we need to provide a fallback layout for, since it's the only desktop browser we're concerned about that doesn't support media queries.
It might be easier to just use the ie conditional class rather than detect media query support. We won't care about other browsers that don't support media queries, since they'll just get the initial mobile layout.
Comment #44
ruplNo, that's not crazy. It's very interesting.
My initial reflex was a bit negative, but the result is not false when considering IE-specific styling. The only shortcoming is that when considering all browsers that lack media query support, we're telling a bit of a lie. However, they won't be old desktop browsers, so hopefully they would handle a well-designed responsive layout.
I agree that it feels a tad wrong, but I've never considered using conditional comments for anything but explicit UA identification, and think it's worth discussing. I would like to hear some other peoples' thoughts about this.
Comment #45
catchThat's not right though.
If you have IE8 fixes in a separate file, then there's a separate http request for that file, if you're using IE8, but not for any other browser.
If you have IE8 fixes embedded in all your other CSS, then there's additional CSS to transfer, as well as for the browser rendering engine to process, for every browser.
So either way, there is a performance trade-off. Both of them are very small in the overall scheme of Drupal front end performance issues (for which we have many open issues getting much less attention than this one).
Comment #46
Jeff Burnz CreditAttribution: Jeff Burnz commentedrupl, agreed - its very interesting and can see far reaching uses for this.
So the no-media queries/not IE8 issue.
The last thing we want to do here is push redundant CSS at mobile devices, as rupl points out this group are mobile browsers - last generation Blackberry, IE7Mobile, Kindle, almost all feature phones and many others - many of which are coming on the market now and in the future in all probability.
So we cannot do:
Since we're trying to do mobile first here always, so this is a fly in the ointment that must be solved.
I suspect there is a set of tests that can combine to accurately target IE8, i.e. modernizr
Is a js test to target only IE8 needlessly replicating conditional comments?
How to handle no-mq/not IE8 issue?
Comment #47
effulgentsia CreditAttribution: effulgentsia commentedHow's this? Feel free to suggest edits to make the comment better / shorter.
Comment #48
dodorama CreditAttribution: dodorama commentedI think it's a little bit overkill to introduce all of this for IE8, a browser that, by the time Drupal8 will start to be used extensively (2014?)' will probably be almost dead #1465948: [meta] Drop some IE8 coddling from Drupal core . I'd rather be happy to serve it the mobile version or a specific style sheet through conditional comments. Ideally our info files should be able to support conditional comments so that we can avoid using drupal_add_css or multiple html.tpl.php templates. See http://drupal.org/project/conditional_styles
Having a separate ie8.css file means you can easily forget about it if you don't plan to support ie8. It might be not what themers do today to target it but it's the easiest way to keep the code clean. If I design a new theme starting by duplicating bartik and I'm not planning to support ie8, it's easier to just delete the file than looking for all .ie8 declarations.
Comment #49
JohnAlbinThat's just your assumption. (I'd love it if it was true!) I've replied in that other issue. Until we are closer to release, we must assume IE8 support is required in D8.
Guess what? We are ALREADY pushing redundant CSS at every device. Core's CSS has plenty of IE-specific hacks that are completely undocumented and not pushed to separate IE-specific stylesheets. The .odd and .even classes? Only needed for IE. We need to simplify our selectors in core and we need to untangle our IE-specific hacks from the rest. That's even more important than this issue. IMO, adding IE-conditional classes makes it easier to maintain these changes for D8's lifetime. In D9, we can kill IE8 support, strip the IE-conditonial classes and easily remove the ".ie8" prefixed rules.
I've thought over your point in #18 and you are not wrong. If we have a theme that uses a slightly-more specific ruleset (
.lt-ie8 .no-sidebars #content { etc. }), and you enable Respond.js for that theme, then you'll get a mess of IE-specific desktop layout plus the media query layouts. But that leads to the obvious question… Why would you enable Respond.js?Yes, but why? Why would you go out of your way to add respond.js to a theme that already supports older IEs? Mobile devices don't need media queries; you should be writing mobile-first CSS. What other desktop browsers besides IE lack media query support? NONE.
The only reasonable situation I can come up with is if you needed respond.js for a contrib theme and used a core theme on part of your site. Or if you had a legacy website with a mix of old themes and new themes and they were using separate techniques for dealing with IE.
But since D8 will not come with respond.js (CSS aggregation off by default == respond.js broken by default), let contrib solve this problem. Correspondingly, I've opened #1504206: Add setting for per-theme enabling of modernizr and #1504200: Add setting for per-theme enabling of respond.js.
@catch: Unfortunately, we are already doing that now. In core modules, there are several IE-specific CSS rules that are undocumented as such and no core modules use a separate IE-only stylesheet. Also…
I agree.
Nope. That would be pointless for IE8 support. There are lots of mobile browsers that don't support media queries. With the responsive themes that D8 will have, the media queries will only be used for larger sized devices. With browsers that don't support media queries, we'll simply assume a mobile layout. That's the essence of the "mobile first" approach.
The only problem with a mobile first approach is what to do with desktop browsers that don't support media queries. And guess what? That means IE8 or earlier ONLY. There aren't any other desktop browsers. So "targeting desktop browsers that don't support media queries" == "targeting IE8 or earlier" and does NOT equal "targeting ANY browser that doesn't support media queries".
So, we shouldn't write JS to check for media queries because it won't be useful to get desktop layout to IE8. We need an IE8 solution which is what conditional classes are.
Also, the patch in #47 is wrong because the conditional classes are checking for versions of IE, not media query support. So the class in HTML should be about IE, not about media queries.
Comment #50
dodorama CreditAttribution: dodorama commentedI would like you to consider the idea if not dropping it of at least leaving IE8 behind for the moment and serve it the mobile version so that it doesn't slow down the transition to responsive design. There will be many issues to tackle to make Seven responsive.
The idea would be to think about a Grade B support for it and then revisit the question before release where we may decide to drop it completely, support it in part or use conditional classes or whatever will make sense 18 months from now.
We already making many bold moves in D8 and it seems like we'll have to revisit much of the theming stuff anyway #1499460: [meta] New theme system . So why bother now?
Comment #51
catchRight, core CSS is notoriously bad, however it would be quite possible to go through and remove/clean this up and put it into separate IE-only stylesheets if we wanted to, in the same way that we could go through and use a .ie8 selector.
Again, it's not "better performance". It's "better performance for IE8", vs. "better performance for every other browser". It's true that the hit for IE8 will likely be a lot more obvious than the hit for other browsers, but assuming we don't actually drop IE8 support in Drupal 8, we'll have those additional classes in core until likely some time well after 2017 when IE8 usage could drop to nearly zero, while the extra CSS will still be getting served to every other browser.
Having said that, anything which means we can isolate IE-specific CSS more than it is currently (i.e. not at all) would be better than nothing, and I'm fine with some degradation for IE8 users - I don't think we should bother trying to fix things like dropshadow or whatever else for example.
I'd prefer it if we could keep discussion about actually dropping IE8 support to #1465948: [meta] Drop some IE8 coddling from Drupal core, I've added a comment there too.
Comment #52
nod_I don't have an opinion about this particular issue but the performance problem will have measurements tools very soon. We'll be able to know precisely how much one solution or the other "costs" in term of performance or how much this or that delays rendering in IE.
Comment #53
Jeff Burnz CreditAttribution: Jeff Burnz commentedI've thought about this more too and I'm gonna get on board with this point regarding respond.js. Lets focus on one right way and do it really well.
I'm still kind of in the do-nothing camp at the moment, starting to lean towards isolating fixes in separate IE stylesheet if we don't do nothing.
nod_ is there an open issue for these performance measurements tools?
Comment #54
nod_No issue for now. It'll become a module in the following weeks, I have a lot of work to do on reporting. Data gathering part is working. I just want to make sure it's accurate before pushing the code.
Comment #55
klonosHey Théodore, will you "officially announce" the release of this module here too?
Comment #56
effulgentsia CreditAttribution: effulgentsia commentedThe point Jacine raised in #12 and #33, and that I summarized in #36.1 is that if core's html.tpl.php has a class named "ie8" or "ie" or "lt-ie8" or anything like that, then contrib modules will abuse this class, because it will appear that Drupal core is sanctioning this technique for IE-specific styling. And while custom/contrib themes have a mechanism to remove ie-specific stylesheets added by contrib modules, they do not have a mechanism to remove ie-specific rules within otherwise necessary module stylesheets. So putting an IE-specific class into D8 core means burdening all D8 websites with IE-specific cruft for several years after many websites will no longer need to support IE8.
My goal with #47 was to add a class that clearly signified a more limited scope, so that modules use it for desktop layout fallback only, and not for whatever random IE quirks.
Comment #57
effulgentsia CreditAttribution: effulgentsia commentedAlso, linking to #678292: Add a development mode to allow changes to be picked up even when CSS/JS aggregation is enabled. That issue might pave the way for us to add Respond.js to core, which maybe is the best solution to this problem.
Comment #58
Jeff Burnz CreditAttribution: Jeff Burnz commentedYeah, but we don't really need respondjs and don't really want it because we don't really want a responsive IE8. All we need is a "standard layout" for IE8, what all other users will get when visiting in a large screen device. That's about the long and the short of it.
Comment #59
effulgentsia CreditAttribution: effulgentsia commentedHuh? IE9 will be responsive. If we manage to solve the blockers to Respond.js, why would we want to code up special stuff for IE8 instead of letting Respond.js make it work like IE9?
Comment #60
JohnAlbinThat's not a bad goal. But you can't test for one thing and claim another. We're testing for IE in that patch; the class must reflect what was tested.
That is the best argument against this. And I don't really have a counter point to it. But its basically impossible to guess whether we'll need IE8 support for most of D8's lifetime. Many think it will be unneeded for most/some of D8's lifetime. I fear it will be needed for all of it.
Comment #61
dcmouyard CreditAttribution: dcmouyard commentedWe need to reach a decision on whether to add IE-conditional classes to html.tpl.php so we can move forward with making the core themes responsive. Either we say "load a separate IE style sheet" and close this issue, or we review the patch in #41 and add conditional classes.
Pros
Cons
I'm comfortable with either option.
Comment #62
effulgentsia CreditAttribution: effulgentsia commentedI'm calling it "won't fix" on the grounds that I think IE-specific stylesheets are fine for now, and that I'm hopeful that we'll be able to remove them before D8 release by either accepting a small layout for IE8, or adding Respond.js. If anyone disagrees, set back to "needs review".
Comment #63
JohnAlbinCan someone please create a follow-up issue for this and post back the issue #?
Comment #64
catchDone #1507960: [meta] Isolate and/or remove IE-specific hacks in core markup, CSS and JavaScript.
Comment #65
hass CreditAttribution: hass commentedJust for my understanding... Themers are normally free to do anything they like/need/must in their theme layer. Drupal allows me officially to replace anything on the layout level. But than - we must leave all the conditional hack integration inside core as long as possible - at least removing them in D8 is far tooo early!
Themers may need this whether core like to support IE6/7 or not. A business need or decision may be completely different. I hope I have not misunderstood the case here, but please make sure you are not going to remove the IE conditionals feature in
drupal_add_css()as this is something that is the themers decision and cannot be the Drupal community's; if they should be used or not.YAML Themes are all responsive, flexible and accessible and are not using class based logic described here and I cannot speak for future days; if such an approach may go in the product or not. But I do not like to get limited by core this way as it's really useful.
Aside I need IE conditionals for HTML5Shiv in
drupal_add_js()in D7, too.Comment #66
JohnAlbinNo, that was a possible follow-up to this issue, but since this issue is "won't fixed", it doesn't make sense to remove them from drupal_add_css().
Also, the patch to get conditionals in drupal_add_js() is in D8 already. If you can figure out a way to backport that, it would be sweet! Likely it would have to be done in contrib.
Comment #67
effulgentsia CreditAttribution: effulgentsia commented#865536: drupal_add_js() is missing the 'browsers' option is the issue that dealt with it in D8. Change its status to "patch (to be ported)" or better yet, port the patch and set to "needs review" :)
Comment #68
hass CreditAttribution: hass commentedThis would be an API change... need help to push...
Comment #68.0
hass CreditAttribution: hass commentedFix typo in "api changes" section.