In SVGRadialGradientElement::collectGradientAttributes(), though *current* element may not SVGRadialGradientElement, it always goes into if (isRadial) { ... } block on a first iteration because *isRadial* is initialized with true. This cl fix it by moving it to a proper place. Now, we don't need to keep *isRadial* flag. Remove it.
Created attachment 221002 [details] Patch
Created attachment 221003 [details] Patch
Comment on attachment 221003 [details] Patch Attachment 221003 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5940320948715520 New failing tests: svg/W3C-SVG-1.1/pservers-grad-13-b.svg svg/custom/gradient-rotated-bbox.svg svg/custom/relative-sized-content-with-resources.xhtml svg/W3C-SVG-1.1/coords-units-01-b.svg svg/custom/gradient-attr-update.svg svg/custom/gradient-userSpaceOnUse-with-percentage.svg svg/W3C-SVG-1.1/pservers-grad-12-b.svg svg/W3C-SVG-1.1/pservers-grad-02-b.svg svg/custom/gradient-with-1d-boundingbox.svg svg/W3C-SVG-1.1/struct-use-05-b.svg svg/W3C-SVG-1.1/pservers-grad-11-b.svg svg/custom/radial-gradient-with-outstanding-focalPoint.svg svg/W3C-SVG-1.1/pservers-grad-05-b.svg svg/W3C-SVG-1.1/pservers-grad-06-b.svg svg/W3C-SVG-1.1/pservers-grad-14-b.svg svg/W3C-SVG-1.1/pservers-grad-04-b.svg svg/custom/large-bounding-box-percents.svg svg/custom/radialGradient-focal-radius.svg
Created attachment 221009 [details] Archive of layout-test-results from webkit-ews-06 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-06 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 221003 [details] Patch Attachment 221003 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6423760454287360 New failing tests: svg/W3C-SVG-1.1/pservers-grad-13-b.svg svg/custom/gradient-rotated-bbox.svg svg/custom/large-bounding-box-percents.svg svg/custom/relative-sized-content-with-resources.xhtml svg/W3C-SVG-1.1/coords-units-01-b.svg svg/custom/gradient-attr-update.svg svg/custom/gradient-userSpaceOnUse-with-percentage.svg svg/W3C-SVG-1.1/pservers-grad-12-b.svg svg/W3C-SVG-1.1/pservers-grad-11-b.svg svg/W3C-SVG-1.1/pservers-grad-02-b.svg svg/W3C-SVG-1.1/struct-use-05-b.svg svg/custom/radial-gradient-with-outstanding-focalPoint.svg svg/W3C-SVG-1.1/pservers-grad-05-b.svg svg/W3C-SVG-1.1/pservers-grad-06-b.svg svg/W3C-SVG-1.1/pservers-grad-14-b.svg svg/W3C-SVG-1.1/pservers-grad-04-b.svg svg/custom/gradient-with-1d-boundingbox.svg svg/custom/radialGradient-focal-radius.svg
Created attachment 221012 [details] Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-11 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Created attachment 221047 [details] Patch
First patch changed behavior. So, I modify this patch.
Created attachment 221585 [details] Patch
Comment on attachment 221585 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221585&action=review > Source/WebCore/svg/SVGLengthContext.cpp:207 > return 0; Change this 0 to a nullptr while you are here. > Source/WebCore/svg/SVGLinearGradientElement.cpp:126 > +static void setGradientAttributes(SVGGradientElement* element, LinearGradientAttributes& attributes, bool isLinear = true) If SVGGradientElement* is never null, please convert it to a SVGGradientElement^ > Source/WebCore/svg/SVGRadialGradientElement.cpp:138 > +static void setGradientAttributes(SVGGradientElement* element, RadialGradientAttributes& attributes, bool isRadial = true) SVGGradientElement&
Created attachment 221607 [details] Patch
(In reply to comment #10) > (From update of attachment 221585 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=221585&action=review > > > Source/WebCore/svg/SVGLengthContext.cpp:207 > > return 0; > > Change this 0 to a nullptr while you are here. > > > Source/WebCore/svg/SVGLinearGradientElement.cpp:126 > > +static void setGradientAttributes(SVGGradientElement* element, LinearGradientAttributes& attributes, bool isLinear = true) > > If SVGGradientElement* is never null, please convert it to a SVGGradientElement^ > > > Source/WebCore/svg/SVGRadialGradientElement.cpp:138 > > +static void setGradientAttributes(SVGGradientElement* element, RadialGradientAttributes& attributes, bool isRadial = true) > > SVGGradientElement& Done. Thanks.
Comment on attachment 221607 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221607&action=review I do not see a huge improvement of the code in comparison to the previous code. Adapting to the new style rules is still a fine. r=me > Source/WebCore/ChangeLog:9 > + one of poor implementations. This cl fix it by extracting a logic into a new method. cl may be replaced.
Committed r162385: <http://trac.webkit.org/changeset/162385>
(In reply to comment #13) > (From update of attachment 221607 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=221607&action=review > > I do not see a huge improvement of the code in comparison to the previous code. Adapting to the new style rules is still a fine. r=me Yes, this one is just small refactor patch. However, I believe this patch is more make sense from code readability perspective. Thank you for your review. > > Source/WebCore/ChangeLog:9 > > + one of poor implementations. This cl fix it by extracting a logic into a new method. > > cl may be replaced. Fixed.