Bug 33643 - Window size changes are not propagated down the render tree
Summary: Window size changes are not propagated down the render tree
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Blocker
Assignee: Nikolas Zimmermann
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-13 17:29 PST by Nikolas Zimmermann
Modified: 2010-01-13 19:06 PST (History)
7 users (show)

See Also:


Attachments
Initial patch (46.18 KB, patch)
2010-01-13 18:33 PST, Nikolas Zimmermann
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.