Fix for ticket #3575

[Vipul B]

The ticket details a problem with Flash on jQuery which is fixed by
changing the last line of "$.ui.mouse._mouseUp" to "return true"
instead of "return false"

There is no change in the output of the visual or unit tests after
this change is applied (I tested version 1.7.1)

I would like to check this fix in. Can anybody tell me how I should go
about doing that ?

thanks,
Vipul

[Scott González]

Hey Vipul,

Thanks for the patch.  What you've done (attached patch to ticket, posted on this thread) is exactly the action you should be taking to get this committed.

I just reviewed the patch and I'm pretty sure the reason that we return false in mouseup is because we hide the original events (by canceling them) whenever an interaction is being performed through a plugin.  With that being said, returning false is not the correct behavior for mouseup, though neither is mousedown.  To be consistent, we should be returning the opposite of the current value of this._mouseStarted when we enter the event handler.

However, I'm not sure that we even need to be canceling the mousedown, mousemove, mouseup events.  We should probably only prevent the click event, which would make your change acceptable, but would require some other small changes as well.

Thoughts?

[Vipul B]

Thanks for the expedited response, Scott. I am not intimately familiar
with the implementation of jQuery-UI so please pardon my ignorance.

A couple of things :

1. I noticed that currently (version 1.7.1) , _mouseDown returns true
whereas _mouseUp returns false. Why is the mouseup being cancelled and
the mouseDown not ? Also, just to make sure I understand the
reasoning behind cancelling the browser events: is that being done so
that the events are localized to only the plugins and so that things
like draggable don't end up passing on the events further up ?

I would think that since these mouse events are important to a lot of
(most?) UIs and a part of ui.core.js, unless there are strong reasons,
they should not be cancelled and allowed to bubble up / be captured.
The plugins, when installed could change the behavior if they wanted
to ?

2. What would the other small changes you mention be ?

thanks,
Vipul

On Jul 6, 3:09 pm, Scott González <scott.gonza...@gmail.com> wrote:
> Hey Vipul,
>
> Thanks for the patch.  What you've done (attached patch to ticket, posted on
> this thread) is exactly the action you should be taking to get this
> committed.
>
> I just reviewed the patch and I'm pretty sure the reason that we return
> false in mouseup is because we hide the original events (by canceling them)
> whenever an interaction is being performed through a plugin.  With that
> being said, returning false is not the correct behavior for mouseup, though
> neither is mousedown.  To be consistent, we should be returning the opposite
> of the current value of this._mouseStarted when we enter the event handler.
>
> However, I'm not sure that we even need to be canceling the mousedown,
> mousemove, mouseup events.  We should probably only prevent the click event,
> which would make your change acceptable, but would require some other small
> changes as well.
>
> Thoughts?
>

[Scott González]

On Mon, Jul 6, 2009 at 3:27 PM, Vipul B <mez....@gmail.com> wrote:

Thanks for the expedited response, Scott. I am not intimately familiar
with the implementation of jQuery-UI so please pardon my ignorance.

A couple of things :

1. I noticed that currently (version 1.7.1) , _mouseDown returns true
whereas _mouseUp returns false. Why is the mouseup being cancelled and
the mouseDown not ?  Also, just to make sure I understand the
reasoning behind cancelling the browser events: is that being done so
that the events are localized to only the plugins and so that things
like draggable don't end up passing on the events further up ?

I'm not sure why we're canceling the mouseup event.  I do know that we cancel the click event because we're changing the event model.  So, if you actually do drag an element, or sort an element, then when you mouse up, you didn't perform a click (you performed some other action).  However, if you mouse down and mouse up on an element without moving (or you move but not long enough or far enough to cause some other action to occur), then the click event should be allowed because that is the actual action that occurred.

I would think that since these mouse events are important to a lot of
(most?) UIs and a part of ui.core.js, unless there are strong reasons,
they should not be cancelled and allowed to bubble up / be captured.
The plugins, when installed could change the behavior if they wanted
to ?

I agree that we shouldn't be messing with the mouse* events.  Hopefully someone else can chime in with why we're doing this.  Paul Bakaus? Richard Worth?

2. What would the other small changes you mention be ?

The other small changes would be to make sure we're not cancelling any other events that we shouldn't be cancelling (like mousedown).  These changes aren't directly related to each other, but we should be consistent in how we're handling the events so I'd like to update all of the handlers at the same time.

[Vipul B]

On Jul 6, 3:45 pm, Scott González <scott.gonza...@gmail.com> wrote:

>
> I'm not sure why we're canceling the mouseup event.  I do know that we
> cancel the click event because we're changing the event model.  So, if you
> actually do drag an element, or sort an element, then when you mouse up, you
> didn't perform a click (you performed some other action).  However, if you
> mouse down and mouse up on an element without moving (or you move but not
> long enough or far enough to cause some other action to occur), then the
> click event should be allowed because that is the actual action that
> occurred.
>

I agree. It might be better for the core to not cancel the events, but
if the plugins need to, they can do so. For example, let's say I am
building a zoom-plugin which is basically a transparent rectangle
floating above my HTML page and which zooms the content underneath it.
In this case, I might want the plugin to only zoom and not handle the
mouse events it is receiving and pass them on to the elements
underneath.

> I agree that we shouldn't be messing with the mouse* events.  Hopefully
> someone else can chime in with why we're doing this.  Paul Bakaus? Richard
> Worth?
>


> 2. What would the other small changes you mention be ?
>
> The other small changes would be to make sure we're not cancelling any other
> events that we shouldn't be cancelling (like mousedown).  These changes
> aren't directly related to each other, but we should be consistent in how
> we're handling the events so I'd like to update all of the handlers at the
> same time.

I notice that the return value of "mouseDown" was changed from true to
false in 1.6 rc 2.5 but mouseUp's return value was left unchanged
(false) :

463 // preventDefault() is used to prevent the
selection of text here -
464 // however, in Safari, this causes select boxes
not to be selectable
465 // anymore, so this fix is needed
466 if(!$.browser.safari) event.preventDefault();
467 return true;

It might be best if the developer of this snippet could be involved in
this discussion as well ...

Best,
Vipul