Bug 149835 - Slider knobs should scale when rendering while zoomed
Summary: Slider knobs should scale when rendering while zoomed
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Mac Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-10-05 23:09 PDT by Wenson Hsieh
Modified: 2015-10-06 10:28 PDT (History)
5 users (show)

See Also:


Attachments
Patch (5.55 KB, patch)
2015-10-06 07:15 PDT, Wenson Hsieh
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2015-10-05 23:09:19 PDT
The knobs of sliders (<input>s of type range) should scale when pinching to zoom.
Comment 1 Wenson Hsieh 2015-10-06 07:15:14 PDT
Created attachment 262514 [details]
Patch
Comment 2 Darin Adler 2015-10-06 09:26:06 PDT
Comment on attachment 262514 [details]
Patch

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

Is there a good way to make a regression test for this? I don’t like making this fix without a test.

> Source/WebCore/platform/mac/ThemeMac.mm:688
> +        std::unique_ptr<ImageBuffer> imageBuffer = ImageBuffer::createCompatibleBuffer(rect.size() + 2 * FloatSize(buttonFocusRectOutlineWidth, buttonFocusRectOutlineWidth), deviceScaleFactor, ColorSpaceSRGB, context, false);

I suggest we use auto here instead of explicitly listing the type.

> Source/WebCore/platform/mac/ThemeMac.mm:697
> +        needsRepaint = drawCellOrFocusRingIntoRectWithView(cell, NSRect(rect), view, drawButtonCell, drawFocusRing);

Why the syntax NSRect(rect) here? Won’t the code compile without the explicit conversation?
Comment 3 Wenson Hsieh 2015-10-06 10:11:40 PDT
Comment on attachment 262514 [details]
Patch

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

Thank you for the review! In regards to testing, there isn't currently a good way to test rendering of zoomed/scaled form controls. When getting other widgets to zoom a few weeks ago, I discussed ways to test this new behavior with Dan and Alexey, and we determined the only way to do this was to write pixel tests, but that since we don't run pixel tests the bots, writing new pixel tests wasn't that useful. We brainstormed a few ideas to address this issue and ended up filing https://bugs.webkit.org/show_bug.cgi?id=147193, but we'll need to have a broader conversation about ways to test rendering in WebKit beyond pixel tests.

>> Source/WebCore/platform/mac/ThemeMac.mm:688
>> +        std::unique_ptr<ImageBuffer> imageBuffer = ImageBuffer::createCompatibleBuffer(rect.size() + 2 * FloatSize(buttonFocusRectOutlineWidth, buttonFocusRectOutlineWidth), deviceScaleFactor, ColorSpaceSRGB, context, false);
> 
> I suggest we use auto here instead of explicitly listing the type.

Got it -- changed.

>> Source/WebCore/platform/mac/ThemeMac.mm:697
>> +        needsRepaint = drawCellOrFocusRingIntoRectWithView(cell, NSRect(rect), view, drawButtonCell, drawFocusRing);
> 
> Why the syntax NSRect(rect) here? Won’t the code compile without the explicit conversation?

Unfortunately, without the explicit conversion, 32-bit builds won't compile.
Comment 4 Wenson Hsieh 2015-10-06 10:28:31 PDT
Committed r190623: <http://trac.webkit.org/changeset/190623>