Bug 5768 - pages with frames that are all "fixed" get no layout and are blank (like www.farnell.nl)
Summary: pages with frames that are all "fixed" get no layout and are blank (like www....
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Critical
Assignee: Dave Hyatt
URL: http://www.farnell.nl/
Keywords:
: 4110 5591 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-11-18 00:16 PST by Henk
Modified: 2011-05-04 14:42 PDT (History)
3 users (show)

See Also:


Attachments
Proposed patch (4.50 KB, patch)
2005-11-19 15:26 PST, mitz
no flags Details | Formatted Diff | Diff
Equivalent patch without all the indenting (911 bytes, patch)
2005-11-22 22:02 PST, mitz
darin: review+
Details | Formatted Diff | Diff
unsuccesful attempt at automatic testing (1.36 KB, application/octet-stream)
2005-11-30 12:19 PST, mitz
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Henk 2005-11-18 00:16:21 PST
Goto http://www. farnell.nl/ 
Click on one of the buttons (NIEUWE PRODUCTEN, PUBLICATIES,ED WORLD,SUPPORT,ZOEK) at the top of 
the page.
The corresponding page is loaded, but nothing is visible.
However the content will appear when you click once in the window content region.
Comment 1 mitz 2005-11-19 13:37:46 PST
I reduced it to the noresize attribute of the frame element. When it's present, even the simplest testcase 
of

<html>
<frameset>
    <frame src="frame1.html" noresize>
</frameset>
</html>

with frame1.html linking to frame2.html and vice versa results in frame content not appearing after the 
second switch (dragging in the empty frame behaves like a resize, after which the content is visible 
again). Without the noresize attribute, content does not disappear. When content is invisible, it's 
because the RenderFrame is at -500000. It is marked as needing layout but nothing triggers layout for 
it.
Comment 2 mitz 2005-11-19 15:26:50 PST
Created attachment 4736 [details]
Proposed patch

Don't skip layout even if all children are fixed.
Comment 3 Darin Adler 2005-11-22 19:55:15 PST
Comment on attachment 4736 [details]
Proposed patch

I was fooled by the formatting changes into thinking this was a bigger change
than it is. The substance of this change is simple, and clearly a good idea.

But can't we just move the end2: label up one line to get the same effect? I
don't see a lot of value in indenting all the code.
Comment 4 mitz 2005-11-22 22:02:44 PST
Created attachment 4778 [details]
Equivalent patch without all the indenting

(In reply to comment #3)

> But can't we just move the end2: label up one line to get the same effect? I
> don't see a lot of value in indenting all the code.

Just moving end2: up one line will cause RenderContainer::layout() to be called
even when there are no children at all. I don't know how significant this is.

This patch is equivalent to the previous patch.
Comment 5 mitz 2005-11-22 22:17:00 PST
Comment on attachment 4778 [details]
Equivalent patch without all the indenting

I'm leaving the r+ on the other patch, but if you prefer this version, please
remove it.
Comment 6 Darin Adler 2005-11-23 06:42:59 PST
Comment on attachment 4778 [details]
Equivalent patch without all the indenting

Excellent. Seems much clearer this way.
Comment 7 Eric Seidel (no email) 2005-11-29 19:38:18 PST
It would be good to have a ready-to-land test case for this.
Comment 8 mitz 2005-11-30 12:19:33 PST
Created attachment 4882 [details]
unsuccesful attempt at automatic testing

This test works (automatically) in Safari, but I couldn't get it to work in
DumpRenderTree. Perhaps relayout needs to be forced at some points, which I
don't know how to do. Posting it here in hope that it will inspire someone
else.
Comment 9 Darin Adler 2005-12-17 20:20:53 PST
I tried really hard to make that test work under DumpRenderTree, but I wasn't able to figure it out. There's 
something about how layout is done for repainting that seems to be required. I spent quite a while 
debugging both DumpRenderTree and Safari to see what's different between the two but it was ultimately 
fruitless.
Comment 10 mitz 2005-12-28 04:49:32 PST
*** Bug 5591 has been marked as a duplicate of this bug. ***
Comment 11 Gustaaf Groenendaal (MysteryQuest) 2006-03-08 10:32:20 PST
*** Bug 4110 has been marked as a duplicate of this bug. ***
Comment 12 Daniel Bates 2011-05-04 14:42:14 PDT
For completeness, this bug was fixed in changeset 11648 <http://trac.webkit.org/changeset/11648>.