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.

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

parseStylesheet crashes on @media rules #11

Open
csnover opened this issue Jan 3, 2013 · 10 comments
Open

parseStylesheet crashes on @media rules #11

csnover opened this issue Jan 3, 2013 · 10 comments

Comments

@csnover
Copy link

csnover commented Jan 3, 2013

The stylesheet parser is naive and assumes that there are never nested rules, which is not the case when CSS3 @media is used. When one of these is encountered, the parser crashes. I don’t know that these actually need to be supported, but it would at least be good if they were safely ignored instead of causing a crash.

@faceleg
Copy link

faceleg commented Feb 28, 2013

👍

@tcg
Copy link

tcg commented Dec 12, 2013

👍 👍 👍 👍 👍 👍
Just ran into this, and it's a huge setback. We're using media-queries for native & CSS-friendly email clients to display things better/different than in clients that strip styles (gmail, etc).

@christiaan
Copy link
Owner

Would it make sense to be able to set a screen-width and have inlinestyle decide if it should keep the part between the media-query or not?

Or should everything between media queries be removed?

@tcg
Copy link

tcg commented Dec 13, 2013

To me, it doesn't make sense to actually use any rules from within a
media query for inlining, but they should be left in place wherever they're
found. They're not used often for email, but when they are, it's only for
certain native clients that support "normal" CSS anyway -- so inlining
those rules wouldn't matter.

I'd personally be fine with the parser just skipping @media blocks.

On Fri, Dec 13, 2013 at 2:56 AM, Christiaan Baartse <
[email protected]> wrote:

Would it make sense to be able to set a screen-width and have inlinestyle
decide if it should keep the part between the media-query or not?

Or should everything between media queries be removed?


Reply to this email directly or view it on GitHubhttps://github.com//issues/11#issuecomment-30494860
.

@christiaan
Copy link
Owner

So when a external stylesheet contains @media blocks you suggest that a new <style> tag is inserted in the containing said @media block?

@tcg
Copy link

tcg commented Dec 13, 2013

That's a good question, and I'm not yet sure what the general expected outcome would be. For my personal needs, if the media block was within the same document, in <style> tags, I'd just ignore it. When it comes to pulling things in from external stylesheets... I'm not sure. I've not yet familiarized myself enough with the internals of InlineStyle to understand how different that is from what it does to normal external styles, so I'd hate to make an ignorant suggestion.

@tcg
Copy link

tcg commented Dec 13, 2013

@christiaan With regards to setting a configurable screen width: I don't think that would be completely useful, because many media blocks may use relative units (ems, %s, etc) for their breakpoints - which don't all have a fixed correlation to each-other.

@tcg
Copy link

tcg commented Dec 13, 2013

What do you think about this idea (a bit of a hack, but workable):

In special cases where media-queries are required (and thus, the client is obviously CSS-friendly), media-queries should be placed in a <style> block with an Inliner-specific attribute, e.g.:

    <style data-inliner-ignore="true">
    @media only screen and (max-width: 41em) {
        div.wrapper { width: 100%  !important; }
        div.boundry { width: 100%  !important; }
        div.footer { width: 100%  !important; }
    }
    </style>

Elements with that attribute (style, link) will be skipped by Inliner, as a workaround.

It's a littler janky, but it's what I'm about to implement now, as I'm pressed for time on a particular project that uses InlineStyle + media queries.

@walter-schwarz
Copy link

I think the way the guys from ZURB do it, for their "Ink" Email CSS framework, is the best way: http://zurb.com/ink/inliner.php

Like you mentioned christiaan, when the CSS rules are parsed, media blocks which are found are put inside a style tag, but located inside the body not the head, and this happens not only for media blocks but also for any rule that contains pseudo classes. This way everything should work out as expected, and email clients that can understand these CSS rules can use them,

@christiaan
Copy link
Owner

Is it important that those style tags are placed in the body and not in the head?

I think using the inliner from zurb as a example on this might be a good start.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants