Bug 26229 – Can't click outside the slider thumb and start dragging

• Component: New Bugs
• Status: RESOLVED FIXED

[Pierre d'Herbemont]

STEPS:
1- Open LayoutTests/media/video-played.html
2- Wait for the page to be loaded
3- Click on the video slider where the slider thumb is not
4- attempt to slide it

OCCURS:
4- Doesn't move the slider thumb.

[Pierre d'Herbemont]

Created attachment 31017 [details]
patch v1.

Patch attached.

[Pierre d'Herbemont]

Created attachment 31018 [details]
patch v2.

This time with a changelog diff that will apply a bit better.

[Pierre d'Herbemont]

Comment on attachment 31018 [details]
patch v2.

In the case of the mouse click on the slider we shouldn't account the mouse position.

[Pierre d'Herbemont]

Created attachment 32494 [details]
patch v3.

[Simon Fraser (smfr)]

Comment on attachment 32494 [details]
patch v3.

> diff --git a/WebCore/rendering/RenderSlider.cpp b/WebCore/rendering/RenderSlider.cpp
> index 610a060..9e00d68 100644
> --- a/WebCore/rendering/RenderSlider.cpp
> +++ b/WebCore/rendering/RenderSlider.cpp
> @@ -127,41 +127,52 @@ private:
>      virtual Node* shadowParentNode() { return m_shadowParent; }
>  
>      Node* m_shadowParent;
> -    FloatPoint m_initialClickPoint;       // initial click point in RenderSlider-local coordinates
> -    int m_initialPosition;
>      bool m_inDragMode;
> +    FloatPoint m_mouseDragVectorToThumb;

Please move this above the bool to optimize packing. I'm not sure 'mouseDragVector' is really tbe best way to describe this. Is it "offsetFromThumb"? Maybe it should be a FloatSize? 

> +    , m_mouseDragVectorToThumb(0, 0)

No need to init a FloatPoint.

> +                }
> +                else {

Should be } else { on one line.

> +                    // We are outside the thumb, move the thumb to the point were
> +                    // we clicked. We'll be exactly at the center of the thumb.
> +                    m_mouseDragVectorToThumb.setX(0);
> +                    m_mouseDragVectorToThumb.setY(0);

Is it possible for the click to be off the end of the slider, so we'd end up with a negative thumb position? i.e. does this code need to handle end conditions?

> +FloatPoint RenderSlider::mouseEventVectorToThumb(MouseEvent* evt)
> +{
> +    ASSERT (m_thumb && m_thumb->renderer());
> +    FloatPoint localPoint = m_thumb->renderBox()->absoluteToLocal(evt->absoluteLocation(), false, true);
> +    IntRect thumbBounds = m_thumb->renderBox()->borderBoxRect();
> +    FloatPoint distance;
> +    distance.setX(thumbBounds.x() + thumbBounds.height() / 2 - localPoint.x());
> +    distance.setY(thumbBounds.y() + thumbBounds.width() / 2 - localPoint.y());

I think you have height and width switched.

> +    return distance;

You call this distance, but the method says vector.

r- because I think this needs another revision, but it's certainly in the right direction.

[Pierre d'Herbemont]

(In reply to comment #5)
> (From update of attachment 32494 [details])
> > diff --git a/WebCore/rendering/RenderSlider.cpp b/WebCore/rendering/RenderSlider.cpp
> > index 610a060..9e00d68 100644
> > --- a/WebCore/rendering/RenderSlider.cpp
> > +++ b/WebCore/rendering/RenderSlider.cpp
> > @@ -127,41 +127,52 @@ private:
> >      virtual Node* shadowParentNode() { return m_shadowParent; }
> >  
> >      Node* m_shadowParent;
> > -    FloatPoint m_initialClickPoint;       // initial click point in RenderSlider-local coordinates
> > -    int m_initialPosition;
> >      bool m_inDragMode;
> > +    FloatPoint m_mouseDragVectorToThumb;
> 
> Please move this above the bool to optimize packing. I'm not sure
> 'mouseDragVector' is really tbe best way to describe this. Is it
> "offsetFromThumb"? Maybe it should be a FloatSize? 

As this is an offset/vector and can contain negative value a Size is not appropriate.

> > +                    // We are outside the thumb, move the thumb to the point were
> > +                    // we clicked. We'll be exactly at the center of the thumb.
> > +                    m_mouseDragVectorToThumb.setX(0);
> > +                    m_mouseDragVectorToThumb.setY(0);
> 
> Is it possible for the click to be off the end of the slider, so we'd end up
> with a negative thumb position? i.e. does this code need to handle end
> conditions?

It's being handled properly already: We use the same code path than before, and we'll place the thumb in a constrained interval through RenderSlider::positionForOffset.

Rest should be addressed, thanks.

[Pierre d'Herbemont]

Created attachment 32500 [details]
patch v4.

[Pierre d'Herbemont]

http://trac.webkit.org/changeset/45658