120107 - [FLOAT]Inset graphic on right renders incorrectly [FIXED_ON_TRUNK]

Status: VERIFIED FIXED

(Core :: Layout, defect, P3)

Description

[Jason Pfeil]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 Galeon/1.1 (X11; Linux i686; U;) Gecko/20011226
BuildID:    2001122617

The inset graphic on the right side of the screen renders incorrectly.  The
graphic partially obscurs and interferes with the rendering of the right-hand
sidebar.

Reproducible: Always
Steps to Reproduce:
1.Visit the URL


Actual Results:  Same as in the description above

Expected Results:  I'm not sure as I can't access another modern browser at this
time.

Comment 1

[Christopher Hoess (gone)]

Confirmed.  This appeared when I first loaded the page and reloaded, but both
going back and then to the page again, and viewing in DOM Inspector, resulted in
correct rendering.

Comment 2

[Jason Pfeil]

Hmmm...how do you do what you described?  I popped open the DOM inspector and
worked my way down the tree to that IMG tag and it still was mis-rendered. 
Then, I went to another page and back to it and it still mis-rendered.  :)

Thanks!

--Jason

Comment 3

[Kevin McCluskey (gone)]

Looks like a floater issue. Reassigning to Alex.

Comment 4

[Alexandru Savulov]

is displayed correctly if you hit corectly if back/fwd or fwd/back with
2002013103(w32) and 2002011408(linux).

I will attach a testcase

Comment 5

[Alexandru Savulov]

Attachment: needed image

Comment 6

[Alexandru Savulov]

Attachment: testcase

i tested the URL with 2002013103(win32) and 2002011408(linux) and if you hit
back/fwd or fwd/back the page displays correctly

Comment 7

[Jason Pfeil]

Yes, it does with my mozilla now.  Any idea why it doesn't rener properly initially?

Thanks!

--Jason

Comment 8

[Boris Zbarsky [:bzbarsky]]

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

Comment 9

[Kevin McCluskey (gone)]

Moving to Mozilla1.1. Engineers are overloaded with higher priority bugs.

Comment 10

[R.K.Aa.]

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

Comment 11

[R.K.Aa.]

duplicate of bug 97057?

Comment 12

[Chris Waterson]

Attachment: more minimal test case

Removed a few unnecessary elements.

Comment 13

[Chris Waterson]

Attachment: incremental reflow of image

This trace is the incremental reflow of the image frame, which makes me think
this is _not_ a floater bug, but a table incremental reflow bug. Notice that
the table cell's width is correctly increased, but the row and the rowgroup do
not expand horizontally.

Comment 14

[Chris Waterson]

-> tables.

Comment 15

[Chris Waterson]

Hmmkay. My bad: I dug a little deeper and it looks like we're not updating the
MES when we incrementally reflow that floater. Taking back...

Comment 16

[Chris Waterson]

Okay, here's what's happening AFAICT.

1. The inline frame is being asked to compute its MES by the outer table. So,
   it does an unconstrained reflow of its lines.

2. The floated table frame is flowed by its placeholder frame at an
   unconstrained width. It correctly detects that it's going to need to do a
   second-pass reflow when the incremental reflow is processed by the cell.

3. However, on the way _out_ of the reflow, it never bothers doing the second-
   pass reflow, assuming that since it's gotten an unconstrained reflow, it'll
   get flowed again. (See nsTableFrame.cpp:2141.)

4. Therefore, the floated element is returned the original table cell's width,
   which it assumes is the MES, and stores it away.

5. We return back out to the inline frame, which now reflows the line again,
   but this time _without_ requesting that its children compute MES.

6. We return the narrower MES out to the enclosing table, and things look
   wrong.

So, it seems to me that the table frame shouldn't be deferring that second-pass
reflow in (3), above. Or, we should be requesting that children compute the MES
*again* during the second-pass line reflow in (5).

Comment 17

[Chris Waterson]

Attachment: proposed fix

So, one way to do this is to allow the second-pass reflow to happen when the
table detects that it needs rebalancing, and _either_ it's not an unconstrained
reflow, or the MES is being requested.

Comment 18

[karnaze (gone)]

Attachment: patch to have fewer reflows on nested tables

This patch does not fix bug 141021 (but I'll be attaching another patch to that
bug shortly).

Comment 19

[karnaze (gone)]

Attachment: revised patch to cause no extra reflows

Comment 20

[Chris Waterson]

karnaze, it looks like you've got this covered. ;-)

Comment 21

[karnaze (gone)]

Attachment: revised patch (a little more efficient)

Comment 22

[Bernd]

Comment on attachment 82656 [details] [diff] [review]
revised patch (a little more efficient)

r=bernd

Comment 23

[Chris Waterson]

Comment on attachment 82656 [details] [diff] [review]
revised patch (a little more efficient)

sr=waterson

Comment 24

[karnaze (gone)]

FIXED_ON_TRUNK

Comment 25

[Aaron Kaluszka]

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

Comment 26

[Doron Rosenberg (IBM)]

According to Bernd, this patch might have fixed 
http://www.nexgenmedia.net/evang/ftd.de/, which is a simplified testcase of a
german topsite/AOL Partner (bugscape 16499).  If so, this should go into the
branch for MachV.

Comment 27

[Christopher Hoess (gone)]

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

Comment 28

[Jaime Rodriguez, Jr.]

amar: can you pls verify this as fixed, and check around for any possible
regressions. thanks!

Comment 29

[Amarendra Hanumanula]

Jaime: This bug is fixed on the trunk. Its not checked into branch. This bug 
still exists on branch build: 2002-08-05-12-1.0.
Verified fixed on trunk build: 2002-08-05-08-trunk.
No regressions found on trunk.

Comment 30

[Jaime Rodriguez, Jr.]

Marking as Verified for the trunk per Comment #29 From Amarendra Hanumanula.

Comment 31

[Jaime Rodriguez, Jr.]

mass changing adt1.0.1 to adt1.0.2 nominations.

Comment 32

[Jaime Rodriguez, Jr.]

mozilla1.0.1 has passed. carrying over nomination to mozilla1.0.2 (and adding
topembed+), as this fixes a topsite in germany.

Comment 33

[Daniel Veditz [:dveditz]]

This bug does not meet the Blackbird criteria, adt-