You will see tgat in Safari, the line "Download all commentaries" is rendered on top of the table of downloadable links, but in Firefox, 1.5, it is rendered above it (as was desired).
Created attachment 10619 [details] Reduction Firefox keeps the blue div below the black box.
Safari's rendering is correct according to the CSS2.1 spec.
Someone should file a bug in Mozilla and point them at this reduction. See also the bug Eric Seidel filed with an <hr> example. That was the exact same bug in Mozilla but with a positive margin (so might be easier to understand).
See Opera 9's rendering as well, which also matches the spec.
This is a subtle bug and not the same as the other bug I mentioned. This is a bug in Safari and Opera.
Is this also causing the bug reported to Chromium? http://code.google.com/p/chromium/issues/detail?id=1144
Created attachment 101816 [details] Patch
Created attachment 101817 [details] Patch
Created attachment 107184 [details] Patch
If you see this message in your bug spam, could you review me? :)
Comment on attachment 107184 [details] Patch This isn't the right way to handle this. If/when things shift to cause new floats to potentially overhang, we should just call addOverhangingFloats again. We should not change the definition of what addOverhangingFloats does.
(In reply to comment #11) > (From update of attachment 107184 [details]) > This isn't the right way to handle this. If/when things shift to cause new floats to potentially overhang, we should just call addOverhangingFloats again. We should not change the definition of what addOverhangingFloats does. In this case the float never overhangs its parent, it ends up intruding into a sibling block that has collapsed into its parent. I've updated the fix to only worry about this exact case. This still allows blocks with cleanace that are children of the parent's sibling to collapse into the parent and not clear the float though. But I think that's a separate bug, such children should only collapse as far as the top of their parent and not into their parent's previous sibling. Am I right in thinking that?
Created attachment 118691 [details] Patch
Created attachment 143643 [details] Patch
Comment on attachment 143643 [details] Patch r=me. Nice catch.
Comment on attachment 143643 [details] Patch Clearing flags on attachment: 143643 Committed r118395: <http://trac.webkit.org/changeset/118395>
All reviewed patches have been landed. Closing bug.
This patch caused a regression: https://bugs.webkit.org/show_bug.cgi?id=87461
Robert & Dave: could either one of you respond to the regression this patch caused? It's pretty bad regression and I'm afraid we might need to revert the patch if we can't fix it in a timely manner or if we don't hear from you guys by the end of the day.
Re-opened since this is blocked by 87526
I won't get to it today but was planning to fix or roll out by Monday. I'm not in a position to roll out or fix today so feel free to take action today if required.
Note the regression reduction is: https://bug-87461-attachments.webkit.org/attachment.cgi?id=143986
Created attachment 144223 [details] Patch
Comment on attachment 144223 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144223&action=review > Source/WebCore/rendering/RenderBlock.cpp:2025 > + if (block->m_floatingObjects && block->lowestFloatLogicalBottom() > logicalTop) Shouldn't you use block->containsFloats() instead of m_floatingObjects here? (the code doesn't seem to draw a strong line between the 2 though) > Source/WebCore/rendering/RenderBlock.cpp:2026 > + addOverhangingFloats(block, false); Overflow: hidden seems covered inside addOverhangingFloats - and in your testing - but how about other cases where CSS 2.1 explicitly mention not to allow a float to overlap: "The border box of a table, a block-level replaced element, or an element in the normal flow that establishes a new block formatting context [...] must not overlap the margin box of any floats in the same block formatting context as the element itself". Are those cases also covered? > Source/WebCore/rendering/RenderBlock.cpp:2027 > + } The logic is very similar to what is done in RenderBlock::layoutBlock, maybe there could be a way for use to share more code between the 2 methods? (I haven't tried so I don't know how difficult / messy it would be)
(In reply to comment #24) > > Overflow: hidden seems covered inside addOverhangingFloats - and in your testing - but how about other cases where CSS 2.1 explicitly mention not to allow a float to overlap: > > "The border box of a table, a block-level replaced element, or an element in the normal flow that establishes a new block formatting context [...] must not overlap the margin box of any floats in the same block formatting context as the element itself". > > Are those cases also covered? > This part of the spec is covered by the tests in bugs 88434 and to a lesser extent 88171. I have a patch for 88171 and am working on 88434.
Created attachment 148596 [details] Patch
(In reply to comment #26) > Created an attachment (id=148596) [details] > Patch The version of this patch that broke Gmail and got rolled out didn't check whether the previous sibling was a float or positioned. In that situation the previous sibling will get avoided anyway and attempting to avoid floated children of floats causes incorrect layout. The four previous-sibling-* tests in the new patch test for this.
Comment on attachment 148596 [details] Patch OK.
Comment on attachment 148596 [details] Patch Clearing flags on attachment: 148596 Committed r120844: <http://trac.webkit.org/changeset/120844>