Bug 6795 – Slow image load causes render problem.

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Dennis Rowe]

I have tried to reduce this problem down as much as possible.  But, as you can see the test case still has a bunch of junk still in it, and when I tried to remove the rest of these tags, it caused the problem to dissappear (which was bad).  My test case uses a specialized image which is really a php script that creates a randomly colored image that changes on every load and also has a three second sleep.  This was done to cause the image to load slowly and also to stop caching of the image.  When the page is loaded the text is drawn on the page and when finally drawn to the page, the text is pushed down, but the html in the div holding "Problem Here" text is pushed down, but also another box appears at the top of the page.  The boxes are connected, because if you click on the button at the bottom of the page, it will check the button on the top instance.

These seems to affect both 417.x and 420+.

[Dennis Rowe]

Created attachment 5951 [details]
Bug Example

Here is the example of the bug.

[Joost de Valk (AlthA)]

Confirmed, this needs more reduction.

Created attachment 6542 [details]
Reduced testcase

Created attachment 6545 [details]
Proposed patch

The test is pixels-only since the render tree was correct.

[Darin Adler]

Comment on attachment 6545 [details]
Proposed patch

Looks right to me, but this is one Hyatt should review.

[Dave Hyatt]

Comment on attachment 6545 [details]
Proposed patch

I will need to think about this.  My gut says this is overkill, but my gut has been wrong before. :)

[Dave Hyatt]

The reason I'm nervous here is we had some horribly pathological behavior (as in pages with nested divs and complex float/margin combinations causing Safari to beachball) that the yPosEstimate stuff helped to prevent, and doing too much of the heavyweight marking can send you into a death spiral of relayout.

[Dave Hyatt]

I do think this is probably on the right track and maybe just needs a little refinement.  One problem I see is that won't this get triggered on the initial layout of elements, since oldChildY will be 0?

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

As Hyatt suggested on IRC, I ran the layout tests and logged the ones where the additional marking was done:

css1/box_properties/acid_test
css1/box_properties/float
css2.1/t09-c5526c-display-00-e
css2.1/t0905-c414-flt-fit-01-d-g
css2.1/t0905-c5525-fltmult-00-d-g
fast/block/basic/011
fast/block/float/016
fast/block/float/024
fast/block/float/031
fast/block/margin-collapse/045
fast/block/margin-collapse/063
fast/css-generated-content/012

I'm not sure I see the problem with initial layout. Wouldn't all descendants be marked for layout anyway?

[Dave Hyatt]

Comment on attachment 6545 [details]
Proposed patch

r=me, make sure we have some good tests for this.