Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

IE conditional comments within body causes error #531

Closed
matb33 opened this issue Dec 8, 2012 · 4 comments
Closed

IE conditional comments within body causes error #531

matb33 opened this issue Dec 8, 2012 · 4 comments

Comments

@matb33
Copy link

matb33 commented Dec 8, 2012

As there is currently no way to do the IE conditional comment technique around the <html> tag popularized by Paul Irish and HTML5 Boilerplate, I tried doing it within the <body> tag. This worked fine in all browsers except IE9 and above. For IE9+ I receive the error:

"SCRIPT14: Not enough storage is available to complete this operation."

Example to reproduce:

<body>
<!--[if lt IE 7]>      <div> <![endif]-->
<!--[if IE 7]>         <div> <![endif]-->
<!--[if IE 8]>         <div> <![endif]-->
<!--[if gt IE 8]><!--> <div> <!--<![endif]-->
{{> someTemplate}}
</body>

It is the last conditional comment that causes the issue (the if gt IE8).

Personally I don't think this is something that should be fixed at this level. That comment looks malformed to me (on purpose though). I think instead it would be nice to allow us to specify our own app.html.in template. We could then do these sorts of conditional comments at the <html> tag. I imagine it would also put it outside of Spark's reach and we wouldn't encounter this particular problem (maybe!).
@gschmidt
Copy link
Contributor

@matb33, the reason that we don't want to let apps specify their own app.html.in is that then we'd have to document the format of it (eg, the specifics of how {{head_extra}}, {{body_extra}} work) and commit to it. If we add a new feature and update app.html.in to support it, then that feature would be broken on all apps that used a custom app.html.in and we'd have to explain to everyone how to manually merge the changes in. I'd rather see app.html.in develop in the direction of adding more custom fields (analogous to {{head_extra}} and the <head> element in template files) in the cases where it's really necessary to get custom stuff in there.

If you're content using a wrapper div as you suggested (rather than putting the class on the <body> directly) what about <div class="{{browserclass}}"> and then write a browerclass helper that returns the class that you'd like to use for various browsers? That to me seem like the more meteoric way to do it. (And eventually I'd like to support template expressions directly on the html/body nodes, which would eliminate the extra div.)

@matb33
Copy link
Author

matb33 commented Dec 10, 2012

@gschmidt I don't think a {{browserclass}} helper would be the best approach because you'd have to sniff for the browser using JS. In the case of detecting IE < 10, the best way to accomplish this today is to use conditional comments.

The meteoric way to do this properly (currently) is to put IE specific CSS in public/ and load them via conditional comments in the <head> tag. The downside is that we don't get to leverage the Meteor build process that concatenates, minifies, etc, not to mention we wouldn't be able to leverage smart packages such as less -- we'd be forced to write our IE CSS old school.

The nice thing about the conditional comment system around the <html> tag is that we can target IE in our CSS within the same CSS we use normally, instead of in a distant separate file:

.box { padding: 10px; }
.lt-ie7 .box { padding: 12px; }

May I then suggest, as a future item in the roadmap, a smart package that would add the IE conditional comments around the <html> tag, matching the implementation found in HTML5 Boilerplate?

@matb33
Copy link
Author

matb33 commented Jan 25, 2013

I thought of a simpler solution to this issue. Simply put a series of inline script tags in the head with conditional IE comments around each, who are setting global variables containing information about the IE version detected.

Then write a Handlebar helper that returns the appropriate class (lt-ie8 etc) based on this global variable, and use this helper in a div class that wraps your entire app (probably a direct child of body).

@matb33 matb33 closed this as completed Jan 25, 2013
@piffie
Copy link

piffie commented Apr 23, 2013

<!--[if lt IE 7]><script type="text/javascript">$(function(){$('html').attr('class','no-js lt-ie9 lt-ie8 lt-ie7')});</script><![endif]--> <!--[if IE 7]><script type="text/javascript">$(function(){$('html').attr('class','no-js lt-ie9 lt-ie8')});</script><![endif]--> <!--[if IE 8]><script type="text/javascript">$(function(){$('html').attr('class','no-js lt-ie9')});</script><![endif]--> <!--[if gt IE 8]><!--><script type="text/javascript">$(function(){$('html').attr('class','no-js responsive')});</script><!--<![endif]-->

that pretty much makes the same, just put it in the head.

@snyk-bot snyk-bot mentioned this issue Jul 15, 2021
Open
@darendarrow darendarrow mentioned this issue Apr 24, 2023
Open
@Bhanditz Bhanditz mentioned this issue Apr 25, 2023
Open
This was referenced Apr 25, 2023
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@gschmidt @matb33 @piffie