I know morten likes to complain about the CSS in Drupal core. And while he does love arguing, its much easier to change the CSS in core if we have useful metrics to show why the CSS is crappy.

Stubbornella (Nicole Sullivan) has been a long-time advocate of simple CSS to improve front-end performance. Complex selectors take more time for browsers to match against them. Needlessly-complex selectors lead to crazy cold-war-scenario specificity escalation as each rule tries to out-specify the more general rules.

She has also just released an open-source tool called csslint that we can use to test our current CSS rules.

http://www.stubbornella.org/content/2011/06/15/css-lint-open-sourced/
AND
http://csslint.net/

There's bound to be some contentious rules in this lint tool (there always are in lint tools), but it gives us a starting point to measure.

For those wishing to run csslint locally, here is a suggested command line

csslint core --format=compact --exclude-list=core/assets/vendor/,core/modules/ckeditor/,core/vendor/ > lintcss-trap.txt

[edit: 2011/06/17]
Here's the current list of Drupal 8 core’s stylesheets. As specific issues are created for each stylesheet, we can edit this list to point at the issues.

Files: 

Comments

Stalski’s picture

Subscribe. Agreed as good starting point.

Maikel’s picture

Subscribe. Interested in seeing how this will be implemented.

Sidenote: Does this also mean that selectors (ids, classes etc) need to be altered too? Maybe interesting to look at additional ways to minify and cleanup the css code OOCSS for example. Or will that be a step too far for implementing in core.

fubhy’s picture

... And after that we just need to move the entire Core CSS into a "Core Theme"... Please?!

jyve’s picture

Subscribing. Sounds good to have a guideline to follow, especially in the world of CSS.

maartenverbaarschot’s picture

Would be extremely nice to have a built-in patch checker for this at drupal.org. Like we currently have for PHP tests. Ideally one checks manually before uploading the patch, but it could automate part of the reviewing process.

mortendk’s picture

+1 :)

dcmouyard’s picture

+1 Sounds good to me!

Hopefully this will lead to applying OOCSS principles to Drupal.

tlattimore’s picture

I am all for this!
+1

Jeff Burnz’s picture

Cool, over the past year or so I've become a big fan of lint tools, so +1.

I don't think there is any problem getting buy-in on this issue - so maybe my initial question is scope. What I might suggest is we start running our CSS through these lint tools to get a documented list of issues - this will give us some scope and may well offer some inspiration on how we can improve on the current situation with core CSS.

What I might suggest is..

1. get a list of all core CSS
2. create a list of suitable lint tools
3. create a meta issue and break it down into separate issues per module/stylesheet
4. run the tests
5. collate the results and see what we come up with

Could even be a sprint.

JohnAlbin’s picture

3. create a meta issue and break it down into separate issues per module/stylesheet

I believe that's what this issue is. :-)

1. get a list of all core CSS

I've got one of those in Zen’s drupal7-reference.css file. (Assuming no one has changed the D8 list yet.) I went ahead and put this in the initial issue description above to make tracking the changes a bit easier.

AdrianB’s picture

Subscribing.

Jacine’s picture

Issue tags:+Front end

There's an initiative page for cleaning up CSS here. It'd be great to add some info about CSS Lint and the core stylesheets to that page. I'll do it if no one else gets to it first. I also think it'd be awesome to get it in our automated patch testing as @mverbaar mentioned. Tagging "Front end" to get this on the radar of people that are interested in helping out with this via @drupalfrontend.

steinmb’s picture

+1

Jacine’s picture

The page I referred to in #12 was updated after the HTML5 Initiative meeting today. @johnvsc created all the issues and linked them in a table here for those interested in helping: http://drupal.org/node/1089868

dodorama’s picture

Oh yes

JohnAlbin’s picture

I'm going to try to resurrect this issue.

cosmicdreams’s picture

Happy I found this issue.

I'm going to "explode" this issue to each of the stylesheets this weekend (Saturday) if it's still a good idea. For the most part, this effort is inline with the CSS cleanup efforts of the HTML5 initiative. using CSSLint at http://csslint.net provides a quick way to code-sniff our css and tweak styles until they pass the tests.

So, as I see it, what needs to be done is:

1. Create an issue for each of the stylesheets that are mentioned in the OP
2. For each stylesheet copy and paste the it into the online tool and test it.
3. Fix any warnings or errors
4. Create a patch and see if the patch passes Drupal's automated tests.
5. Debate about the changes.

cosmicdreams’s picture

Issue summary:View changes

added link to online css lint tool.

ZenDoodles’s picture

Issue summary:View changes

Updated issue summary.

webchick’s picture

Probably. If it's the exact same "robotic" changes just in lots of different files though, it might be easier to review/commit as a single patch. If we're talking a 500K+ patch, then probably not since the re-rolls alone would be really irritating.

Might as well try it and see what we end up with then go from there.

tlattimore’s picture

Thanks a lot for chiming in on this, webchick. I anticipate it will likely be a very large patch if all the changes a rolled into one. There has been a great deal of effort to clean things up (noted in the issue summary). But my guess is that css lint will still have a boat load of changes suggested.

tlattimore’s picture

Issue summary:View changes

Updated issue summary.

ZenDoodles’s picture

Title:Use csslint as a weapon to beat the crappy CSS out of Drupal core» Meta: Use csslint as a weapon to beat the crappy CSS out of Drupal core
RobLoach’s picture

I ran CSSLint through node, and this is the result.

droplet’s picture

Ahh. that's good.

There also a CSS Cleaup mission. Some of them are duplicated issue
http://drupal.org/node/1089868

barraponto’s picture

Aren't we cleaning up the core themes CSS as well?

barraponto’s picture

btw, here's how I roll: find core/modules/openid/ -type f -name *.css -exec csslint {} \;

barraponto’s picture

Issue summary:View changes

Updated issue summary.

kerasai’s picture

Issue summary:View changes

Updated issue summary.

greggles’s picture

Issue summary:View changes

removing openid issue - it's still valid in the contrib openid but not relevant for this core meta-issue.

LewisNyman’s picture

I just created #2222049: Add a .csslintrc file that's in line with our CSS standards. It would be nice to get some opinions please

LewisNyman’s picture

Issue tags:+CSS, +frontend
LewisNyman’s picture

StatusFileSize
new1.42 KB

So #2222049: Add a .csslintrc file that's in line with our CSS standards is now committed which means we can use it to get better feedback from CSSlint that's inline with our standards. I've attached the patch for a grunt task you can run that pulls in the csslintrc file.

LewisNyman’s picture

Component:markup» CSS

I had a look over the current status of CSS in core, here are the results:

https://gist.github.com/lewisnyman/10c756cb6f7a204485ca

mortendk’s picture

auch - well now we know what to do the next couple of weeks ;)

cosmicdreams’s picture

Making progress with ids is an interesting challenge. These ids have been deeply rooted for a long time. Should we endeavor to eliminate them?

In the meantime Lewis, it might be good to put the gulpfile you're using to automate the css linting. I assume you also have a package.json that defines how to import the necessary packages with bower.

Establishing a project like that would make the future work faster for all of us.

mortendk’s picture

ID's are bad practice they always have and always will be.

LewisNyman’s picture

@cosmicdreams Funny you should say that because I started working on something like this today ;-) https://github.com/lewisnyman/drupalcore-frontend-toolkit

I don't know if it's completely usable yet but feel free to give it a go.

RobLoach’s picture

There's now an HTML Report up at http://lewisnyman.co.uk/drupalcore-frontend-toolkit/ so you can get an overview.

tim.plunkett’s picture

Title:Meta: Use csslint as a weapon to beat the crappy CSS out of Drupal core» [meta] Use csslint as a weapon to beat the crappy CSS out of Drupal core
mortendk’s picture

it should be called "Drupals css wall of shame" ;)
lewis its very usable and gives us a really good tool to find all the little things, that probably needs a cleanup (like the use of important etc)

mortendk’s picture

and for those that are running it locally heres my node csslint
csslint core --format=compact --exclude-list=core/assets/vendor/,core/modules/ckeditor/,core/vendor/ > lintcss-trap.txt
yes im ignoring venor, cause we have enough crap as it is b our self ;)

martin107’s picture

Issue summary:View changes

Move, along nothing to see here... just moving the command line suggestion into the issue summary, just to save me digging around for #37 later. :-)

LewisNyman’s picture

The status as of February 10th:

57 lint free files out of 145
846 errors

LewisNyman’s picture

The status as of February 12th:

62 lint free files out of 145
802 errors

mortendk’s picture

83 files on the wall - 83 files on the wall, clean one up & sent it to review
82 files on the wall ....

LewisNyman’s picture

Title:[meta] Use csslint as a weapon to beat the crappy CSS out of Drupal core» [META-789] Use csslint as a weapon to beat the crappy CSS out of Drupal core

Today's status:

63 lint free files out of 144
789 errors

LewisNyman’s picture

Title:[META-789] Use csslint as a weapon to beat the crappy CSS out of Drupal core» [META-782] Use csslint as a weapon to beat the crappy CSS out of Drupal core

Today's status:

63 lint free files out of 144
782 errors

dawehner’s picture

Well, do you really want to fix every single issue with their own issue?

LewisNyman’s picture

Well, do you really want to fix every single issue with their own issue?

Our strategy at the moment is to fix individual files/components. It becomes very hard to test regressions when we have one patch that fixes one problem across all files. Some files have 40+ errors in them, so we can be quite productive still.

mortendk’s picture

we need to go through each file & clean them up, that at the same time have the advantage that we then as well quickly gets into cleaning them up as well with the MAT filenaming & putting the right css where it belong. That should if the might Drupal gods wants it, make it easier for us to clean the css even more

Jeff Burnz’s picture

The issue for Forum CSS linked to in the IS is postponed on an issue that itself is postponed to an issue that is now closed and didn't really appear to fix anything - the closed issue says changes were merged into the D8 mobile initiative branch but I have no idea what happened to that since those CSS changes are not in core: #2028049: Clean up the CSS for Forum module, can we re-open #1217002: Clean up the CSS for Forum module and link to in the IS?

LewisNyman’s picture

Yep that makes sense, looks like there are a few issues that were closed as duplicates that we could reopen and tag correctly?

#1217054: Clean up the CSS for User module #1217052: Clean up the CSS for Update module #1217048: Clean up the CSS for Toolbar module #1217040: Clean up the CSS for Simpletest module

mgifford’s picture

Can we remove #1663144: Clean up css in overlay?

Are we worried about D7 at this point in this issue?

LewisNyman’s picture

@mgifford I guess this meta s only for 8.x so we can remove the reference for the overlay issue?

LewisNyman’s picture

Title:[META-782] Use csslint as a weapon to beat the crappy CSS out of Drupal core» [META-780] Use csslint as a weapon to beat the crappy CSS out of Drupal core

Todays status

62 lint free files out of 143
780 errors