[Webkit-unassigned] [Bug 59774] Add template parameter to ApplyPropertyColor to improve clarity by removing constructor parameter side effects.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Apr 29 07:55:35 PDT 2011
https://bugs.webkit.org/show_bug.cgi?id=59774
Dimitri Glazkov (Google) <dglazkov at chromium.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #91629|review?, commit-queue? |review-, commit-queue-
Flag| |
--- Comment #3 from Dimitri Glazkov (Google) <dglazkov at chromium.org> 2011-04-29 07:55:35 PST ---
(From update of attachment 91629)
View in context: https://bugs.webkit.org/attachment.cgi?id=91629&action=review
I like it, but a few nits:
> Source/WebCore/css/CSSStyleApplyProperty.cpp:132
> +template <bool inheritColorFromParent = false>
Can a template parameter be an enum, like enum InheritColor { InheritColorFromParent, WhateverTheOtherValue };
> Source/WebCore/css/CSSStyleApplyProperty.cpp:343
> + setPropertyValue(CSSPropertyColor, new ApplyPropertyColor<true>(&RenderStyle::color, &RenderStyle::setColor, 0, RenderStyle::initialColor));
This way, you could write this as new ApplyPropertyColor<IneritColorFromParent> -- much more readable.
> Source/WebCore/css/CSSStyleApplyProperty.cpp:404
> + setPropertyValue(CSSPropertyBackgroundColor, new ApplyPropertyColor<>(&RenderStyle::backgroundColor, &RenderStyle::setBackgroundColor, 0));
And without the default value, this could look prettier too, explicitly explaining what's happening.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list