[Webkit-unassigned] [Bug 79490] Percent width/height SVG not always scaled on window resize

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=79490





--- Comment #4 from Florin Malita <fmalita at google.com>  2012-02-27 09:40:01 PST ---
(In reply to comment #3)
> > Source/WebCore/rendering/RenderBlockLineLayout.cpp:1453
> > +    layoutPercentHeightDescendants();
> 
> Doesn't this cause changes for inline children of RenderBlocks in general? Are we doing unnecessary work for HTML? Is HTML maybe affected as well by this bug (inline percent height elements not reacting to size changes).

Good point. Based on my initial tracing, it seemed that this is needed to get percent height/width inline elements working. But thanks to your question I took another dive and realized that HTML inline percent elements work just fine (and without using the global percent-height descendant map) thanks to this little nugget I missed in RenderBlock::layoutInlineChildren():

  if (relayoutChildren || o->style()->width().isPercent() || o->style()->height().isPercent())
    o->setChildNeedsLayout(true, false);

We can refactor this to take advantage of your RenderBox::hasRelativeLogical{Height,Width} suggestion and then the whole percentHeightDescendantMap dance is no longer needed. It changes the HTML semantics slightly - testing max{Height,Width} & min{Height,Width} for percent units also - but I think that should be the correct behavior.


> Hm, this feels hackish. How about changing RenderReplaced::computePreferredLogicalWidths() to avoid asking the style() whether its dimensions are relative. Changing
> if (style()->width().isPercent() || style()->height().isPercent() || ....
>     m_minPrefLogWidth = 0;
> ..
> 
> to
> 
> if (hasRelativeLogicalWidth(style()) || hasRelativeLogicalHeight(style())))
>     m_minPrefLogWidth = 0;
> 
> and move style()->logicalWidth().isPercent() etc. checks into RenderBox::hasRelativeLogicalWidth/Height(RenderStyle*), mark it virtual and override it in RenderSVGRoot, to respect SVG width/height properties. It needs to be in RenderBox, so styleDidChange can use it as well.

Excellent suggestion. Now that I figured how to avoid messing with the percent height descendant map, this actually simplifies the patch quite a bit - thanks!


> > LayoutTests/svg/custom/svg-percent-scale-expected.html:16
> > +    window.resizeTo(400, 300);
> 
> Oh my! I wasn't aware that this exists! In the past I always simulated that by enclosing the <svg> in a <div> and changing that <div>'s width/height CSS property, to simulate a window resize -- hence I never ran into this bug!

It's a bit confusing because current browsers don't observe it, but thankfully it works with NRWT :)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.