Bug 38307 - REGRESSION: gradient background of LayoutTests/fast/backgrounds/resources/balloon.svg fails to draw
Summary: REGRESSION: gradient background of LayoutTests/fast/backgrounds/resources/bal...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: All OS X 10.5
: P1 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-28 21:25 PDT by Simon Fraser (smfr)
Modified: 2010-04-29 02:07 PDT (History)
3 users (show)

See Also:


Attachments
Testcase (6.39 KB, image/svg+xml)
2010-04-28 21:26 PDT, Simon Fraser (smfr)
no flags Details
Fix regressions (1.33 MB, patch)
2010-04-29 01:07 PDT, Nikolas Zimmermann
mjs: review+
Details | Formatted Diff | Diff

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