Consider a non-comment way to detect media query

[edwardoriordan]

It would be great to have a non-comment way to detect media queries because

• They will get stripped out when minifying.
• It is difficult to correctly place them after the query with no line break when using compass.

[scottjehl]

I agree that'd be ideal; the comments sure save weight in the JS logic though.

I'm currently discussing potential scenarios where matching two }'s in a row (allowing for whitespace and comments in between them) will fail. I've got a regexp locally that'll match my examples just fine without the comments, but I'd want to build a larger test suite before rolling something like this out as the current test page is pretty limited. The question is, what sort of {'s can appear within a media query? @keyframes, @font-face, etc.

I'll give it some thought.

Your second bullet seems easy enough to fix though! I'll make a new bug for it

[edwardoriordan]

I figured that there was a good reason why it was implemented that way. Thanks for letting me know the status. Good luck with a great project.

[pyrsmk]

But, just a matter of interest, why should you put comments to find @media? Can't you just find @media?

[scottjehl]

The comments allow for a lightweight implementation. Without the comments, the script would need to implement more parsing logic to detect the end of a media query. Simply detecting two }'s in a row won't be enough, as several CSS features use braces.

[pyrsmk]

You're right, I didn't thought about that ^^

Good luck with this new implementation, if you need testers I could help you ;)

[gillesv]

Could you perhaps adapt the regular expression used to select the comment to this one:

/@media ([^{]+){([\S\s]+?)(?=}\s\/\/mediaquery\/)/gmi

This will also work if the mediaquery is written as:

@media screen{ ... } //mediaquery/

... which is what LESS (http://lesscss.org/) compiles to when you disable minifying. The extra linebreak between the closing bracket and the comment breaks your existing implementation. This regex should solve that.

[chendrix]

I'm also pushing for @gillesv said, as I use LESS and was hoping for Respond support.

[pyrsmk]

Just to explain a bit, the LESS compiled code is the same as Sass. And right, to rectify @gillesv, the compiled comment is /*/mediaquery*/.

@gillesv @chendrix: the only problem with that is that the comment will be dropped when minifying, so we can't keep this regexp as is, but it can be a nice ephemeral fix...

[scottjehl]

yeah you'll need to ensure certain comments aren't stripped. Most minifiers have ways to specify comments to exclude from minification, but I'm not sure about LESS/Sass.

[pyrsmk]

It doesn't exists for Sass, we can just remove all the comments or none :s

[chendrix]

I just opened an issue in less asking to preserve the no-line-break between closing braces and comments, as well as selective non-minify, but I doubt much will come out of it.

[scottjehl]

@chendrix: the line break shouldn't cause a problem now at least... if you grab latest. Selective non-min is critical though...

[rupl]

I would be in favor of a non-comment approach. Minifying CSS is a standard practice for production sites, and no one will stop doing that. Instead of altering each minify process to make an exception (LESS, Sass, Drupal etc), a more globally effective solution is to make respond.js marginally smarter. In my opinion, the performance loss is worth the broader application.

A possible solution is to create a Respond "plugin" that could implement the non-comment approach. That would keep the core Respond code clean/fast, and Respond would only use the plugin if it's required for a particular situation.

[pyrsmk]

I'm agree with you but I'm not sure that will be faster. Using a plugin, so write more code, will, generally, slow down performances.

On the other side, a logical implementation of the non-comment approach would be a parser which cut up the brackets to create a token tree. And, with the right functions, we can greatly divide the plugin integration performance cost. I don't know what about js, but I have developed a template engine, in php, with the aim of rapidity. I noticed the preg_split function has nearly the same benchmark as the "strpos implementation". Perhaps there are other possible optimizations.

[scottjehl]

Good feedback, thanks.

I'm certainly not advocating that anyone stop minifying their CSS. :) Minifiers like YUI allow you to pass exceptions for comments that should be preserved, so that's what I was referring to. This sort of thing is often used in JS too, for preserving portions of scripts that use conditional compilation (however improper that may be).

Anyway, I will experiment with a couple approaches to this problem when I get a chance.

Thx again!

[pyrsmk]

@scottjehl: But sass doesn't support excludes, as less maybe.

Anyway, good luck :)

[scottjehl]

Quick update: @rupl and @pifantastic are working on a a plugin for respond.js that parses without comments. I believe it's a bit heavier, naturally, but please check it out and see if it's working for you.

I'm interested in pulling something in eventually, at least as a plugin. https://github.com/rupl/Respond

Thanks!

[chuanxshi]

To use /*/mediaquery*/ is indeed an issue, currently, it gets stripped out by the boilerplate build which is based on YUI CSS Compressor that uses ! as an option to prevent stripping. An alternative option is to allow user to define the match option when init respond.js or something?

[scottjehl]

@shichuan that sounds like a nice intermediate step. Good idea.

[pyrsmk]

@scottjehl @rupl

Works nice in IE8 and not at all in IE7. Since I saw "Fuck IE7" in the commit comment, I think it was expected :D

[pifantastic]

@pyrsmk The "Fuck IE7" comment was when I discovered that you can't address specific indexes of a string in IE <= 7 (d'oh!). I replaced all usages of str[] with str.charAt(). This is to say, it should work in IE7. At least it does in my testing. I would be interested to see what case in which it is failing for you.

[pyrsmk]

@pifantastic Hu ok... So bad for the str[], it's very useful... To let you see, that's the site which fails with IE7: http://home.dreamysource.fr/dreamysource.fr

[pifantastic]

Sweet, thanks, I'll check it out.

[pifantastic]

@pyrsmk One point I will make is that you should load the parser before respond. That way respond knows about the parser and can use it for the initial page load. Aside from that, I'm still looking into why it isn't working in IE7.

[pyrsmk]

Indeed, invert both lines make all works ;)

[pifantastic]

@pyrsmk Awesome! Thanks for taking the time to test it out.

[pyrsmk]

That was a pleasure, I've many interests in Respond :)

[scottjehl]

so, @pifantasic: I'd like to see about officially integrating this into Respond.js, perhaps even doing away with the comment-based approach if we can keep things lightweight.

Want to work with me on this idea?

[scottjehl]

I did some more testing on the parser branch on a local project and unfortunately, I'm seeing some not-great performance. It works okay in the simple unit tests, but on a more real-world site, I'm seeing up to 4 seconds delay before the layout snaps in. I'm not sure walking the file will be the fastest approach, but it's great to have a working option for those who need it :)

Would anyone here have time to test the src from this branch in their own project? I'm seeing all but one unit test fail, so it seems viable (though it'll need a bit more logic before it's ready to handle all bracket scenarios).

I'm working on modifying the regexp so that it doesn't need comment tokens. It'd be nice to get a gauge for whether it's working yet in live projects. Thanks! https://github.com/scottjehl/Respond/tree/comment-free

[pyrsmk]

Walking could be greedy... Maybe a String.split(regexp) could be faster, much than a regexp match (at least in PHP, but I think it would be the same in js). I could make a benchmark to see what is fastest if you want.

I've tested the comment-free Respond.js and IE6/7/8 just hang and crash :s

[pifantastic]

A string.split to boostrap the parsing is a really good idea. I've been planning to implement something like that when I have time. The only complication that arises is the edge case of ending up in the middle of a comment or string that contains "@media".

[scottjehl]

@pyrsmk: it'd be useful to see the css you're using. I'm not seeing crashes when running the unit tests or a local client project (which is quite complex). Maybe you're using a combination of CSS rules I haven't accounted for. Thanks

[scottjehl]

btw - you may not have time for this one, but if you check out the comment-free branch and run the unit tests on a web server, are you seeing crashing there too, or just in your project?

[pyrsmk]

@pifantastic: It will be very rare to have content:"blabla@mediablabla"; and we can advise users that don't use @media in a comment. It's restrictive but it will work as fast as possible. Or else, you can build your regexp to match @media type[,type[,...]](params){, but it will be slower.

@scottjehl: Also, I made my tests with IETester (to have IE5.5 to IE10 directly). It's not really stable. But it worked with the previous parser version. Here's the file: http://pastie.org/1968274 . It's quite big, but there's also a base.css ^^

I runned tests with test.html but not the unit testing since it needs to run in a popup and IETester just opens me tabs ^^ With test.html, media queries don't work at all with IE6/7/8.

[thulstrup]

Using @font-face makes IE crash for me every time. @pyrsmk: Could you try testing without the @font-face rules in your CSS?

[scottjehl]

Good idea, @thulstrup.

@pyrsmk: Does your page crash without the @font-face rules? That'd be good to know...

[thulstrup]

Apparently IE doesn't like having a @font-face declaration inside a @media selector. Once I moved it outside the selector, IE started behaving again.

So it doesn't look like it's the same problem as @pyrsmk after all...

[pyrsmk]

Bingo! Without @font-face rules, my website works again ;)

[scottjehl]

hmm ok. So to clarify, you're using the comment-free branch, yes?

Interesting: "At-rules inside @media are invalid in CSS2.1." http://www.w3.org/TR/CSS21/media.html#at-media-rule

Not sure if css3 is so strict.

[pyrsmk]

Yes, I am :)

[scottjehl]

Good to hear. Well, if anyone's interested in helping, I'd really like to work out some of the edge cases for the code in the "comment-free" branch.

The code in question will be line 76 https://github.com/scottjehl/Respond/blob/comment-free/respond.src.js#L76

[pyrsmk]

In fact, you can't extract media queries with a js regexp. You'll need regex recursion and it's not supported in javacsript. For comment-free support you must match with regexes and walk a bit to find the good closing bracket, like:

var i=actual_position;
var nests=0;
var position;
do{
    if(contents[i]=='{'){
        ++nests;
    }
    if(contents[i]=='}'){
        if(--nests==0){
            // found!
            position=i;
            break;
        }
    }
}
while(++i);
if(position!==undefined){
    // it's right, it's found!
}

It's just an example but you'll see the matter.

Note: do-while loops and pre-(in)(de)crementation are known as faster :)

[pyrsmk]

This, coupled with a contents.split(/@media[^{]+\{/gmi) will do the perfect stuff.

[scottjehl]

Anxious to see an attempt at this. If anyone is able to work it out as a first pass, starting from the current code in master would be ideal.

Thanks everyone!

[pyrsmk]

I currently work on the parsing implementation (but not with the Respond src, to keep it simple). I'll give you some news later ;)

[scottjehl]

thanks!

[pyrsmk]

Done.

Here's the wide test: http://home.dreamysource.fr/dreamysource.fr

The script is at the bottom of the page. Sheethub is the CSS API I developed (if you remember). Consolelog is a wrapper to have a log() function through browsers. All the results are in the console :)

[scottjehl]

Thanks pyrsmk. So, I played with this a little bit, and while it does seem to match the correct bracket, I'm not quite understanding the difference between this result and the one provided by the following regexp:

styles.match( /@media ([^\{]+)\{((?!@media)[\s\S])*(?=\}(?![^\{]*\}))/gmi )

Seems to me that this does meet our needs: match all text after each @media statement that doesn't contain "@media" and is followed by a } that isn't followed by another } (forgiving whitespace in between).

If we were to take the following css: @media only all { body { background: red; } }

The regexp above would match this: @media only all { body { background: red; }

Which matches the comment-based match in master (the logic that follows in respond.js expects the final closing bracket to be excluded).

You may be seeing something that I'm missing here, though. Can you point out for me some situations where this would not meet our needs (aside from other bracket nesting scenarios we have not yet covered in either approach)?

Thanks!

[pyrsmk]

I'm not certain that a regexp is really strong for that stuff. Indeed, your regexp matched @font-face :) But you're right, matching a closing bracket after a closing bracket should be good.

Also, we could benchmark the two methods to watch what is faster. Logically, it's the split/walk method because there are much less code to compile.

[scottjehl]

Okay, so I've improved the comment-free regexp. It's now passing all unit tests in IE 6-8 and it renders a client site of mine flawlessly as well, which happens to have rather uncommon media query complexity.

That said, I'll need some testing help. Please grab the minified src and see how it fairs in your codebases. https://github.com/scottjehl/Respond/blob/comment-free/respond.min.js

Thanks so much!