Look at the submitted page, it contains a table (containing an image) aligned to the left, and to the right a text paragraph with a negative margin-top (in this case, of 7px). The first line of the text paragraph overlaps with the table, making the text unreadable (I'm using the default 96 dpi setting). This HTML code works ok with both Internet Explorer and Netscape Communicator, and according to w3c.org, negative margins are allowed. You might also want to take a look at the pre- and reviews at http://www.topofgames.de, this problem occurs in all articles.

Right now the testcase in the URL field is not available (the document is empty). However, negative margins are allowed so that authors may suggest overlap. It's not a bug if they cause it, it's a feature. There could be something wrong in this case, but since I can't see the testcase and I don't see the problem on the site mentioned, I can't tell.

I'm sorry, it seems the webserver was down yesterday. It should work now again.

Taking over 1/3 of Pierre's NEW bugs to help reduce his doomage factor

Reassigned back to me these bugs that shouldn't have left my list.

It looks like there may be a layout bug here. The margin is not merely moving up by 7 pixels as the style rules says it should, it is also moving left by the width of the table. Buster says this looks like a floater layout bug so I'll forward to him and then attach a simple testcase...

floaters in M15

moving all buster m15 bugs to m16.

post-beta issue

redistributing bugs across future milestones, sorry for the spam

This bug has been marked "future" because the original netscape engineer working on this is over-burdened. If you feel this is an error, that you or another known resource will be working on this bug,or if it blocks your work in some way -- please attach your concern to the bug for reconsideration.

*** Bug 49067 has been marked as a duplicate of this bug. ***

We need to examine this to work out exactly what is causing it and whether it is wrong or not.

Yeah, this is a valid bug (simplified attachement coming up). Line boxes should never overlap floats that are in their formatting context; in this case we are seeing an element moved up using negative margins so that it's line boxes begin higher than a float earlier in the flow, but we are not making the first line box wrap around the float.

I think this has the same cause as bug 41412 -- see my 2001-09-17 13:56 comment on bug 86947.

Build reassigning Buster's bugs to Marc.

any progress with this bug ?

Confirmed using FizzillaCFM/2002070913. Setting All/All.

->Layout: Floats

Comment on attachment 22328 [details] Test case - the text should not overlap the blue box This testcase doesn't show the bug anymore, but the original one does.

*** Bug 198800 has been marked as a duplicate of this bug. ***

*** Bug 117400 has been marked as a duplicate of this bug. ***

*** Bug 222739 has been marked as a duplicate of this bug. ***

*** Bug 300193 has been marked as a duplicate of this bug. ***

replacing http://stud3.tuwien.ac.at/~e9825754/mozilla/margin_top_overlap.html as it gives 404 Using URL from dupe Bug 300193 Text overlaps float http://en.wikipedia.org/wiki/Hurricane_Dennis Some en.wikipedia pages are showing this bug on Seamonkey, but not on Firefox2 Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1) Gecko/20061104 SeaMonkey/1.1b http://en.wikipedia.org/wiki/WP:RFAR http://en.wikipedia.org/wiki/World_Bank_Group http://en.wikipedia.org/wiki/International_Monetary_Fund http://en.wikipedia.org/w/index.php?title=Oahu&oldid=58021284 http://en.wikipedia.org/wiki/Orangequat http://en.wikipedia.org/wiki/Image:Layout_engine_usage_share.svg

I just checked in a bunch of reftests for this.

Given an easier-to-work-with space manager (or a replacement for it), this would be pretty simple to fix. We'd just need to do the following: * store a line available space height on the stack (maybe in nsBlockReflowState), and initialize it to zero in nsBlockFrame::ReflowInlineFrames inside the loop over bands (but outside the loop over no-pull redos). * at the end of nsBlockFrame::DoReflowInlineFrames, check that the height the line got doesn't run into another band that's narrower than the band at its top. (Note that we have to check all bands, not just the bottom one.) If so, return LINE_REFLOW_REDO_NO_PULL and initialize the line available space height to the height of the line (after asserting that it's currently zero, maybe, although I'm not sure how this interacts with the other uses of NO_PULL). * at the beginning of nsBlockFrame::DoReflowInlineFrames, initialize the available space rect based on the line available space height. We'd need to check the interaction with the other causes of NO_PULL redoes, though. The idea is that the line available space height would be zero for the first try of a line at a position and potentially increase (once) to a larger amount, but would never decrease until we move to the next band.

Note that it appears that automated tests are in for this bug: layout/reftests/bugs/25888-1l-notref.html layout/reftests/bugs/25888-1l-ref.html layout/reftests/bugs/25888-1l.html layout/reftests/bugs/25888-1r-notref.html layout/reftests/bugs/25888-1r-ref.html layout/reftests/bugs/25888-1r.html layout/reftests/bugs/25888-2l-ref.html layout/reftests/bugs/25888-2l.html layout/reftests/bugs/25888-2r-ref.html layout/reftests/bugs/25888-2r.html layout/reftests/bugs/25888-3l-ref.html layout/reftests/bugs/25888-3l.html layout/reftests/bugs/25888-3r-ref.html layout/reftests/bugs/25888-3r.html Right now, we have no tag to state whether a test is automated. The in-testsuite flag does not speak to this.

I can't tell if any of the testcases cover the following situation, so I thought I'd throw it out: <div style="width: 200px; font-size: 0"> <div style="float:left; height: 10px; width: 50px"></div> <div style="float:left; clear: left; height: 10px; width: 100px"></div> <span style="display: inline-block; width: 10px; height:5px;"></span> <span style="display: inline-block; width: 100px; height:15px;"></span> </div> Here, the second inline-block only fits below both floats; the issue here, though, is making sure that the first span is next to the first float, not the second.

Another testcase can be found at http://klebom.se/stellan/css/textboxclearing/

See description at end of attachment. Illustrates incorrect wrapping of lists and paragraphs around floated blocks. Shows different possibilities when you hover on different parts, as per instructions.

I don't think this check should be needed anymore. But if it is, it needs to be extended.

Full reftests pass with patches 1-3.

Full reftests pass with patches 1-4, but I'm still not happy with this patch (see comments within).

A patch 5 is still needed to fix list item markers/bullets.

So (with the updated patches in my patch queue at http://hg.mozilla.org/users/dbaron_mozilla.com/patches/ ) I'm seeing a hang loading http://www.flightaware.com/ and I wrote what I think is in an equivalent testcase at 25888-2.html in the crashtests in the patches. This stack explains why it's a problem: it shows where my assumptions about mAvailSpaceRect not being updated during reflow are being violated. I think the way to move forward here (which would also fix the issues that I have comments about regarding when mAvailSpaceRect is valid) is to pull mAvailSpaceRect out of nsBlockReflowState. Instead, we should hold one avail space rect in nsBlockFrame::ReflowInlineFrames, one in nsBlockReflowState::FlowAndPlaceFloat, and one (or more) for bullet reflow (which might be a little more complicated, but could also be a little less efficient). This would likely solve the "when is mAvailSpaceRect" valid, and also fix this hang (infinite loop in ReflowInlineFrames), which results from the things I'm trying to do with mAvailSpaceRect in PlaceLine/DoReflowInlineFrames/ReflowInlineFrames invalid.

I've made a good bit of progress on the above in my patch queue. However, I explicitly stopped dealing with floats inside the line, which means I'm no longer fixing the case in bug 345369. I need to figure out how to deal with that.

So I locally reverted the change that I'd made that I expected made this not fix bug 345369 anymore. Reverting it actually doesn't fix that bug... I haven't investigated that problem yet. But it does reintroduce an infinite loop (broken by the spins == 1000 test) on layout/base/crashtests/266360-1.html . And that points to a bigger problem, which is that our use of aAvailableWidth in nsBlockReflowState::AddFloat presumes this bug. We actually do need to place floats there if they push below part of the line, but not all of it, since when we redo lines for "more floats", some of those additional floats could actually be in the line, and we don't want to skip placing them in the redo pass. For example, given: <!-- narrow float above that intrudes only a few pixels into div below --> <div> <div id="B" style="float:left;clear:left;width:99%"></div> hello </div> we should determine on the first pass that hello intersects the "B" div and that we need to reflow again, but on the second pass we still need to place the "B" div. And if "B" is after "hello", it's even messier, since we then need to push "hello" on the second pass, which in turn pushes the float, which means we need a third pass to actually place "hello", but for that third pass we need to remember that we have to push the float that we pushed on the second pass below the line even though it looks like there was room for it. So there's actually some messy interaction with bug 50630 here that I hadn't observed before.

Comment on attachment 355640 [details] [diff] [review] pre-patch 1: remove aState.IsImpactedByFloat() check in PrepareResizeReflow Removing this check should make the rest of this bug a good bit easier, and I think it's valid. I may as well get this part out of my tree (and land it separately from the rest to allow separating regressions).

Here's a series of preparatory patches for this bug that I think I'd rather get out of my tree sooner rather than later. This is in the format of the output of "hg out -p", but it's really a rather long sequence of patches that I'd rather keep separate (but I don't see a strong need to upload 8 separate attachments to bugzilla). The point of these patches is described primarily in comment 42. We already have a bit of existing confusion about when mAvailSpaceRect and mBandHasFloats are valid. Since the patches for this bug will make their meaning more complicated (i.e., it relates to a good bit more current state than just a single vertical coordinate), this series of patches just moves mAvailSpaceRect out of nsBlockReflowState. There's one patch to start things off, then one patch for each of the areas of code in which we track available space, and then one patch to remove the old bits of nsBlockReflowState. Most of the patches are relatively straightforward, but the bullet one is not. It also rewrites the hacky fix to bug 427370 in a better way that simultaneously fixes bug 428810. The new fix involves the ability to compute available space using a saved space manager state, which is what we want in this case. (I think this ability will also turn out useful for other patches in this bug, although I'm not entirely sure; see comment 44.)

Landed pre-patch 1: http://hg.mozilla.org/mozilla-central/rev/98d7db017563

+ aResult->mTop = kidPosition.mTop + offset; + aResult->mBaseline = kidPosition.mBaseline + offset; + aResult->mBottom = kidPosition.mBottom + offset; Worth declaring an operator+ on LinePosition? Or a method "LinePosition MoveBy(nscoord)"? There are two or three places you could use it. + // Note: mAvailSpaceRect.x is relative to the content box and never You mean floatAvailSpace.x? nsBlockReflowState::ComputeBlockAvailSpace(nsIFrame* aFrame, const nsStyleDisplay* aDisplay, + PRBool aBandHasFloats, + const nsRect& aFloatAvailableSpace, PRBool aBlockAvoidsFloats, nsRect& aResult) As soon as we have more than one boolean parameter I reckon we should define a flags word. It's a little more efficient and much easier to read. Do you think it might be worth defining a struct that contains an nsRect and a boolean to indicate whether there are any floats in the band, that GeGetFloatAvailableSpace and friends can return directly, and that we can pass around references to?

(In reply to comment #48) > Do you think it might be worth defining a struct that contains an nsRect and a > boolean to indicate whether there are any floats in the band, that > GeGetFloatAvailableSpace and friends can return directly, and that we can pass > around references to? Yeah... I'd sort of been thinking that for a while. So I went ahead and did it, but as a patch that applies on top of the other patches in the series. (And thus I ignored the comment immediately before the one quoted, since that function now takes the new struct rather than having an additional boolean.)

This fixes the issues in comment 48 (except the ComputeBlockAvailSpace one, since that's no longer needed due to the change addressing the last comment).

A few things that I'd changed in the previous patch can still be just nsRect, actually.

Comment on attachment 371363 [details] [diff] [review] series of preparatory patches implementing comment 42 + LinePosition operator+(nscoord aOffset) { Make it const

preparatory patch series landed: http://hg.mozilla.org/mozilla-central/rev/50ba649cfea2 http://hg.mozilla.org/mozilla-central/rev/4f86f4c54224 http://hg.mozilla.org/mozilla-central/rev/df0130484852 http://hg.mozilla.org/mozilla-central/rev/698d52722aeb http://hg.mozilla.org/mozilla-central/rev/800e279fe24d http://hg.mozilla.org/mozilla-central/rev/f8ac1e4ffe9c http://hg.mozilla.org/mozilla-central/rev/8fdfeef7e076 http://hg.mozilla.org/mozilla-central/rev/f82ad9a10b7d http://hg.mozilla.org/mozilla-central/rev/e5b1ab035426

(In reply to comment #43) > I've made a good bit of progress on the above in my patch queue. However, I > explicitly stopped dealing with floats inside the line, which means I'm no > longer fixing the case in bug 345369. I need to figure out how to deal with > that. (In reply to comment #44) > So I locally reverted the change that I'd made that I expected made this not > fix bug 345369 anymore. Reverting it actually doesn't fix that bug... I > haven't investigated that problem yet. I've been thinking that maybe I could un-revert that change, but then make certain types of line-redo get the available width for the call to AddFloat from a band rather than a position. Or maybe it's more that I need to rethink the interaction between the different types of line-redo operations, e.g., by preserving the REDO_NO_PULL state (forceBreakInContent, etc.) when we get a REDO_MORE_FLOATS.

This leaves a bunch of issues I was hoping to fix still unfixed, but does fix the most common cases. I'd like to get this in soon and then come back to the other parts later. (It leaves unfixed a bunch of issues dealing with a line wrapping around floats that are in the line itself. Right now nsLineLayout::UpdateBand and its caller only handle those cases when such floats intersect the top of the line. Getting all these issues really right would probably be easiest if we deferred float placement until we hit a break opportunity (unless we're already at one) and if we did vertical alignment and line height calculations as we reflowed the line rather than in a separate pass at the end.)

Comment on attachment 377450 [details] [diff] [review] patch 3: fix this bug for inlines wrapping around floats This seems OK.

I had another (more recent) preparatory patch lying around: I don't see any need to have similar methods InitFloat and AddFloat.

http://hg.mozilla.org/mozilla-central/rev/5e76aa18541f http://hg.mozilla.org/mozilla-central/rev/efc6ec4a3c35 http://hg.mozilla.org/mozilla-central/rev/be75e40365ce

Bug 179596 comment 26 has some thoughts relevant to fixing this for horizontal positioning of bullets (intended to be patch 5 in the series above, but which I've never started on).

I landed the tests from patch 4 in http://hg.mozilla.org/mozilla-central/rev/ec3bca6461a5

I'm going to move the work of patch 4 to bug 538194 and patch 5 to bug 416732. This bugs summary has always covered just what was already fixed by patches 1 through 3. Thus I'm going to mark it as fixed, which it really has been for years. (In milestone 1.9.2a1.)