Bug 33643

Summary: Window size changes are not propagated down the render tree
Product: WebKit Reporter: Nikolas Zimmermann <zimmermann>
Component: SVGAssignee: Nikolas Zimmermann <zimmermann>
Status: RESOLVED FIXED    
Severity: Blocker CC: darin, dglazkov, eric, hyatt, jamesr, krit, webkit.review.bot
Priority: P1    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Initial patch oliver: review+

Description Nikolas Zimmermann 2010-01-13 17:29:05 PST
Amazing that we have a bug like this, anyhow, we don't react on window size changes at the moment -- if you use any percentual values in a svg document, the RenderSVGRoot object does not propagate these children correctly.

For example:
<svg xmlns="..."><rect width="50%" height="50%"/></svg>

Resize Safari -> no reaction. As layout tests use a fixed window size, we can only create a testcase which puts svg content in a <div> within a compound document and resize that box - it suffers from the same bug.
Comment 1 Nikolas Zimmermann 2010-01-13 18:33:15 PST
Created attachment 46533 [details]
Initial patch

<use> content still won't react to size changes, due its design - will be fixed in a follow up patch.
It's already done, just submitting in chunks, to avoid a monster patch :-)
Comment 2 Eric Seidel (no email) 2010-01-13 18:51:52 PST
Comment on attachment 46533 [details]
Initial patch

I seem to remember Darin saying that marking things for layout during layout was bad?

258         if (needsLayout)
 259             child->setNeedsLayout(true, false);
Comment 3 WebKit Review Bot 2010-01-13 18:58:31 PST
Attachment 46533 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/185957
Comment 4 Nikolas Zimmermann 2010-01-13 19:00:24 PST
(In reply to comment #2)
> (From update of attachment 46533 [details])
> I seem to remember Darin saying that marking things for layout during layout
> was bad?

Huh? Please reread the patch, it's the regular flow of a renderer containing children to layout them - this patch mimics RenderBlock behaviour for SVG (see layoutBlockChildren + relayoutChildren).

Furthermore it's just refactoring existing code while cleaning up some things, that lead to unneeded relayout and/or missing layouts for relative-sized kids.

If you trace the code starting from RenderSVGRoot, you'll see that window size changes (induced by WebKit -> FrameView::layout(), only cause a setChildNeedsLayout(true) call on the RenderSVGRoot object, at least for standalone svg documents) - so comparing to RenderBlock behaviour we have to take care of relayouting relative sized kids ourselves.

Hope that clarifies it a bit.
Comment 5 Oliver Hunt 2010-01-13 19:01:54 PST
Comment on attachment 46533 [details]
Initial patch

r=me
Comment 6 Nikolas Zimmermann 2010-01-13 19:06:28 PST
Landed in r53229.