[webkit-reviews] review denied: [Bug 73682] Attribute: Remove style() since it returns the same as decl(). : [Attachment 117670] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Andreas Kling <kling at webkit.org>'s
request for review:
Bug 73682: Attribute: Remove style() since it returns the same as decl().
https://bugs.webkit.org/show_bug.cgi?id=73682

Attachment 117670: Patch
https://bugs.webkit.org/attachment.cgi?id=117670&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=117670&action=review


Great idea. The name decl is so sad.

review- because I don’t want to add that static_cast.

> Source/WebCore/svg/SVGStyledElement.cpp:440
> -    return cssSVGAttr->style()->getPropertyCSSValue(name);
> +    return
static_cast<CSSStyleDeclaration*>(cssSVGAttr->decl())->getPropertyCSSValue(name
);

This cast is not the right way to code this. There are three ways to do it that
I am OK with:

1) If we think it’s OK that CSSMutableStyleDeclaration hides the string
overloads for the getters, then we should write:

    cssSVGAttr->decl()->CSSStyleDeclaration::getPropertyCSSValue(name);

2) Or we could write:

    CSSStyleDeclaration* style = cssSVGAttr->decl();
    return style->getPropertyCSSValue(name);

3) But, best of all, we should probably do this:

    using CSSStyleDeclaration::getPropertyCSSValue;

If we put that in CSSMutableStyleDeclaration.h then it should bring down the
overloads of getPropertyCSSValue so they are visible and not hidden by the
function we are overriding with.