Bug 94473 – Clicking input type=range with padding or border sets wrong value

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Keishi Hattori]

Clicking input type=range with padding or border sets wrong value.

[Keishi Hattori]

Created attachment 159409 [details]
Patch

[Kent Tamura]

Comment on attachment 159409 [details]
Patch

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

> Source/WebCore/html/shadow/SliderThumbElement.cpp:273
>          trackSize = input->renderBox()->contentHeight() - renderBox()->height();

Need to change to trackElement->renderBox()->contentHeight()?

> Source/WebCore/html/shadow/SliderThumbElement.cpp:274
> +        position = offset.y() - renderBox()->height() / 2 - trackBoundingBox.y() + inputBoundingBox.y();

Doesn't  - (input->renderBox()->borderTop() + input->renderBox()->paddingTop()) work? If it works, we can remove trackBoundingBox and inputBoundingBox.

[Keishi Hattori]

Comment on attachment 159409 [details]
Patch

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

>> Source/WebCore/html/shadow/SliderThumbElement.cpp:274
>> +        position = offset.y() - renderBox()->height() / 2 - trackBoundingBox.y() + inputBoundingBox.y();
> 
> Doesn't  - (input->renderBox()->borderTop() + input->renderBox()->paddingTop()) work? If it works, we can remove trackBoundingBox and inputBoundingBox.

I think that will work for all cases where user is styling the input element, but using trackBoundingBox will be more robust to styling pseudo elements. (e.g. adding margin to the track element or adding ::before to slider container element)

[Keishi Hattori]

Created attachment 159602 [details]
Patch

[Kent Tamura]

Comment on attachment 159602 [details]
Patch

ok

[WebKit Review Bot]

Comment on attachment 159602 [details]
Patch

Clearing flags on attachment: 159602

Committed r126132: <http://trac.webkit.org/changeset/126132>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Zan Dobersek]

The test introduced in the reviewed patch has been failing on all non-Chromium ports:
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=fast%2Fforms%2Frange%2Frange-hit-test-with-padding.html

How is this test expected to behave on ports that have Shadom DOM or subpixel layout or perhaps both disabled?

[Keishi Hattori]

(In reply to comment #8)
> The test introduced in the reviewed patch has been failing on all non-Chromium ports:
> http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=fast%2Fforms%2Frange%2Frange-hit-test-with-padding.html
> 
> How is this test expected to behave on ports that have Shadom DOM or subpixel layout or perhaps both disabled?

Ports without subpixel layout won't match up exactly. I will be changing the test in Bug 94585. Please rebaseline when it lands.

I don't know of any ports that support <input type=range> but don't have Shadow DOM.

[Keishi Hattori]

Bug 94585 is in the commit queue. I was able to change the test in a way that you won't need to rebaseline.

[Keishi Hattori]

> I don't know of any ports that support <input type=range> but don't have Shadow DOM.
I misunderstood. This patch has nothing to do with the Shadow DOM flag.