48138 - [FLOAT] {inc} Text not flowed around IMG when IMG changes size during inc reflow

Status: RESOLVED FIXED

(Core :: Layout, defect, P2)

Description

[larry]

This is using the M17 build with FullCircle under Win32 (Gecko/20000807).

Go to the specified URL and read about the top ten released
Dreamcast games.  Note that the text describing each game flows
_over_ the screenshot of the game, rather than around it.

My programmer's intuition says this has something to do with
a) each image is align=right, b) they don't specify the height
and width for the images in the HTML, and c) it's nested a couple
levels deep in tables.

I spent a little time trying to make a minimal case for this,
but Mozilla handled 'em fine.  So I'm wimping out and just
sending you the original URL.  Sorry :(

Comment 1

[Jeffrey Baker]

Attachment: Valid HTML 4.01 Trans testcase

Comment 2

[Jeffrey Baker]

Confirmed on Linux M18 2000-08-08-08.  I have attached a valid HTML 4.01
Transitional testcase which demonstrates the problem.  The <IMG align="right">
is being clobbered by the text.  The testcase is fragile: if you remove a
certain amount of text, it flows properly.  Same if you remove the <P> from in
front of the IMG.

Changing platform/opsys, adding screenshot, etc.

Comment 3

[Jeffrey Baker]

Attachment: Screenshot on Linux

Comment 4

[Jeffrey Baker]

The other thing of note is that the problem exists when the table has a fixed
width of 400 pixels, but if you remove the fixed width and make your window 400
pixels wide, the problem does not exist.

Comment 5

[Nisheeth Ranjan]

Re-assigning 6 bugs from Clayton's list to Harish for further triage.

Comment 6

[harishd]

Triaging Clayton's list:
-------------------------

Another layout bug. To waterson - the layout guru ;)

Comment 7

[Chris Waterson]

I think that the reporter's original intuition is correct. It looks like we're
not generating a reflow when the image's w&h arrive. Maybe has to do with the
image being in a float. This would be good to fix: I imagine that this will lead
to incorrect layout of many pages. Marking [nsbeta3+], but lowest priority. (The
"workaround" is to reload the page, and the image's w&h will come from the cache.)

Comment 8

[Hixie (not reading bugmail)]

reloading the page won't fix it. Changing the font size will, because that
triggers a complete reflow. The problem seems to be that incremental reflows 
are not checking to see if the IMG frame has changed its intrinsic dimensions?
Something like that...

Comment 9

[Chris Waterson]

Attachment: different test case illustrating problem

Comment 10

[Chris Waterson]

next train...

Comment 11

[Jesse Ruderman]

Might be related to bug 56201.

Comment 12

[Chris Waterson]

Ok, I think I know what the problem is. In nsBlockFrame::ReflowDirtyLines(), 
we're passing in a deltaY to PropogateReflowDamage(). The problem is that we're 
accumulating too much deltaY because we've got continuing frames up above. 
Here's why.

- Since we're inside a table cell, we're asked to compute a maximum width.

- Since we're asked to compute a maximum width, we do two reflows in
  nsBlockFrame::ReflowLine(): one at unconstrained width & height to
  determine the max width, and another to actually reflow at the constrained
  width. (There's a wet sloppy XXX kiss from troy that talks about how this
  screws floaters.)

- The unconstrained reflow clobbers all the continuing frames and their
  lineboxes, which we then recreate (partially) in the second reflow.

- This leaves us stuck with oldHeight's of zero for each linebox that we flow
  until we reach the block frame. Since newHeight will be, well, not zero,
  we end up skewing the deltaY by more and more for each continuing frame,
  when in fact we really wanted deltaY to be zero!

- By the time we hit PropogateReflowDamage(), deltaY makes it seem like the
  floater that changed was way above us.

I'm now trying to figure out how to fix this...

Comment 13

[Hixie (not reading bugmail)]

Chris: Dude! What a kick-ass testcase! You sure you don't want to work in QA?

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

See also bug 61962 in the same area of code.  (I'd love to come up with a good
data structure to represent vertical space occupied by floats, but I haven't
thought of one.)  While thinking about that bug I couldn't understand why the
code did what it did in some other places...

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This is fixed by the changes on bug 86947.  (Well, it will be in the next patch
- I overoptimized away the case that causes this to work, and it will be fixed
in the next version of the patch I attach.)  Note that I had to rework what
deltaY means since I'm using it differently, and I think the changes I made
fixed this problem.  

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: slightly more complex testcase (split lower paragraph in two)

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fixed by checkin of bug 86947 at 2001-10-24 18:08 PDT.