Counting selectors does not appear to be accurate

[brian-mann]

When I run our css file through http://snippet.bevey.com/css/selectorCount.php I get a count of 4279.

When I use the count_selectors class method and pass in the same CSS file, I get a count of 2561.

I'm inclined to believe that the selector counting algorithm of this gem is incorrect, because IE is missing styles on our stylesheet, and thus I know it's exceeded 4095, as accurately indicated by the first link. :-)

Here's the gist containing our stylesheet. https://gist.github.com/4478290

This is obviously after its been compiled into one giant manifest. Since this gem counts our total number of selectors incorrectly it won't split it into two files, which is what we desperately need.

I haven't spent a lot of time on it yet, but my off the cuff guess would be that its only counting the total starting/ending curly braces and not taking into account selectors above the curly braces that are comma separated. We have a ton of those further down in our stylesheet. I'll dig into the source and see if this is the case or not.

[jhilden]

Thanks for the bug report @brian-mann

I added a test for the methods in question using your example stylesheet and actually did find a bug which resulted in the first rule with 379 selectors not being counted correctly. So the "more correct" count is now 2939 instead of 2561. See 66a71f7

However, besides that I wasn't able to find any other issues with the code. Too bad, that the code of that PHP selector counter is not open source for easy comparison.

I also tried to do a quick & dirty, "manual" selector count of your CSS file and the result was also ~2900. What I did was just some simple text-editor-regex-replacing to put each selector on its own line and remove everything else. See: https://gist.github.com/4479280 (But that was only quick & dirty and mistakes can't be ruled out).

The only other possible culprit I can think of right now is the comment stripping (https://github.com/zweilove/css_splitter/blob/master/lib/css_splitter/splitter.rb#L97), because I used the same regex during my "manual" counting. But that is just a guess.

I would really appreciated any additional information on that issue.

[brian-mann]

Okay I'm going to dig deeper and manually go through the file until I can find the areas causing the discrepancies between the php version and this splitter. Then we'll work from there to see how that could be.

[brian-mann]

So some updates....

I 100% determined that the php script I referenced earlier is NOT stripping comments, and thus its total selector count is WRONG. I originally thought the php script was correct because IE8 was not recognizing selectors in my CSS file, but after looking into that more, I can't recreate that problem, and it could have been a simple CSS specificity issue causing my styles to now show up....

Nevertheless, I dug into this more, and setup a test environment for IE to ensure that we could make sure we are counting the selectors properly within this gem. So with my test css file, I continued to add more faux selectors until I pushed IE over the limit by exactly 1. I am 100% sure this is exceeding the limit by exactly 1 selector. The gist for this CSS file is here:

https://gist.github.com/4480951

What's interesting about this css file is that if this gem did indeed count the number of selectors properly, then we could call it a day. However when I call the method: count_selectors I get 4172 selectors returned. This is obviously too many selectors, as I'd expect to get exactly 4096 selectors returned, but interestingly this gem does NOT create a 2nd split version of the excess selectors. So because it wasn't creating the 2nd CSS file, I investigated further.

When I patched the split class method, and told it to output the selector_count variable, I got a total count of 3794. So that's why its not creating the 2nd split CSS file. But these two methods obviously count the number of rules differently, and neither of them get the count right, which I would have to assume should be 4096, since based on my IE tests, I can confidently say we're over by exactly 1.

I think perhaps the media queries https://gist.github.com/4480951#file-ie-selector-bug-L12960 may have something to do with it, but I don't have anything to back that up. The good news coming out of this is that I can confidently say that this gem does strip comments correctly, and that comments themselves do not count as selectors to IE.

[brian-mann]

To add one last little tidbit...

When I turn on debug_info in the sass_options (which outputs a media query per selector) - which FireSass can look at, and thus tell you where the selector was found, it obviously creates more than 2x the selectors, and for us that puts us at around ~8800. The benefit is that you can use FireSass plugin, the downsize is that this would require 2 additional split css files, instead of just one, since this exceeds 8190.

In the readme it discloses this gem only creates 1 split CSS file, and suggests if you need more than that, your CSS probably needs to be refactored.

At least now I've presented a use case where the total selectors you have can instantly double to help with debugging, and would not be a condition for refactoring. :-)

[jhilden]

Thank you for all the debugging work you did @brian-mann

First, concerning the 8190 selector limit, if you really need more than 2 splits all you need to do is to register another file extension with Sprockets like this:

app.assets.register_engine '.split3', CssSplitter::SprocketsEngine

Concerning the wrong counts, I also suspect that it has to do with the nested media queries. The simplistic example stylesheet from the test dummy app is definitely split correctly (https://github.com/zweilove/css_splitter/blob/master/test/dummy/app/assets/stylesheets/too_big_stylesheet.css.scss). We will just have to dig deeper, what the deal is with those media queries.

[jhilden]

Hey Brian, I added the stylesheet in your gist to the test dummy app and it did split it at some point. So there must have been something going wrong when you tried to split it (maybe some bug that has been fixed in the meantime, or you missed something in your configuratoin).

You can find the second split here: https://github.com/zweilove/css_splitter/blob/master/test/dummy/public/assets/test_stylesheet_with_media_queries_split2.css
However, it includes exactly 76 selectors so there is still some issue somewhere (probably related to media queries).

[jhilden]

Closing this in favor of #9

@brian-mann did you have any further issues or more information on the existing ones?