Whitespace below the footer when there is not enough content on the page

some browsers have this problem (but not all) and require a background-color{} to be applied to the html like this:

html {
  background-color: #000;
}

that was easy....

Also, my worries about the color module stuff was unfounded because the footer is not effected by the color module.

oops ... forgot about overlay ... please hold

ok ... this fixes the whitespace AND doesn't mess with overlay

nevermind ... this does not fix the issue. Still workin

this gets us really really close. It fixes the main issue described in teh original post, but it leaves a black bar at the bottom of the overlay (if and only if Bartik is the admin theme) see here: http://skitch.com/bleen/dyawj/d7

thoughts?

Great work bleen18, it looks a lot better. I came to the same conclusion about the overlay. I cannot find a way to target the html that's only in the overlay, I've been wrestling with it for about 45mins and can't figure it out.

Ok, try this. I tried with a few color schemes and it's looking good, but I would like a review before committing this.

Awesome, tested it with several color schemes and it looks great.

looks good in FF 3.6, Safari & Chrome

Nice job. RTBC++

Great, thanks for testing. Committed :)

There have been some changes to color.css, it's now colors.css. Not sure if it was the initial name change made in #783698: rename color.css to colors.css (and fix it in .info), or some of the work done in #783574: Clean up color module settings page, but the changes from the patch in #8 here are no longer present?

To fix this I simply changed the index information from the patch in #8 from color to colors, then applied the patch and then the changes were made again. I attached screenshots of before and after the patch was applied, along with the patch.

This needs to be tested with some WYSIWYG tools. Can someone do that and report back?

Oh and good catch on the colors.css file name change. Yeah that got changed earlier tonight to solve a different problem.

I forgot to mention, I looked around and apparently some of the wysiwyg modules have a setting for this... Like, use the theme background or use the editor background, so hopefully it's good.

hmm, wait... missed #12 before.

@tlattimore This is working fine right now for me. We had a color file name change, but that was after I committed this, so maybe you just needed to clear your cache?

@jacine - This is really strange then. I tested it yesterday and it looked great, but now its looking off again for me ever since the file name changes. Are we sure the things weren't altered when the files name was changed? It is really odd that it is off for me and not for you, FF 3.6 in Windows.

Cache cleared, the problem is only fixed when I apply the #13 patch. Check out the screenshots from #12 for before and after I applied the patch.

http://drupal.org/files/issues/bartik_before_patch.png - Before

http://drupal.org/files/issues/bartik_after_patch.png - After

I will test it and investigate further tomorrow.

Oh, gosh you are right! Sorry, not sure how I missed that, but it seems the changes were reversed somehow with the file name change. I just committed them again. Hopefully it's all good now. :)

It's fixed! Thanks Jacine.
I will go ahead and doing some testing around with some WYSIWYG tools as requested in #13 tomorrow, though by your statement in #14 it sounds like there shouldn't be any problems.

I have yet to actually install a WYSIWYG on a D7 site, : o , so it probably wouldn't hurt for me to go ahead and do that and perform some testing while I am at it. I will report back if I run into any problems.

Stil an issue apparantly

The whitespace below the footer is not always white :))) Actually it is white by default, but it can be overridden by selecting a different "Main background" color through the Color module.
Speaking in Color module terms, the theme's "Main background" is used as background-color for html, #page-wrapper and body.overlay. Instead the "Footer background" is used for body and #footer-wrapper — see themes/bartik/css/colors.css :

/* ---------- Color Module Styles ----------- */

body {
  background-color: #2d1e0f;
  color: #ffffff;
}
html,
#page-wrapper,
body.overlay {
  background-color: #ffffff;
  color: #3b3b3b;
}

      // some rows skipped...

#footer-wrapper {
  background: #2d1e0f;
}

The problem is that the html tag's background-color appears in the empty space below the footer/body when there is not enough content on the page.
The attached patch uses the same background color — "Footer background" — for html, body and #footer-wrapper.
Tested in Opera 10.60, Firefox 3.6.8, Chrome 5.0, Safari 5.0 and IE8.

Yeah, regression was caused here: #810300: Resizing and Zooming can make text unreadable.

So we need to think about his a bit more.

There's a better solution for this same problem being worked on at #888412: Make footer stick to the bottom of the page. The problem with putting the main background color on html is that then it shows up as the background color for WYSIWYG editors. :( THis other solution doesn't have that unwanted consequence.

So — even though this issue is older, and the new one is the dup, I'm going to close this as a duplicate. Let's work on the sticky-footer solution.

There is quite some discussion going on in the sticky footer topic, and imho they do feel like 2 separate issues: this one being a bug, sticky footer being a feature request.

Attached patch needs extensive testing, it seems like it solves the issue at hand, but it might also club baby seals. Apparently height works kinda like min-height for body and html. min-height does not work at all.

Clever... and good :)

Going to test some more cross browsers.

I knew I had to test more carefully ;)
Now he page is too long, there appears a scrollbar in every browser.

I don't get a scrollbar in opera 10.6, firefox 3.6 or IE8. Unless there is more content to be scrolled to :).

Ever tried to login or to enable toolbar ;). I think the toolbar adds some offset.

** double post **

Ah right. That's the baby seal clubbing part. Have to think about it.

Different approach. Reverts the patch commited in #810300: Resizing and Zooming can make text unreadable., effectively solving this issue again. Also fixes #810300: Resizing and Zooming can make text unreadable. by using a more specific selector (instead of html, #main).

Shouldn't club baby seals :), but needs testing for both these issues!

Still clubs my baby seal...

EDIT: srry I forgot to revert the previous patch. My baby seal is ok now.

Same screenshot as posted above, without a scrollbar.

Setting background colors and font colors can never be the reason for scrollbars. Different issue then.

Ok cross browser testing.

chrome: OK
firefox: OK
IE8: OK
opera: OK
ie7: NOT OK :( ==> scrollbar area appears (no scrollbar just the area)

again, scrollbars cannot have anything to do with my patch. The patch is only about colors and font colors. Hence, the patch should be tested for no white space below the footer, and for this one: #810300: Resizing and Zooming can make text unreadable..

IE7 showing a scrollbar is a different issue, or just quirky default behavior of IE7.

Currently the white below the footer is the html color being displayed. These changes display the body(and body color) below the footer, which might be a problem if body and footer have different colors.

ps. body{background-color} is set in both stye.css and color.css, and to different colors.

@Kiphaas7 probably you meant that your patch in #32 reverts the patch committed in #810300: Resizing and Zooming can make text unreadable.

IMO #32 is the right approach. It should also prevent the background color issue with WYSIWYG editors mentioned by jensimmons in #23

Err, yes I did >.<. Updated my post to not cause any confusion.

Well I think we can't allow any visual regression this late in development cycle. And adding a stupid scrollbar where there shouldn't be one on a (still) popular browser is a no go for me.
Maybe webchick or dries don't agree with my opinion, just felt that I needed to say this.

If you patch causes a regression in accessibility that is unwanted then you MUST account for that regression in the new patch and fix both. Jen has already indicated not wanting to go this route yet you are persisting - please do not revert accessibility for the sake of design - you must account for both, and if you can't, accessibility always wins.

Reading above I believe you have accounted for both, but its not clear if this is widely tested, or what browsers?

Why is there a scroll bar in IE7 that is being reported - any hint of why this might be happening, cant see anything in the patch that would cause this.

Please provide screen-shots to show us what this does - its much easier to scan these issues when there are screen-shots.

The fix in #32 is going to work for text in the main content area, but will possibly relate to #845834: Fix Bartiks Header because its totally borked as it does not account for overflow of text from the header - thinking we should hold off on a commit for this until we decide on the header region positioning issue in case we have any issues with overflowing text, otherwise this is a good compromise should we abandon the sticky footer approach.

The disabled vertical scrollbar in IE7 is not related to the patch in #32. In fact it is also displayed with the patch reverted – also after switching the theme to Garland.
The disabled vertical scrollbar was IE default behavior (bug?) until version 8, so probably it could be left as it is. Otherwise a separate issue should be filed and a possible fix would be to add html { overflow: auto } in IE conditional style sheet... hoping that it doesn't club baby seals.

#43: the overflowing text in main is a problem, because dark text is overflowed onto a dark background. In the header region, there is white text, so overflowing text there should not give any problems on the dark background. Whether or not it looks good is verse 2, but #810300: Resizing and Zooming can make text unreadable. never was about making it look good, just to fix accessibility. Or did I miss something in that topic?

#44, You are probably right. If it is default behavior in IE7, then it should not need fixing, and if there is a need for it, it should be fixed in a separate issue. Slightly offtopic: I actually prefer to always have a scrollbar, since it stops content jumping when there is a sudden need for a scrollbar to be shown.

Incidently, it looks like the child item containing the text is larger than the container allows it to be after zooming in. Nice images and explanation related to this problem here:
http://www.brunildo.org/test/OverflowSide.html

I am getting the same issue. (screenshot attached)

I think the solution is setting html, body and maybe a few other elements to Height: 100%;

Fixed it by setting the body background colour to the same as the footer colour. Although Jen has told me I can't do that due to a WYSIWYG issue.

RE #47 — yeah, that was the first approach I took months ago. But then it was reported as a bug in #819218: Body background color is using the footer background color. And we had to remove the footer background color from body.

(Or something like that. It's hard to remember all the back and forth now.)

jensimmons and timmillwood, did you try the #32 patch?

I haven't actually tested, but #32 will stop working if the overlay module is disabled. That's no good :(

Bleen, I don't know how you get that idea. Again, the only thing the patch does is swap out html in favor of #main. #main does not have anything to do with the overlay!

To emphasize, these are the only changes in the patch:

-html,
+#main,

Attached is a screenshot of the patch on a recent drupal 7 dev, with the patch applied and overlay off. Prior testing was done with overlay on, and I couldn't detect any differences. Please, elaborate on why you think overlay would break this patch.

Screenshot also shows the only desired effects of patch in #32.

I think this is a good fix, its basically the same fix we had before but more precise. Lets be real here peeps - this is real simple fix and the drawbacks are very few, in fact we're talking about an extremely rare edge case accessibility issue - it certainly beats using JS for a sticky footer or similar.

Tested in IE8, Chrome 5, Opera 10 and Firefox 3.6. I cannot test in IE7 or 6 because in my test environment proper text zooming simply does not work correctly for these old school browsers (win7).

If someone can pass this in IE6 and 7 I think we we're good to RTBC.

Tested in IE6. Both white footer and zoom accessibility fixed. As a note, zoom accessibility is not an issue here even with the patch reverted.
I don't have IE7 installed, but the patch passes in IE8 with Compatibility View On (IE7 mode).

I think this is safe to commit.

my mistake ... I misread the patch

Just tested in IE6 7 & 8 .. all good

Committed to CVS HEAD. Thanks.

In case anyone is scratching their heads as to why this change doesn't seem to affect your re-colored theme: you have to re-save your color modifications for it to take effect.
Only affects those using D7 before this patch, probably doesn't need any documentation.