Bug 38307

Summary: REGRESSION: gradient background of LayoutTests/fast/backgrounds/resources/balloon.svg fails to draw
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: SVGAssignee: Nikolas Zimmermann <zimmermann>
Status: RESOLVED FIXED    
Severity: Normal CC: krit, mitz, zimmermann
Priority: P1    
Version: 528+ (Nightly build)   
Hardware: All   
OS: OS X 10.5   
Attachments:
Description Flags
Testcase
none
Fix regressions mjs: review+

Description Simon Fraser (smfr) 2010-04-28 21:25:16 PDT
At some point the rendering of LayoutTests/fast/backgrounds/resources/balloon.svg changed; the gradient background no longer draws.

This breaks the LayoutTests/fast/backgrounds/svg-as-background-2.html test in pixel mode.
Comment 1 Simon Fraser (smfr) 2010-04-28 21:26:17 PDT
Created attachment 54669 [details]
Testcase
Comment 2 Simon Fraser (smfr) 2010-04-28 21:36:01 PDT
Broke in http://trac.webkit.org/changeset/58212.
Comment 3 Simon Fraser (smfr) 2010-04-28 22:06:18 PDT
It seems like the difference with this file is that the <linearGradient> is inside a <def>.

The <def> has no renderer, so we never create a renderer for the linearGradient, and therefore never call SVGDocumentExtensions::addResource() for it.

How is this supposed to work?
Comment 4 Nikolas Zimmermann 2010-04-29 00:40:01 PDT
Hi Simon,

thanks a lot for the investigation. Some details why the code changed recently:
We had a long-standing hack in SVGStopElement, to do manual style resolution, because of svg/W3C-SVG-1.1/pservers-grad-19-b.svg, which used code like:

<g display="none">
<linearGradient>
<stop>
...

RenderSVGGradientStop was never created because of the display="none" in the parent <g>.
To remove manual style resolution hacks (calling styleForRenderer() manually) I made SVGGElement always create renderers, even for display="none" (RenderSVGHiddenContainer, instead of the regular RenderSVGTransformableContainer).

For <defs> a RenderSVGHiddenContainer is always created, so renderers are _supposed_ to be created.
I will investigate today what is happening, stay tuned.
Comment 5 Nikolas Zimmermann 2010-04-29 00:42:59 PDT
Heh, the testcase uses <def>, instead of <defs>. That element does not exist, so the current rendering is actually a progression ;-) Obviously the testcase needs to be fixed, I'll prepare it.
Comment 6 Nikolas Zimmermann 2010-04-29 01:07:11 PDT
Created attachment 54681 [details]
Fix regressions

Updated all current failing expected.png files. Fixed problem in balloon.svg (s/def/defs/) -> works again as expected.
svg-as-background-2.html now properly paints the gradient, instead of pure red color, even an improvement.

I hope this also fixes the Chromium problems, that have been reported in bug 38108.
Comment 7 Maciej Stachowiak 2010-04-29 01:44:29 PDT
Comment on attachment 54681 [details]
Fix regressions

r=me
Comment 8 Nikolas Zimmermann 2010-04-29 02:07:49 PDT
Landed in r58489.