[webkit-reviews] review denied: [Bug 87844] Implement painting slider tick marks : [Attachment 145751] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Jun 5 21:33:06 PDT 2012
Kent Tamura <tkent at chromium.org> has denied Keishi Hattori
<keishi at webkit.org>'s request for review:
Bug 87844: Implement painting slider tick marks
https://bugs.webkit.org/show_bug.cgi?id=87844
Attachment 145751: Patch
https://bugs.webkit.org/attachment.cgi?id=145751&action=review
------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=145751&action=review
> Source/WebCore/html/shadow/SliderThumbElement.cpp:168
> + style()->setHeight(Length(trackHeight, Fixed));
> + } else
> +#endif
> + style()->setHeight(height);
height = Length(trachHeight, Fixed));
}
style()->setHeight(height);
> Source/WebCore/rendering/RenderTheme.cpp:992
> + int tickRegionMargin = (thumbSize.width() - tickSize.width()) / 2.0;
You assume horizontal thumb width == vertical thumb height. I don't think we
should introduce such restriction. We should have
RenderThem::sliderThmbSize(ControlPart) or we should obtain the thumb size from
an existing renderer.
> Source/WebCore/rendering/RenderTheme.cpp:994
> + int tickRegionSideMargin = rect.x() + tickRegionMargin;
> + int tickRegionWidth = rect.width() - tickRegionMargin * 2 -
tickSize.width();
The initial values are not used.
> Source/WebCore/rendering/RenderTheme.cpp:1014
> + double value =
parseToDoubleForNumberType(input->sanitizeValue(optionElement->value()),
std::numeric_limits<double>::quiet_NaN());
You can omit the last argument of parseToDoubleForNumberType().
Also, you have to check isfinite(value).
More information about the webkit-reviews
mailing list