Summary: | Clicking input type=range with padding or border sets wrong value | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Keishi Hattori <keishi> | ||||||
Component: | Forms | Assignee: | Keishi Hattori <keishi> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | mifenton, tkent, webkit.review.bot, zan | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | 94578 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Keishi Hattori
2012-08-20 05:48:21 PDT
Created attachment 159409 [details]
Patch
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. 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) Created attachment 159602 [details]
Patch
Comment on attachment 159602 [details]
Patch
ok
Comment on attachment 159602 [details] Patch Clearing flags on attachment: 159602 Committed r126132: <http://trac.webkit.org/changeset/126132> All reviewed patches have been landed. Closing bug. 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? (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. Bug 94585 is in the commit queue. I was able to change the test in a way that you won't need to rebaseline. > 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.
|