Why should drupal.css control this font size? It shouldn't.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chrismessina’s picture

There's a lot of things drupal.css shouldn't do and this is just one of them. There should be ZERO font tags in it, and the number of floats and clears are enough to make a grown man say "Shoot!".

Yeah, that's how bad it is.

Dries’s picture

Agreed. Can we tidy this up further?

adrinux’s picture

Like many theme developers I think drupal.css is a pain and ought to be removed :)

There are however some parts of it that would need to stay, for instance ensuring the correct font in the user login box is a security issue (as Steven pointed out to me).

While we are on the subject of drupal.css I'd like to suggest we move it. I got quite confused making my first theme at one point because my styles weren't working as expected and eventually remembered drupal.css - I'm sure I'm not the only one - could we move it somewhere more obvious? Into the themes directory for instance?

Secondly I think the presence of drupal.css adds to the "all drupal sites look the same" problem, because things like book navigation work without the theme developer making any effort it tends not to get experimented with (I'm guilty of this myself). Plus it forces you to redefine a lot of styles quite specifically to fit your theme, adding to the verbosity of your stylesheet. So I'm all for stripping out anything that isn't necessary, though we'll probably also have to provide patches for current themes to stop breakage.

I'd happily do some work on drupal.css...if drumm isn't already?

Chris Johnson’s picture

+1 to Adrian's ideas.

I was debugging a style sheet for a theme a couple days ago, and in addition to the slightly increased challenge posed by all the various styles which need to redefined, I was struck by the verbosity and size of it all. There are styles for things in drupal.css that are not even enabled on my site.

Doesn't all that extra unused CSS slow the rendering of each page fractionally? The stylesheets are cached by the browser, so at least they are not downloaded on each page, right? But they still have to be read off local disk and parsed, don't they?

Robert Castelo’s picture

Like many theme developers I think drupal.css is a pain and ought to be removed :)

Please count me as one of the anti drupal.css theme developers!

There are however some parts of it that would need to stay, for instance ensuring the correct font in the user login box is a security issue (as Steven pointed out to me).

Right, it should be stripped down to absolute basics.

While we are on the subject of drupal.css I'd like to suggest we move it.

Very true, all theme elements should be in the theme directory.

...we'll probably also have to provide patches for current themes to stop breakage.

In the case of Pushbutton it will probably reduce the size of the CSS file, as I had to override a lot of the drupal.css rules (e.g. http://drupal.org/node/8970). I'll be happy to update Pushbutton once drupal.css has been trimmed.

Steven’s picture

The advantage of having drupal.css contain default styles for tabs and such is not to be underestimated. Many people are still new to CSS theming and don't know how to turn an unordered list into tabs. If we move CSS around, we should put clear comments in the themes.

Also, in spite of drupal.css' rules, it is pretty neutral. I think it's only advanced theme developers that really want to get rid of it.

jhriggs’s picture

I don't feel quite as strongly as others on this topic in general, but one thing I do feel strongly about is that drupal.css should not (unless completely unavoidable) use any clears...and probably any floats. I have been bitten several times by the clears in drupal.css when trying to do pure CSS layouts that involve floats for the sidebars and/or main content. Clears should be left to the theme where the results are known.

Morbus Iff’s picture

I'll throw in my two bits of agreeance - the clears have been a problem for me as well.

Steven’s picture

Clears cannot simply be removed. Take for example the node form. The administration options have to be left floated (otherwise they each cover the full width as they are block-level elements). The regular form has to have a clear: both; in order to stay below the admin options. If we move this to the theme, then we add extra, unnecessary stuff to theme author's work.

I have encountered some situations where clearing was a problem, but this was in someone else's template. If you're having trouble getting a clear to apply only to certain elements, set the containing div to "float: left; width: 100%;". Apparently IE has some troubles with applying clears up the DOM hierarchy.

jhriggs’s picture

I am not advocating blindly removing clears. Rather, it may be worth revisiting why some of them are currently needed (i.e. revisit how some of the layout is done). Note that not all of the clears currently in drupal.css are required in my experience.

Bèr Kessels’s picture

My idea on this:

We need a drupal-clean.css that does nearly nothing. It will not even make ULs and LIs into tabs. That drupal.css should be just enough for the CSS savvy theme developer. (who says i want the admin form things besides one another and not under one another? So why should i be forced to use floats on those?)

For Joe Designer, we need a drupal.css with all the things, like clears etc.Quite similar to what we have now. In fact it could be a drupal.css liek we have now, thats is only a little bit cleaner.

Default themes will include the drupal.css, theme developers who want clean styles to start with (like me) are perfectly able to change the filenames of these two css files.($ mv drupal.css drupal-bloated.css && mv drupal-clean.css drupal.css)

Dries’s picture

I commit drumm's tiny patch but I'm leaving this issue open.

adrinux’s picture

Title: Drupal.css shouldn't size monospaced fonts » Make misc/drupal.css optional
Assigned: drumm » adrinux
Category: bug » feature
FileSize
2.57 KB

From discussions on node/15792 and other threads plus #drupal it seems we can reach a compromise over drupal.css. We can remove the
to drupal.css that gets inserted by drupal_get_html_head() and move it to the themes.

node/15792 talks about some of the logic behind this patch a bit more verbosely.

This attached patch removes the link element for drupal.css added by drupal_get_html_head() and moves it to each of the core themes, so each theme now needs this to include drupal.css:

<link rel="stylesheet" type="text/css" href="../../misc/drupal.css" media="all" />

(Note: drupal.css is also referenced in update.php (line 47), this patch doesn't touch that, I can see no problem with that at the moment.)

Now anyone that wants to remove drupal.css from their themes simply has to alter the theme template, removing the link to drupal.css.

adrinux
adrinux@gmail.com

Robert Castelo’s picture

Nice approach Adrianux - theme designers who only want to make a variation on Drupal's default style can include the link, more ambitious designers can omit the link and just transfer the bits they want from drupal.css to their theme style sheet.

A good solution for everyone.

adrinux’s picture

Neglected to say you can apply the patch to a checkout of CVS head by dropping it into the Drupal folder and doing:
patch -p0 -u < 20050123-HEAD-optional-drupal.css.patch

Steven’s picture

We should probably use @import in the style.css, as this way the choice to use drupal.css is tied to the theme and not the HTML template.

However, -1 on this patch until drupal.css is actually cleaned up. The two issues are tied together.

Steven’s picture

By the way, I'm leaning towards keeping all clears. The main issue people have with them is that they will cause content to drop below the sidebars. However, this is the result of bad CSS:

One way of doing sidebars is to float them left/right before the content starts, and to give the main content margins or paddings left/right which the sidebars will cover. This is bad, because conceptually, the sidebars and main content are simply 3 columns sitting in a container. By floating the sidebars inside the main content, the sidebars become structurally part of the main content rather than sitting beside it. The clears inside the main content will take the sidebars into account and will cause the unwanted float drops.

The solution is the put the main content inside a floated wrapper too. Some like "float: left; width: 100%;" works, but even better is to use source ordered columns instead, where the main content is loaded before the sidebars. Typically these use nested wrappers which include 1, 2 then 3 columns. They'll stretch correctly, and clears inside one column (be it main or sidebar) will only affect other content in that column.

This is the approach taken by Piefecta (fixed width, source ordered columns) and by Bluebeach (fluid width, source ordered columns).

clairem’s picture

By the way, I'm leaning towards keeping all clears. The main issue people have with them is that they will cause content to drop below the sidebars. However, this is the result of bad CSS:

...

The solution is the put the main content inside a floated wrapper too. Some like "float: left; width: 100%;" works, but even better is to use source ordered columns instead, where the main content is loaded before the sidebars.

I think that there is an interesting discission to be had about the "correct" way to do sidebars: I would be probably argue against some parts of your prescription, but I suspect that the full discussion is not really appropriate to this thread. I'll just say that my source-ordering deliberately has navigation links before content, with a "skip navigation" link at the very top of the page.

But whatever the merits of the pre-floated sidebar, it is a very widely-used technique (and a much better than one than using full-page tables!), recommended in plenty of CSS guides.

If its use is deprecated, the theme designer's handbook should explain that ... but it's not nice for the theme designer to find their theme breaking without explanation.

clairem’s picture

Clears cannot simply be removed. Take for example the node form. The administration options have to be left floated (otherwise they each cover the full width as they are block-level elements). The regular form has to have a clear: both; in order to stay below the admin options.

This was one place I got caught in designing my layout, but the eventual cure was simple.

No need to clear the regular form, just set its width to 100% and float it left:

.node-form .standard {
  display: block; 
  width: 100%;
  clear: none;
  float: left;
}
chrismessina’s picture

1 to gutting drupal.css AND its clears!

While Steven may be correct about his layout approach, by no means should Drupal be prescribing THE way to code your CSS. I’ve used the negative-margin because of sidebar background issues. You can’t just say “float everything” because you can’t use sidebar backgrounds with that technique (a problem Drupal.org avoids by not having any backgrounds!).

So I would recommend this:

  • Document best practices for columnar layouts for CSS/theme designers, both using and not using drupal.css
  • Document the issues that are created for non-floated layouts that use drupal.css
  • Document every drupal.css rule
  • Document source ordering best practices; as clairem pointed out, occassionally you want to have a sidebar block or navigation section come first. I’m all about putting the main content first as a general rule, but not every site is going to adhere to the standard main-content, secondary content standard.

That said, I’ll get to work on gutting drupal.css as I promised I’d do a long time ago!

Bèr Kessels’s picture

FileSize
0 bytes

This patch removes the calendar stuff. event module has its own CSS. archive.module should not enforce colors etc. drupal.css is about usability, *not* style!

Bèr Kessels’s picture

FileSize
0 bytes

This patch removes colours etceteras. All the styles I removed enforce colour etc, drupal.css is about usability, but not about colours and styles.

Thox’s picture

+1 for drupal.css to be optional to themes.

I don't appreciate having to hack my way around everything in drupal.css for all my CSS-based themes. I end up literally having a whole section in my CSS file simply to remove the drupal.css styles!

adrinux’s picture

Is it just me or are Ber's patches empty?
Drupal says they are 0 Bytes and Firefox won't download, hard to comment without them.

Bèr Kessels’s picture

FileSize
7.48 KB

They are empty. odd. attaching again

Steven’s picture

Such a patch is useless without porting the removed styles into the core themes.

Bèr Kessels’s picture

which of the styles i removed is required then? I removed the styles that are silly, cluttered or inappropriate. Please comment on the patch in more detail to discus which styles you think are required.

adrinux’s picture

Indeed, this patch will cause major breakage without themes being modified.
I list of what needs to be added to contrib themes would be handy too...
And most of all cross browser testing, I suspect many of the more frivolous looking styes fix things up in crumby browsers...but time will tell.

As for the patch itself:
-1 for removing:
img {
border: 0;
}
This is exactly the sort of nice general low specificity rule that most themes would use anyway and is easy to override. Ditto the table highlighting.

-1 for the addition of comments to blatantly obvious styles, e.g. "watchdog colours" for rules with selectors like tr.watchdog-user - surely it's obvious this is for the watchdog!

+1 for moving module styles (aggregator, book, calendar, forum, search) to module specific sheets
+1 to the rest of the tidying

drumm’s picture

I see the need for themers to remove drupal.css all together as a quick reaction to the fact that it simply does too much. I think we should follow the path of removing the offending rules and if what is left is still bothersome then we can talk about making it optional. There is a good chance we will have to actually make it optional in the end, but I don't think we know enough right now.

With each style removal we need to put in appropiate replacement rules for the core themes. We need to be better about updating the theme upgrading guide as well. In the future I would like to see a good comparison of what changed in drupal.css and the core themeable functions.

We shouldn't muck up this discussion with bringing in things like the potential "admin theme" (sure, a lot of people have agreed on the general concept, but it is potential until someone starts working on it). If we try to do everything nothing gets done.

drumm’s picture

Status: Active » Closed (won't fix)

The theme can intercept the stylesheet inclusion of drupal.css using the theme_add_style() themeable function.

factoryjoe’s picture

Drumm's solution is hackish, but I used it on my theme and it was sufficient. This should be well documented though, so that other themers know that there is a solution for this problem (note that I also removed the event styles).

function phptemplate_stylesheet_import($stylesheet, $media = 'screen') {
  if ($stylesheet == 'misc/print.css') {
    // Return custom print stylesheet
    return theme_stylesheet_import(path_to_theme() .'/styles/print/print.css', $media);
  }
  else if ($stylesheet != 'misc/drupal.css' && $stylesheet !=  drupal_get_path('module', 'event') .'/event.css') {
    // Gut drupal.css. Good riddance!
    return theme_stylesheet_import($stylesheet, $media);
  }
}

Let's just hope that no themer ever names their stylesheet "drupal.css".

Bèr Kessels’s picture

The simplest solution stuill is to simply replace drupal.css with an empty file. Well, simplest - after proper handling of that much-hated drupal.css.

Gábor Hojtsy’s picture

As Ber asked for some comments on the list, here are mine:

  • Submitting a patch for only drupal.css to remove several styling stuff, without adding these to core themes is quite a bad idea. There are no modifications in core themes in this patch.
  • The comments are definitely not cleaned up in this patch, there are some module headers added, but not for all modules. Some examples:
    /* required to allowbetter floats. but in fact needs some improvement */
    /* Pager styles*/
    /* Pager styles*/
    /*
    ** Module specific styles
    */
    
  • Some module specific styles are left, some are not. Like book nav, and book tree are kept intact with all their border, padding, etc. rules, while book name, book title, and even book itself are removed. Why?
  • There are a lot of colors, padding and sizes removed, but stuff like watchdog colors, and the above mentioned leftover styles are still here? Where could we find the consistency?