[webkit-reviews] review denied: [Bug 18294] Strange Result for getComputedStyle on borderWidth set in em : [Attachment 122486] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Jan 17 10:48:54 PST 2012
Tony Chang <tony at chromium.org> has denied Alexis Menard (darktears)
<alexis.menard at openbossa.org>'s request for review:
Bug 18294: Strange Result for getComputedStyle on borderWidth set in em
https://bugs.webkit.org/show_bug.cgi?id=18294
Attachment 122486: Patch
https://bugs.webkit.org/attachment.cgi?id=122486&action=review
------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=122486&action=review
I think you're right that it's safe to use more bits because the size isn't
changing (we're probably 4 byte aligning), but we should add a compile assert
to verify.
Color also packs poorly because of the bool it contains, but there's probably
no way to get that back.
> Source/WebCore/css/CSSStyleApplyProperty.cpp:1710
> + setPropertyHandler(CSSPropertyOutlineWidth,
ApplyPropertyComputeLength<unsigned short, &RenderStyle::outlineWidth,
&RenderStyle::setOutlineWidth, &RenderStyle::initialOutlineWidth,
NormalDisabled, ThicknessEnabled>::createHandler());
> + setPropertyHandler(CSSPropertyWebkitColumnRuleWidth,
ApplyPropertyComputeLength<unsigned short, &RenderStyle::columnRuleWidth,
&RenderStyle::setColumnRuleWidth, &RenderStyle::initialOutlineWidth,
NormalDisabled, ThicknessEnabled>::createHandler());
Should this be initialColumnWidth or initialColumnRuleWidth? This fix seems
unrelated to this patch.
> Source/WebCore/page/animation/AnimationBase.cpp:87
> +static inline unsigned blendFunc(const AnimationBase*, unsigned from,
unsigned to, double progress)
Is there a version of this function using "unsigned short" that we can remove?
> Source/WebCore/rendering/RenderTheme.cpp:114
> + if (static_cast<unsigned>(borderBox.top().value()) !=
style->borderTopWidth()) {
I would cast border*Width to an int because it's going to fit (since it's only
27 bits) and Length::value() could be negative. That said, this still makes me
a bit nervous.
More information about the webkit-reviews
mailing list