The blockquote messes up ol lists

I see the problem: lines 162-164 of bartik/css/style.css are

blockquote :first-child {
  display: inline;
}

This CSS rule does not apply just to the first child of the <blockquote> tag. It applies to any tag that is the first of its kind within a parent element, as long as it is inside the blockquote! In particular, the first <li> element in the <ol> is affected. Try a nested list: even worse!

The safest thing is to remove the offending lines. A reasonable alternative is to replace them with this version:

blockquote > p:first-child {
  display: inline;
}

This has the desired effect if the block quote starts with a <p> element. I cannot think of any problems if the first paragraph comes later. I have no idea what the effect will be in IE6.

I am still new at this. I hope it is appropriate for me to change the status to "needs work."

The more I test it, the more I like using blockquote > p:first-child. This correctly targets what we want, the first child element of the blockquote. It gets ugly if we use blockquote > :first-child since the first child might be a list. Are there any elements other than <p> where we want it to apply? If not, I think this is the right fix.

I have no idea what the effect will be in IE6.

Hmm. We need some screenshots of before and after. And cross-browser testing (with screenshots). I'm glad its working so far, but in order to make sure we don't introduce a new problem in Drupal 7.1, we must test it more thoroughly before patching Drupal core.

@jensimmons,
I have attached before and after screenshots using Chrome 8, Firefox 3.6, and Safari 5 on Mac OS. When I said that nested lists looked ugly before the patch, I was not kidding!

Just to be clear: before means Drupal 7.0, and after means after applying my patch in comment #2.

More screen shots, this time from Windows 7. My opinion is that the verdict is unanimous as far as modern browsers are concerned. I am not an expert on older browsers, but I think that any browser that recognizes the :first-child selector should also implement the > p:first-child selector.

@jensimmons,
What does it take to commit a bug fix?

I realize that I am new here, and thus have not yet established any credibility. Nonetheless, if there was ever a patch that did not need thorough testing, this is it. I am the first to admit when I am hacking and not sure if I have the right patch. Not here. I know enough CSS to recognize this as a bug.

The patch adds three characters ... one of which is a space. Does it get any simpler?

Look at the CSS selector blockquote :first-child and look at what it does in the screen shots. If necessary, check the docs on w3.org or w3schools. It targets way more than intended. A big improvement is to use the selector blockquote > :first-child. This targets only the first child of the blockquote.

There is still a problem if the first child of the blockquote is something like <ol> or <ul>. (See the screen shot attached to this comment.) That is why I changed the selector to blockquote p:first-child. Now, maybe this is too restrictive. Maybe you want some other elements to be rendered in-line if they are at the start of a blockquote. If so, that is a feature request, not a bug. For example, we might add the selector blockquote > div:first-child. Now, that would require testing!

I exaggerated when I said I had "no idea" how this would look on IE6. I think that IE6 does not support :first-child, and perhaps it does not support > either. If so, then it should ignore any version of this line.

Tested this in IE6 and 7, attached screenshots.

In IE6 the patch makes no difference because IE6 doesn't support :before/:after and :first-child, so there no need to print the first blockquote child element inline.

In IE7 it makes a difference because IE7 supports :first-child but not :before/:after, so the quotes are not printed but ol's and ul's are messed by the :first-child selector.

Also tested this in every other browser (FF 3.6, Chrome, IE8, Opera) and benjifisher's logic and screenshots are correct.

I think that the patch from #2 is our best way to go here.

Forgot the screenshots.

Also tested and confirmed.

Great, thanks for all the testing of this fix, folks!

Committed to HEAD.