The location of the slider thumb is set by the slider container assuming the render slider has been laid out. When this happens, the location of the slider thumb is reset during the layout ignoring any previous location set. If the slider thumb is not laid out, the previous value is added to the new one by the slider container.
Created attachment 175231 [details] Patch
Comment on attachment 175231 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175231&action=review > Source/WebCore/html/shadow/SliderThumbElement.cpp:185 > + if (input->sliderThumbElement() && input->sliderThumbElement()->renderer()) { > + thumb = toRenderBox(input->sliderThumbElement()->renderer()); > + // Reset the thumb location before layout. > + thumb->setLocation(LayoutPoint()); > + } When we call thumbLocation.setX, can we set the position to offset rather than adding in the old position?
(In reply to comment #2) > (From update of attachment 175231 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=175231&action=review Thanks for reviewing. > > Source/WebCore/html/shadow/SliderThumbElement.cpp:185 > > + if (input->sliderThumbElement() && input->sliderThumbElement()->renderer()) { > > + thumb = toRenderBox(input->sliderThumbElement()->renderer()); > > + // Reset the thumb location before layout. > > + thumb->setLocation(LayoutPoint()); > > + } > > When we call thumbLocation.setX, can we set the position to offset rather than adding in the old position? Yes, I think so, I'll try
Comment on attachment 175231 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175231&action=review > Source/WebCore/ChangeLog:13 > + ignoring any previous location set. If the slider thumb is not > + laid out, the previous value is added to the new one by the slider > + container. This sounds like it should be observable testable. Is it not?
It will be observable once https://bugs.webkit.org/show_bug.cgi?id=102352 is fixed. It's already covered by existing tests.
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 175231 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=175231&action=review > > Thanks for reviewing. > > > > Source/WebCore/html/shadow/SliderThumbElement.cpp:185 > > > + if (input->sliderThumbElement() && input->sliderThumbElement()->renderer()) { > > > + thumb = toRenderBox(input->sliderThumbElement()->renderer()); > > > + // Reset the thumb location before layout. > > > + thumb->setLocation(LayoutPoint()); > > > + } > > > > When we call thumbLocation.setX, can we set the position to offset rather than adding in the old position? > > Yes, I think so, I'll try The initial position of the thumb slider is set by the parent container in RenderBlock::layoutBlockChild() and it depends on the padding, border and margin. I don't know if the thumb can be affected by those, because after the layoutBlockChild() the thumb position is always 0. In case of being greater than 0 I guess it makes sense to add the offset, but I guess we should also use the current thumb position when the container is not laid out.
Committed r135388: <http://trac.webkit.org/changeset/135388>
I landed the patch without any modification in the end to be extra sure it doesn't break anything.
(In reply to comment #6) > The initial position of the thumb slider is set by the parent container in RenderBlock::layoutBlockChild() and it depends on the padding, border and margin. I don't know if the thumb can be affected by those, because after the layoutBlockChild() the thumb position is always 0. In case of being greater than 0 I guess it makes sense to add the offset, but I guess we should also use the current thumb position when the container is not laid out. If there was a padding/border/margin on the thumb, after bug 102352 lands, we're no longer going to be applying them on a relayout since we will reset the location to 0, 0 then not call layout on the thumb. I would probably call thumbLocation.setX directly on the new position since it's less code, but it doesn't really matter. Conceptually, I don't think it makes sense for web authors to be able to use CSS to control the position of the thumb.