Bug 87844 - Implement painting slider tick marks
Summary: Implement painting slider tick marks
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keishi Hattori
URL:
Keywords: WebExposed
Depends on: 89067 89544
Blocks: 86821
  Show dependency treegraph
 
Reported: 2012-05-30 04:31 PDT by Keishi Hattori
Modified: 2012-07-19 09:20 PDT (History)
14 users (show)

See Also:


Attachments
Patch (14.52 KB, patch)
2012-05-30 22:21 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (30.75 KB, patch)
2012-06-01 04:57 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (28.50 KB, patch)
2012-06-03 19:30 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (32.44 KB, patch)
2012-06-04 23:52 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (37.19 KB, patch)
2012-06-05 03:50 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (39.59 KB, patch)
2012-06-06 04:49 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (71.78 KB, patch)
2012-06-13 06:38 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-04 (939.29 KB, application/zip)
2012-06-13 15:39 PDT, WebKit Review Bot
no flags Details
Patch (75.57 KB, patch)
2012-06-13 20:07 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (81.03 KB, patch)
2012-06-14 00:28 PDT, Keishi Hattori
tkent: review-
gns: commit-queue-
Details | Formatted Diff | Diff
Reabsed, used HTMLInputElement::sliderThumbElement, fixed Qt build. (38.81 KB, patch)
2012-06-18 02:41 PDT, Keishi Hattori
tkent: review-
Details | Formatted Diff | Diff
Removed unrelated test expectation changes. (31.93 KB, patch)
2012-06-18 19:22 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (54.07 KB, patch)
2012-06-18 20:34 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (54.23 KB, patch)
2012-06-18 22:06 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (54.22 KB, patch)
2012-07-17 06:00 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (56.12 KB, patch)
2012-07-17 20:37 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (57.13 KB, patch)
2012-07-17 22:26 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (57.09 KB, patch)
2012-07-17 22:56 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (57.21 KB, patch)
2012-07-18 21:36 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keishi Hattori 2012-05-30 04:31:23 PDT
Implement painting slider tick marks for <datalist> for <input type=range>
Comment 1 Keishi Hattori 2012-05-30 22:21:59 PDT
Created attachment 144987 [details]
Patch
Comment 2 Kent Tamura 2012-05-30 22:32:10 PDT
Comment on attachment 144987 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=144987&action=review

> Source/WebCore/ChangeLog:8
> +        No new tests. Tests will be added when turned on.

This patch affect platforms with ENABLE_DATALIST, right?  So, we need tests now.

> Source/WebCore/rendering/RenderThemeChromiumLinux.h:63
> +        virtual Color sliderTickColor() const;
> +        virtual IntSize sliderTickSize() const;
> +        virtual IntSize sliderThumbSize() const;
> +        virtual int sliderTickOffsetFromTrackCenter() const;

Need OVERRIDE.
Comment 3 Keishi Hattori 2012-06-01 04:57:17 PDT
Created attachment 145272 [details]
Patch
Comment 4 Build Bot 2012-06-01 05:03:36 PDT
Comment on attachment 145272 [details]
Patch

Attachment 145272 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12870439
Comment 5 Build Bot 2012-06-01 05:19:33 PDT
Comment on attachment 145272 [details]
Patch

Attachment 145272 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12868491
Comment 6 Gyuyoung Kim 2012-06-01 05:57:33 PDT
Comment on attachment 145272 [details]
Patch

Attachment 145272 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/12861601
Comment 7 Keishi Hattori 2012-06-03 19:30:47 PDT
Created attachment 145503 [details]
Patch
Comment 8 Kent Tamura 2012-06-03 22:26:13 PDT
Comment on attachment 145503 [details]
Patch

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.
Comment 9 Keishi Hattori 2012-06-04 23:52:31 PDT
Created attachment 145703 [details]
Patch
Comment 10 Kent Tamura 2012-06-05 01:17:22 PDT
Comment on attachment 145703 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=145703&action=review

> Source/WebCore/html/shadow/SliderThumbElement.cpp:154
> +                    if (trackRenderer)
> +                        trackRenderer->style()->setHeight(thumbRenderer->style()->height());
> +#if ENABLE(DATALIST)

I'd like to change this to

  Length height = thumbRenderer->style()->height();
  if (trackRenderr)
      trackRenderer->style()->setHeight(height);

and

> Source/WebCore/html/shadow/SliderThumbElement.cpp:167
> +                        style()->setHeight(Length(trackHeight, Fixed));
> +                    } else
> +#endif
> +                        style()->setHeight(thumbRenderer->style()->height());

height = Length(trackHeight, Fixed);
}
style()->setHeight(height);

> Source/WebCore/rendering/RenderTheme.h:217
> +    virtual IntSize sliderTickSize() const;

Unclear if this is for horizontal or vertical.

> Source/WebCore/rendering/RenderTheme.h:218
> +    virtual IntSize sliderThumbSize() const;

This should be integrated with RenderTheme::adjustSliderThumbSize().

> Source/WebCore/rendering/RenderTheme.h:219
> +    virtual int sliderTickOffsetFromTrackCenter() const;

We have an assumption that abs(sliderTickOffsetFromTrackCenter) * 2 > height-of-slider-thumb, right?  We should add a comment.
Comment 11 Keishi Hattori 2012-06-05 03:46:33 PDT
(In reply to comment #10)
> (From update of attachment 145703 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=145703&action=review
> 
> > Source/WebCore/html/shadow/SliderThumbElement.cpp:154
> > +                    if (trackRenderer)
> > +                        trackRenderer->style()->setHeight(thumbRenderer->style()->height());
> > +#if ENABLE(DATALIST)
> 
> I'd like to change this to
> 
>   Length height = thumbRenderer->style()->height();
>   if (trackRenderr)
>       trackRenderer->style()->setHeight(height);
> 
> and
> 
> > Source/WebCore/html/shadow/SliderThumbElement.cpp:167
> > +                        style()->setHeight(Length(trackHeight, Fixed));
> > +                    } else
> > +#endif
> > +                        style()->setHeight(thumbRenderer->style()->height());
> 
> height = Length(trackHeight, Fixed);
> }
> style()->setHeight(height);
Done.

> > Source/WebCore/rendering/RenderTheme.h:217
> > +    virtual IntSize sliderTickSize() const;
> 
> Unclear if this is for horizontal or vertical.

Renamed to horizontalSliderTickSize and horizontalSliderThumbSize.

> > Source/WebCore/rendering/RenderTheme.h:218
> > +    virtual IntSize sliderThumbSize() const;
> 
> This should be integrated with RenderTheme::adjustSliderThumbSize().
Done.

> > Source/WebCore/rendering/RenderTheme.h:219
> > +    virtual int sliderTickOffsetFromTrackCenter() const;
> 
> We have an assumption that abs(sliderTickOffsetFromTrackCenter) * 2 > height-of-slider-thumb, right?  We should add a comment.

Yes it would be true as long as the length of the thumb on the tick side of the track is longer than the other side.  Does this assumption need to be in the comment?
Comment 12 Keishi Hattori 2012-06-05 03:50:44 PDT
Created attachment 145751 [details]
Patch
Comment 13 Kent Tamura 2012-06-05 21:33:06 PDT
Comment on attachment 145751 [details]
Patch

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).
Comment 14 Kent Tamura 2012-06-05 21:48:11 PDT
Comment on attachment 145751 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=145751&action=review

>> 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).

Also, you need to reject value<min and value>max.
HTMLInputElement::isValidValue() might be useful.
Comment 15 Kent Tamura 2012-06-05 21:50:01 PDT
Comment on attachment 145751 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=145751&action=review

> LayoutTests/fast/forms/datalist/input-appearance-range-with-datalist.html:17
> +<datalist id=foo>
> + <option>0</option>, 
> + <option>10</option>
> + <option>20</option>
> + <option>40</option>
> + <option>80</option>
> + <option>100</option>
> +</datalist>

Please add
 <option>-10</option>
 <option>120</option>
 <option>50.1</option>
 <option>Infinity</option>
 etc.
Comment 16 Keishi Hattori 2012-06-06 04:49:20 PDT
Created attachment 145991 [details]
Patch
Comment 17 Keishi Hattori 2012-06-13 06:38:10 PDT
Created attachment 147306 [details]
Patch
Comment 18 WebKit Review Bot 2012-06-13 15:39:46 PDT
Comment on attachment 147306 [details]
Patch

Attachment 147306 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12960006

New failing tests:
fast/repaint/slider-thumb-float.html
fast/dom/HTMLInputElement/input-slider-update.html
fast/dom/HTMLInputElement/input-slider-update-styled.html
fast/repaint/slider-thumb-drag-release.html
Comment 19 WebKit Review Bot 2012-06-13 15:39:50 PDT
Created attachment 147429 [details]
Archive of layout-test-results from ec2-cr-linux-04

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-04  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 20 Keishi Hattori 2012-06-13 20:07:19 PDT
Created attachment 147470 [details]
Patch
Comment 21 Kent Tamura 2012-06-13 20:13:31 PDT
Comment on attachment 147470 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=147470&action=review

> Source/WebCore/css/html.css:573
> +    color: #909090;

Let's make a separated patch to update html.css, themeChromiumLinux.css, and themeWin.css.

> Source/WebCore/rendering/RenderTheme.cpp:972
> +IntSize RenderTheme::sliderTickSize() const
> +{
> +    return IntSize(1, 1);
> +}
> +
> +int RenderTheme::sliderTickOffsetFromTrackCenter() const
> +{
> +    return 0;
> +}

Remove them.  These default implementations are useless.

> Source/WebCore/rendering/RenderTheme.cpp:993
> +    RenderObject* thumbRenderer = input->shadow()->oldestShadowRoot()->firstChild()->firstChild()->firstChild()->renderer();

We had better have a helper function to get a thumb element.

> Source/WebCore/rendering/RenderTheme.h:220
> +    // Returns size of one slider tick mark for a horizontal track.
> +    virtual IntSize sliderTickSize() const;
> +    // Returns the distance of slider tick origin from the slider track center.
> +    virtual int sliderTickOffsetFromTrackCenter() const;

Make them pure virtual.
Also, we need a comment for sliderTickSize about rotation for vertical sliders.

> Source/WebCore/rendering/RenderTheme.h:221
> +    virtual void paintSliderTicks(RenderObject*, const PaintInfo&, const IntRect&);

This can be non-virtual.
Comment 22 Keishi Hattori 2012-06-14 00:28:16 PDT
Created attachment 147507 [details]
Patch
Comment 23 Keishi Hattori 2012-06-14 00:32:14 PDT
Comment on attachment 147470 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=147470&action=review

>> Source/WebCore/css/html.css:573
>> +    color: #909090;
> 
> Let's make a separated patch to update html.css, themeChromiumLinux.css, and themeWin.css.

Done. Bug 89067

>> Source/WebCore/rendering/RenderTheme.cpp:972
>> +}
> 
> Remove them.  These default implementations are useless.

Done.

>> Source/WebCore/rendering/RenderTheme.cpp:993
>> +    RenderObject* thumbRenderer = input->shadow()->oldestShadowRoot()->firstChild()->firstChild()->firstChild()->renderer();
> 
> We had better have a helper function to get a thumb element.

Done. Added HTMLInputElement::sliderThumbElement()

>> Source/WebCore/rendering/RenderTheme.h:220
>> +    virtual int sliderTickOffsetFromTrackCenter() const;
> 
> Make them pure virtual.
> Also, we need a comment for sliderTickSize about rotation for vertical sliders.

Done.

>> Source/WebCore/rendering/RenderTheme.h:221
>> +    virtual void paintSliderTicks(RenderObject*, const PaintInfo&, const IntRect&);
> 
> This can be non-virtual.

Done.
Comment 24 Gustavo Noronha (kov) 2012-06-14 00:48:01 PDT
Comment on attachment 147507 [details]
Patch

Attachment 147507 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12958147
Comment 25 Build Bot 2012-06-14 00:53:36 PDT
Comment on attachment 147507 [details]
Patch

Attachment 147507 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12961103
Comment 26 Gyuyoung Kim 2012-06-14 00:56:32 PDT
Comment on attachment 147507 [details]
Patch

Attachment 147507 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/12965075
Comment 27 Build Bot 2012-06-14 00:59:53 PDT
Comment on attachment 147507 [details]
Patch

Attachment 147507 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12955315
Comment 28 Early Warning System Bot 2012-06-14 01:05:34 PDT
Comment on attachment 147507 [details]
Patch

Attachment 147507 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12949898
Comment 29 Early Warning System Bot 2012-06-14 01:27:34 PDT
Comment on attachment 147507 [details]
Patch

Attachment 147507 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12956258
Comment 30 Jon Lee 2012-06-14 11:32:04 PDT
As an aside: are there no proposals for CSS attributes that allow for specifying which side the ticks appear?
Comment 31 Keishi Hattori 2012-06-17 19:23:16 PDT
(In reply to comment #30)
> As an aside: are there no proposals for CSS attributes that allow for specifying which side the ticks appear?

I'm afraid not. If you have plans to use it, we have been thinking about ideas for doing that kind of non standard customizations for form controls. I would love to discuss!
Comment 32 Kent Tamura 2012-06-17 22:38:06 PDT
Comment on attachment 147507 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=147507&action=review

r- because we need rebase.

> Source/WebCore/html/shadow/SliderThumbElement.cpp:150
> +                RenderObject* thumbRenderer = input->shadow()->oldestShadowRoot()->firstChild()->firstChild()->firstChild()->renderer();

We had better use sliderThumbElement().
Comment 33 Keishi Hattori 2012-06-18 02:41:44 PDT
Created attachment 148078 [details]
Reabsed, used HTMLInputElement::sliderThumbElement, fixed Qt build.
Comment 34 Kent Tamura 2012-06-18 03:05:56 PDT
Comment on attachment 148078 [details]
Reabsed, used HTMLInputElement::sliderThumbElement, fixed Qt build.

View in context: https://bugs.webkit.org/attachment.cgi?id=148078&action=review

The code looks good.  But the patch looks broken. r- because of purple EWS

> LayoutTests/fast/forms/range/slider-zoomed-expected.txt:2
> -PASS: slider value set to 72
> +FAIL: expected slider value 72, got 71

Is this related to this bug?

> LayoutTests/platform/chromium-linux/fast/dom/HTMLInputElement/input-slider-update-expected.txt:-1
> -layer at (0,0) size 800x600

ditto.

> LayoutTests/platform/chromium-linux/fast/dom/HTMLInputElement/input-slider-update-styled-expected.txt:-1
> -layer at (0,0) size 800x600

ditto.

> LayoutTests/platform/chromium-linux/fast/repaint/slider-thumb-drag-release-expected.txt:-2
> -  RenderView at (0,0) size 800x600

ditto.

> LayoutTests/platform/chromium-linux/fast/repaint/slider-thumb-float-expected.txt:-1
> -layer at (0,0) size 800x600

ditto.

> LayoutTests/platform/chromium-win/fast/dom/HTMLInputElement/input-slider-update-expected.txt:6
> -      RenderSlider {INPUT} at (2,2) size 129x21 [color=#C4C4C4] [bgcolor=#FFFFFF]
> +      RenderSlider {INPUT} at (2,2) size 129x21 [bgcolor=#FFFFFF]

ditto.

> LayoutTests/platform/chromium-win/fast/repaint/slider-thumb-drag-release-expected.txt:6
> -      RenderSlider {INPUT} at (2,2) size 129x21 [color=#C4C4C4] [bgcolor=#FFFFFF]
> +      RenderSlider {INPUT} at (2,2) size 129x21 [bgcolor=#FFFFFF]

ditto.

> Source/WebCore/rendering/RenderTheme.cpp:1023
> +        if (!o->style()->isLeftToRightDirection())
> +          tickPosition = 1.0 - tickPosition;

wrong indentation

> Source/WebCore/rendering/RenderThemeChromiumWin.cpp:475
> +    PlatformSupport::paintTrackbar(painter.context(),
> +                                  themeData.m_part,
> +                                  themeData.m_state,
> +                                  themeData.m_classicState,
> +                                  painter.drawRect());

inconsistent indentation.
Comment 35 Keishi Hattori 2012-06-18 19:22:33 PDT
Created attachment 148226 [details]
Removed unrelated test expectation changes.
Comment 36 Kent Tamura 2012-06-18 20:20:36 PDT
(In reply to comment #35)
> Created an attachment (id=148226) [details]
> Removed unrelated test expectation changes.

The patch doesn't contain binary diff.  That's the reason of purple EWS.


+</datalist>
diff --git a/LayoutTests/platform/chromium-linux/fast/forms/datalist/input-appearance-range-with-datalist-expected.png b/LayoutTests/platform/chromium-linux/fast/forms/datalist/input-appearance-range-with-datalist-expected.png
new file mode 100644
index 0000000..25b1824
Binary files /dev/null and b/LayoutTests/platform/chromium-linux/fast/forms/datalist/input-appearance-range-with-datalist-expected.png differ
diff --git a/LayoutTests/platform/chromium-linux/fast/forms/datalist/input-appearance-range-with-datalist-zoomed-expected.png b/LayoutTests/platform/chromium-linux/fast/forms/datalist/input-appearance-range-with-datalist-zoomed-expected.png
new file mode 100644
index 0000000..b9a5eb2
Binary files /dev/null and b/LayoutTests/platform/chromium-linux/fast/forms/datalist/input-appearance-range-with-datalist-zoomed-expected.png differ
diff --git a/LayoutTests/platform/chromium-mac/fast/forms/datalist/input-appearance-range-with-datalist-expected.png b/LayoutTests/platform/chromium-mac/fast/forms/datalist/input-appearance-range-with-datalist-expected.png
new file mode 100644
index 0000000..ad48c70
Binary files /dev/null and b/LayoutTests/platform/chromium-mac/fast/forms/datalist/input-appearance-range-with-datalist-expected.png differ
diff --git a/LayoutTests/platform/chromium-mac/fast/forms/datalist/input-appearance-range-with-datalist-zoomed-expected.png b/LayoutTests/platform/chromium-mac/fast/forms/datalist/input-appearance-range-with-datalist-zoomed-expected.png
new file mode 100644
index 0000000..7a9c091
Binary files /dev/null and b/LayoutTests/platform/chromium-mac/fast/forms/datalist/input-appearance-range-with-datalist-zoomed-expected.png differ
diff --git a/LayoutTests/platform/chromium-win/fast/forms/datalist/input-appearance-range-with-datalist-expected.png b/LayoutTests/platform/chromium-win/fast/forms/datalist/input-appearance-range-with-datalist-expected.png
new file mode 100644
index 0000000..74f650d
Binary files /dev/null and b/LayoutTests/platform/chromium-win/fast/forms/datalist/input-appearance-range-with-datalist-expected.png differ
diff --git a/LayoutTests/platform/chromium-win/fast/forms/datalist/input-appearance-range-with-datalist-zoomed-expected.png b/LayoutTests/platform/chromium-win/fast/forms/datalist/input-appearance-range-with-datalist-zoomed-expected.png
new file mode 100644
index 0000000..d1ce0fa
Binary files /dev/null and b/LayoutTests/platform/chromium-win/fast/forms/datalist/input-appearance-range-with-datalist-zoomed-expected.png differ
diff --git a/LayoutTests/platform/chromium/fast/forms/datalist/input-list-expected.txt b/LayoutTests/platform/chromium/fast/forms/datalist/input-list-expected.txt
index 4981a80..5556966 100644
--- a/LayoutTests/platform/chromium/fast/forms/datalist/input-list-expected.txt
Comment 37 Keishi Hattori 2012-06-18 20:34:35 PDT
Created attachment 148234 [details]
Patch
Comment 38 Kent Tamura 2012-06-18 21:52:42 PDT
Comment on attachment 148234 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148234&action=review

Almost ok.
But please do not commit this patch until resolving the <datalist> update issue.

> Source/WebCore/platform/qt/RenderThemeQt.cpp:412
> +IntSize RenderThemeQt::sliderTickSize() const
> +{
> +    return IntSize(0, 0);
> +}
> +
> +int RenderThemeQt::sliderTickOffsetFromTrackCenter() const
> +{
> +    return 0;
> +}
> +

Need to wrap by #if ENABLE(DATALIST) - #endif.
Need to add FIXME comments.
Comment 39 Keishi Hattori 2012-06-18 22:06:05 PDT
Created attachment 148245 [details]
Patch
Comment 40 Keishi Hattori 2012-07-17 06:00:13 PDT
Created attachment 152748 [details]
Patch
Comment 41 Keishi Hattori 2012-07-17 06:00:34 PDT
Rebased.
Comment 42 Gyuyoung Kim 2012-07-17 06:29:26 PDT
Comment on attachment 152748 [details]
Patch

Attachment 152748 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/13278142
Comment 43 Keishi Hattori 2012-07-17 20:37:29 PDT
Created attachment 152912 [details]
Patch
Comment 44 Kent Tamura 2012-07-17 20:41:49 PDT
Comment on attachment 152912 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=152912&action=review

> Source/WebCore/ChangeLog:9
> +
> +        This implements painting slider tick marks for <datalist> support for input type=range.
> +

Please add a sentence about a defect that this doesn't support automatic-update by list target changes and a following patch will fix it.

> LayoutTests/ChangeLog:17
> +        * platform/chromium-mac/fast/forms/datalist/input-appearance-range-with-datalist-expected.png: Added.
> +        * platform/chromium-mac/fast/forms/datalist/input-appearance-range-with-datalist-zoomed-expected.png: Added.

Probably these pixel results won't work on chromium-mac-snowleopard.
Please add a line to platform/chromium/TestExpectations.
Comment 45 Keishi Hattori 2012-07-17 22:26:16 PDT
Created attachment 152922 [details]
Patch
Comment 46 Kent Tamura 2012-07-17 22:34:59 PDT
Comment on attachment 152922 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=152922&action=review

> LayoutTests/platform/chromium/TestExpectations:3762
> +BUG87844 REBASELINE SNOWLEOPARD : fast/forms/datalist/input-appearance-range-with-datalist-rtl.html = IMAGE+TEXT
> +BUG87844 REBASELINE SNOWLEOPARD : fast/forms/datalist/input-appearance-range-with-datalist-zoomed.html = IMAGE+TEXT
> +BUG87844 REBASELINE SNOWLEOPARD : fast/forms/datalist/input-appearance-range-with-datalist.html = IMAGE+TEXT

Do not add REBASELINE.  It will confuse a WebKit gardener.
IMAGE+TEXT looks incorrect.  The text result doesn't depend on platforms.  I think it should be IMAGE.
Comment 47 Keishi Hattori 2012-07-17 22:56:07 PDT
Created attachment 152933 [details]
Patch
Comment 48 Kent Tamura 2012-07-17 23:00:38 PDT
Comment on attachment 152933 [details]
Patch

ok
Comment 49 Keishi Hattori 2012-07-18 21:36:54 PDT
Created attachment 153175 [details]
Patch
Comment 50 WebKit Review Bot 2012-07-18 22:30:29 PDT
Comment on attachment 153175 [details]
Patch

Clearing flags on attachment: 153175

Committed r123072: <http://trac.webkit.org/changeset/123072>
Comment 51 WebKit Review Bot 2012-07-18 22:30:38 PDT
All reviewed patches have been landed.  Closing bug.
Comment 52 Tony Chang 2012-07-19 09:20:37 PDT
Comment on attachment 153175 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=153175&action=review

> LayoutTests/platform/chromium/TestExpectations:3709
> +BUG87844 SNOWLEOPARD : fast/forms/datalist/input-appearance-range-with-datalist-rtl.html = IMAGE
> +BUG87844 SNOWLEOPARD : fast/forms/datalist/input-appearance-range-with-datalist-zoomed.html = IMAGE
> +BUG87844 SNOWLEOPARD : fast/forms/datalist/input-appearance-range-with-datalist.html = IMAGE

This was failing new-run-webkit-tests --lint-test-files because it was missing the WK.  Fixed in r123109.