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

Already on GitHub? Sign in to your account

Fails if the string @media appears in a comment #22

Closed
necolas opened this Issue May 14, 2011 · 12 comments

Comments

Projects
None yet
7 participants

necolas commented May 14, 2011

The styles in an @media block don't get applied if the string '@media' appears in a comment within the @media block (not sure about if it appears between blocks). I know it's an edge case, but I usually include @media in a comment this to mark the closing } of the MQ! The experimental parser didn't experience this issue.

joelwan commented Jan 25, 2012

Ran into the same issue. This can probably be avoided if comments are stripped before translate() is called

Owner

scottjehl commented Jan 27, 2012

@necolas, @joelwan added a pull for stripping comments out. What's your take on whether Respond should do that? I think it's a pretty good idea, but trying to consider where it might trip someone up...

Thanks

necolas commented Jan 27, 2012

Sounds like a good idea if it can be done reliably.

I encountered this issue recently as well. As a workaround I added a space between @ and media so that the commented MQ wouldn't be parsed.

I ran into this error a few minutes ago, but just in Internet Explorer 8 and 7. Had a comment with @media [...] in it and my first media-query after this comment was ignored. In Firefox, Chrome, IE9, Safari and Opera there were no problems.

joelwan commented Jul 12, 2012

@paulchrablass I don't think my pull request that contains the patch that strips comments out has been merged accepted/merged yet. But you can try that as a temporary solution

@ghost ghost assigned jefflembeck Nov 19, 2013

Collaborator

jefflembeck commented Nov 19, 2013

We have decided to keep the parser dumb for speed purposes for the time being. Will potentially revisit this issue in the future. Thank you.

@jefflembeck jefflembeck reopened this Nov 21, 2013

Collaborator

jefflembeck commented Nov 21, 2013

Reopening this for discussion.

Collaborator

zachleat commented Nov 25, 2013

We could take a very similar approach to what I did for the @Viewport fix: strip the comments out with a regex.

joelwan commented Nov 26, 2013

Stripping via regex is in line with what i submitted as a pull request a while back. (looking at the timestamp - it was 2 years ago.)

Collaborator

jefflembeck commented Nov 27, 2013

@joelwan if you want to cook up another PR for it, I'll include it for the 1.5 release.

Marking as 1.5 milestone.

@zachleat zachleat closed this in f8057a5 Dec 26, 2013

Collaborator

zachleat commented Dec 26, 2013

Thanks @joelwan!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment