Bug 91736 - Use NSSliderCell to draw slider track
Summary: Use NSSliderCell to draw slider track
Status: NEW
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:
Depends on:
Blocks:
 
Reported: 2012-07-19 05:48 PDT by Keishi Hattori
Modified: 2014-02-05 10:58 PST (History)
9 users (show)

See Also:


Attachments
Patch (155.35 KB, patch)
2012-07-19 05:53 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (156.05 KB, patch)
2012-07-19 22:05 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Screenshot (6.99 KB, image/png)
2012-07-19 22:06 PDT, Keishi Hattori
no flags Details
Archive of layout-test-results from gce-cr-linux-05 (1.35 MB, application/zip)
2012-07-19 22:38 PDT, WebKit Review Bot
no flags Details
Patch (155.05 KB, patch)
2012-07-20 03:51 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (155.08 KB, patch)
2012-07-22 19:25 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-07-19 05:48:12 PDT
Use NSSliderCell to draw slider track so we get the native look.
Comment 1 Keishi Hattori 2012-07-19 05:53:16 PDT
Created attachment 153238 [details]
Patch
Comment 2 Build Bot 2012-07-19 06:06:19 PDT
Comment on attachment 153238 [details]
Patch

Attachment 153238 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13284862
Comment 3 Alexey Proskuryakov 2012-07-19 09:12:14 PDT
Comment on attachment 153238 [details]
Patch

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

> Source/WebCore/rendering/RenderThemeMac.mm:80
> +@interface SliderTrackCell : NSSliderCell

I think that all classes defined in WebKit should have a "Web" prefix.

> LayoutTests/ChangeLog:13
> +        * platform/chromium-mac/fast/forms/datalist/input-appearance-range-with-datalist-expected.png:
> +        * platform/chromium-mac/fast/forms/datalist/input-appearance-range-with-datalist-zoomed-expected.png:
> +        * platform/chromium-mac/fast/forms/input-appearance-height-expected.png:
> +        * platform/chromium-mac/fast/forms/range/input-appearance-range-expected.png:
> +        * platform/chromium-mac/fast/forms/range/slider-padding-expected.png:
> +        * platform/chromium-mac/fast/forms/range/slider-thumb-shared-style-expected.png:

How does this affect regular Mac results? I don't know if we ever got complaints about these controls looking non-native.
Comment 4 Keishi Hattori 2012-07-19 22:05:21 PDT
Created attachment 153405 [details]
Patch
Comment 5 Keishi Hattori 2012-07-19 22:06:18 PDT
Created attachment 153406 [details]
Screenshot
Comment 6 Keishi Hattori 2012-07-19 22:08:26 PDT
Comment on attachment 153238 [details]
Patch

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

>> Source/WebCore/rendering/RenderThemeMac.mm:80
>> +@interface SliderTrackCell : NSSliderCell
> 
> I think that all classes defined in WebKit should have a "Web" prefix.

Renamed to WebCoreSliderTrackCell.

>> LayoutTests/ChangeLog:13
>> +        * platform/chromium-mac/fast/forms/range/slider-thumb-shared-style-expected.png:
> 
> How does this affect regular Mac results? I don't know if we ever got complaints about these controls looking non-native.

Here is a screenshot in HiDPI mode comparing the before and after. The border, inner shadow, and background gradient are all different.
https://bug-91736-attachments.webkit.org/attachment.cgi?id=153406
Comment 7 WebKit Review Bot 2012-07-19 22:38:34 PDT
Comment on attachment 153405 [details]
Patch

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

New failing tests:
css3/filters/effect-opacity.html
Comment 8 WebKit Review Bot 2012-07-19 22:38:39 PDT
Created attachment 153416 [details]
Archive of layout-test-results from gce-cr-linux-05

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-05  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment 9 Keishi Hattori 2012-07-20 03:51:36 PDT
Created attachment 153470 [details]
Patch
Comment 10 Keishi Hattori 2012-07-22 19:25:26 PDT
Created attachment 153719 [details]
Patch
Comment 11 Keishi Hattori 2012-07-25 12:25:16 PDT
Could someone review this? I have some patches that rely on this change. ap@ or jonlee@?
Comment 12 Keishi Hattori 2012-07-25 21:01:14 PDT
To be more specific the upcoming patch will adjust the slider track position so we can draw an arrow slider knob for <input type=range> with <datalist>.
Comment 13 Anders Carlsson 2014-02-05 10:58:29 PST
Comment on attachment 153719 [details]
Patch

Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.