[webkit-reviews] review denied: [Bug 87844] Implement painting slider tick marks : [Attachment 145503] Patch

[bugzilla-daemon at webkit.org]

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 145503: Patch
https://bugs.webkit.org/attachment.cgi?id=145503&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=145503&action=review


> Source/WebCore/html/shadow/SliderThumbElement.cpp:163
> +			   style()->setHeight(Length(trackHeight, Fixed));
> +		       } else {

Don't we need to set trackRender's height?

> Source/WebCore/rendering/RenderTheme.cpp:1014
> +    i.context->setFillColor(sliderTickColor(), ColorSpaceDeviceRGB);

How about using o->style()->color() here and put the default color to html.css?
 We can remove sliderTickColor(), and page authors can override the color.

> LayoutTests/fast/forms/datalist/input-appearance-range-with-datalist.html:1
> +<input type=range list=foo />

- Please add a vertical slider too.
- We had better have another slider of which width is different in order to see
tick marks are correctly positioned.
- Tick marks are not shown in renderer dumps.  So we may call
layoutTestController.dumpAsText(true) to merge text results into one.