Draggable: Improve detection for when to blur the active element

[scottgonzalez]

Fixes #12472

I'd like to start a discussion about how we want to handle this. @tjvantoll and @mikesherov previously worked on this problem for http://bugs.jqueryui.com/ticket/10527. This expands on that solution to fix http://bugs.jqueryui.com/ticket/12472. However, the problem comes from mouse, but the fix is inside draggable. This means that every other interaction has this problem as well. There's actually an old ticket that's properly categorized as being a mouse issue: http://bugs.jqueryui.com/ticket/6642. So the question is, should we just fix the root cause now or just handle this in the interaction rewrite?

Also, how many and which test cases do we want to write? The DOM structures that exist are:

• active element is the handle
  • e.g., a draggable div which can gain focus
• active element is a descendant of the handle
  • e.g., an input inside a draggable that doesn't specify a handle (not great UX)
• active element is an ancestor of the handle
  • e.g., a draggable element inside a dialog
• active element is not associated with the handle
  • e.g., an input in another part of the document

We also have to consider which element the user is interacting with, as they can be interacting with the handle itself or a descendant of the handle. So it's possible that the active element is a descendant of the handle and the user is interacting with a descendant of the handle, but the active element and event target are siblings/cousins.

[tjvantoll]

So the question is, should we just fix the root cause now or just handle this in the interaction rewrite?

I don't see any reason not to handle it now. The interaction rewrite is going to have the same problem.

The code here looks fine to me. My one suggestion would be to add tests to cover the new cases you're adding—especially “active element is not associated with the handle”. There's already a test that handles this method.

[lepolt]

Any updates on this issue? Will get get pulled into jquery:master?

[scottgonzalez]

I need to rewrite this.

[workmanw]

👍. This has definitely caused a few issues in our app.

[lepolt]

Is this still in progress with a chance to make it to jquery:master? This might not be a common use case, but it's definitely a pain point for users in our app...

Any suggested workaround? Thanks.

[jzaefferer]

Rewrite it!

[dcherman]

👍 Has there been any more consideration here? We recently encountered this in several applications as well.

[scottgonzalez]

After giving this much thought yesterday, I think the generic solution is way too complicated for us to implement now prior to the rewrite (it may even be too complicated to write with the rewrite). I'm going to write some tests for this and land just the draggable fix. I want to make sure this gets into 1.12.0.

[scottgonzalez]

@dcherman Just to confirm, this patch does fix bug 14905 for you, right?

[workmanw]

@scottgonzalez I've confirmed that this patch does fix the issue in our app!

Additionally, we did some other basic testing with this patch (again in our app) and did not discover any issues introduced by it.

👍

[dcherman]

@scottgonzalez Seems like it, I replaced my patch with this one locally and the bug I was running into still looks to be fixed. Thanks!

[dcherman]

I think it'd be a good idea to change this condition to

if ( this._getHandle(event) && target.closest( activeElement ).length )

While this patch works, it still duplicates the logic of "did this event occur within a handle" which opens the possibility of future bugs if the two different implementations aren't kept in sync

[scottgonzalez]

I've incorporated @dcherman's suggestion, added a test, and rebased (this sure was old).

This does regress on https://bugs.jqueryui.com/ticket/10527, but that only affects IE8.

[arschmitz]

LGTM

[jzaefferer]

Please don't use onevar anymore.

[adawit]

This does regress on https://bugs.jqueryui.com/ticket/10527, but that only affects IE8.

This is not entirely correct. This change also regresses #10527 on the latest version of Chrome for me, Mac - El Capitan with Chrome version 52.0.2743.116 (64-bit) as well as FireFox 48.0 .

If you create a draggable bootstrap dialog with a text area and click inside of the text area, a blur event is fired even when the text area has the focus and the handle for Draggable is set to the header of the dialog.

Only solution for me was to downgrade to jquery-ui 1.11.4 where the _blurActiveElement method's behavior is different.

[JoolsCaesar]

Is there any setting or easy way to disable this new behaviour? This broke an app I'm working on when we made the 1.12 upgrade. We use draggable for interface splitters and it's really annoying that a control loses focus when you're just trying to resize it by moving a sibling splitter. Before the update we explicitly prevented this behaviour by calling preventDefault on any splitter mousedowns, but these changes just ignore that.

[scottgonzalez]

No. You could replace the method with your own logic if you really want to, but it's not a public method, so it can change at any time without notice.