Garland shows script source in header

Fixed in CVS commit 47804.

This turns out to be caused by the Garland theme. Removing line 285 ("display: inline;") from style.css will fix it, but I don't know what the side effects would be.

Make sure you're commenting CSS correctly: replace "display: inline;" with "/*dispaly: inline;*/".

I can confirm this issue and have been unable to find a solution myself. I'm also using Garland, but on Drupal 5 RC1 instead of beta 2.

I had the problem as well, using Drupal 5.x dev, downloaded on 2007-01-01.

I have successfully removed the visible Javascript code (site is http://arancaytar.ermarian.net, the code was directly above the ad) by commenting out the entire wildcard property below. I guess it's the display:inline that does it, but I have seen no other adverse effects on my site, so this works for now.

themes/garland/style.css, line 284:
283 /*
284 #header-region * {
285 display: inline;
286 line-height: 1.5em;
287 margin-top: 0;
288 margin-bottom: 0;
289 }
290 */

Be sure to clear the cache to update the stylesheet.

As for actually fixing it, I recommend that the designers review whether the wildcard (*) is really appropriate here. It affects script elements as well, which doesn't strike me as intended, so I suggest it be replaced with an explicit list of elements that *do* need this property.

Isn't there a more elegant solution possible? I find it very hard to believe that this won't break anything, maybe it just doesn't break anything for the setup you're using...

Marked issue 126274 as a duplicate of this issue.

Marked issue 130586 as duplicate of this issue.

For some reason I thought the core maintainers would investigate this before releasing Drupal 5. Since that hasn't happened, here is a patch to review.

A screen shot can be seen here for those interested. http://drupal.org/files/issues/Snapshot.jpg

The rule needs to be replaced with one that does the same thing, but does not affect script tags. Just removing it is not right.

What does the rule do, besides mysteriously causing script source to be displayed in the header region? According to the CSS specification, "inline" is the default value of the display property.

So far there have been no reports of side effects from this patch. None of the other core themes contain the "display: inline" property. I've been searching for documentation related to this problem. This article may be related: Properly Using CSS and JavaScript in XHTML Documents. I'm setting this back to "code needs review" because a serious review is needed to determine if the patch needs work.

Um, it obviously makes all block-level elements display as inline. This is necessary for each block to be as thin vertically as possible.

A serious review should address the following points:

1. Is specifying an inline value for the display property necessary when inline is already the default value? If so, why?
2. Why do none of the other core themes specify this property?
3. Why does specifying this property affect the display of HTML comments?

Do we really need to spell everything out? This is CSS 101.

Is specifying an inline value for the display property necessary when inline is already the default value? If so, why?

Because tags like div, blockquote, paragraph, headers, list item, etc default to 'display: block' instead of 'inline' in all browsers. See "Vertical formatting" in CSS 1 and the suggested default stylesheet for HTML4.

Why do none of the other core themes specify this property?

Because Garland is the only theme that has any styles for the header region.

Why does specifying this property affect the display of HTML comments?

Because browsers suck.

I found a solution that doesn't touch the drupal core code.

Create a .js file with the script information and call the script from within the block.

Block code should look like this:

 <script type="text/javascript" src="files/adsense.js"></script>
<script type="text/javascript"
  src="http://pagead2.googlesyndication.com/pagead/show_ads.js">
</script>

content of my adsense.js looks like this:

google_ad_client = "pub-0831875713013077";
google_ad_width = 728;
google_ad_height = 15;
google_ad_format = "728x15_0ads_al_s";
google_ad_channel = "";
google_color_border = "D8E8F2";
google_color_bg = "D8E8F2";
google_color_link = "4290C2";
google_color_text = "000000";
google_color_url = "008000";

Here's what it looks like:
http://www.yogavicki.com

Rock on, Drupal peeps.

I can't think of how to apply the solution to a script that must be dynamically generated by a module. By the way, I finally saw what the inline value does: it makes blocks that were designed to have multiple lines display on one line. Apparently this is a higher priority than being able to run scripts in the header region.

Okay, in the meantime I've learned some more about CSS and also about how to make patches.

Here are the possible values for display:

- block: is displayed inside a rectangular region, with a margin. (and usually starts on a new line, unless float and clear are changed)
- inline: is displayed as flowing text without linebreaks or margins.
- none: is not displayed at all.

Yes, each element has a default display value, and yes, blocks can be made to use less space by making them inline, but there are also elements that are by default invisible (like script tags).

By setting *, the declaration is affecting all elements, including those that would normally be invisible. In my personal and unimportant opinion, this wildcard is not so much elegance as carelessness, as the block elements (div and what else?) that should be changed could just be listed.

But since

Note: The above patch also works on HEAD as of now.

Beautiful solution! Thanks for figuring this out. This has been a good CSS course for me.

Except it's not RTBC, since it doesn't conform to code style and is documented badly.

For code style, the objections appear to be a lacking line of whitespace separating the block from the preceding one, a lacking space between the colon and the value, and the fact that the comment was placed on the line of the declaration rather than above the block. Fixed.

Comment has been reworded to a full sentence. However, "badly documented" implies that there is something wrong with the content rather than the phrasing of the comment; if the comment in this patch still runs afoul of standards, please suggest a different one.

Thank you for trying to guess what "coding style" and "badly documented" meant. Since the objections to the patch were not specific, I'm setting this back to "ready to be committed". It's outrageous that Garland has been allowed to be broken for almost the whole time Drupal 5 has been released.

I just noticed that Steven is the one who committed this buggy CSS in the first place. I would have thought he would want to do more than criticize other people's efforts to fix it.

Committed to 5.x.

Looks like it needs to go in for 6.x too.

I've read the comments but I'm not sure I understand the exact cause. Is this due to a browser bug or due to a bug in Drupal?

I've never seen anything like:

+#header-region script {
+  display: none;
+}

so I'm a bit puzzled. The PHPdoc does not really help me understand why this is necessary. I'd appreciate a small explanation as I've lost my overview on this one.

It overrides

#header-region * {
  display: inline;
  ...

I'm not sure whether to classify it as a "bug" in the browser. It might be, if a browser interpreting a directive in an unexpected way (different from other browsers) is a bug - but if you submit this to Bugzilla, it will probably end up marked "by design" or equivalent.

Unlike other browsers, Mozilla and Firefox allow CSS to dictate the layout far more than HTML (there is in fact one Bugzilla comment claiming that "Mozilla is not an HTML user agent, it is a CSS user agent that happens to have knowledge of some HTML semantics.") Which in this case means that it hides the content of <script> elements by default, but it does so through CSS (instead of hard-coding it), it can be changed by a CSS directive like "display: inline". The "*" wildcard then refers to all child elements, including script elements, and the result is this.

In effect, this is just Firefox taking the CSS directive too literally and applying it too broadly, without doing a "sanity check" on whether a designer would ever actually want this. To get rid of the problem, the stylesheet must tell Firefox to specifically hide the script element again.

(Unfortunately, I can't phrase this in the few sentences that would need to go into the PHPDoc comment.)

Maybe a somewhat extended comment could be distilled from the longer description :) Like 'Prevent Mozilla browsers to display the contents of the style element'.

Attaching a patch for head that has this comment:

/* Prevent the previous directive from showing the content of script elements in Mozilla browsers. */

Since it was already committed in 5.x and is a non-essential change, I think it will be enough to change the comment in head without modifying 5.x again, but that's just my 2c. All patches in this part of the file work in both branches, so if necessary the last 5.x patch can be rolled back and replaced by this one (a lot of trouble for a one-line comment, but hey).

May I RTBC this again? It was before, and it's been approved by everyone save the core committers already...

Well, these obscure CSS details need careful documentation or otherwise they could be removed maybe even in this release cycle by some cleanup effort or something, marked as being superfluous.

Committed to 6.x-dev.