Opened 7 years ago

Closed 7 years ago

#8866 closed bug (fixed)

Menu: select event not firing due to mouseHandled flag reset bug

Reported by: MrBigDog2U Owned by:
Priority: minor Milestone: 1.10.0
Component: ui.menu Version: 1.9.2
Keywords: Cc:
Blocked by: Blocking:

Description

The ui.menu mouseHandled flag does not get reset properly when creating a menu within an event handler other than a button click.

The first time a menu is shown, the mouseHandled flag is set to false and the control works correctly. The .ui-menu-item click handler sets the mouseHandled flag to true but, if the menu is displayed a second time, the mouseHandled flag does not get reset. This results in the menu ignoring the .ui-menu-item click event in the second instance.

http://jsfiddle.net/x9kfK/

Change History (6)

comment:1 Changed 7 years ago by tj.vantoll

Minified - http://jsfiddle.net/tj_vantoll/9gGKW/. I can see the behavior you're referring to but it's only happening for me when the menu is created in a setTimeout. Not exactly sure what's going on there.

If you're looking for a workaround don't use setTimeout. Or for a better approach don't destroy and recreate the menu on every button click - http://jsfiddle.net/tj_vantoll/CGgB3/.

comment:2 Changed 7 years ago by MrBigDog2U

It will happen with other events as well - for example, when the menu is created in response to an AJAX success callback. This is the behavior that exposed the bug in the first place - my application requests data from the server and creates a menu with the information when the AJAX call returns. It behaves the same when the menu is created in response to a keyboard event (see http://jsfiddle.net/R2u95/). Pretty much ANY event other than a mouse click.

As I explained in the initial description, this is due to errant handling of the mouseHandled flag in the ui.menu. If the menu is created in response to a button click event, then the "onclick" handler resets the mouseHandled flag AFTER the menu is created (thus resetting the flag). In response to ANY other event, the "onclick" handler either doesn't fire or fires before the menu gets created with the incorrect mouseHandled flag value. The workaround I have implemented is to add a mouseHandled reset to the ui.menu _create method however I don't know if that approach will have side-effects in other parts of the widget.

I believe that I can use the approach of hiding and showing the menu like you demonstrated in your second jsfiddle link as a workaround that will not require changes to the jqueryui code.

comment:3 Changed 7 years ago by tj.vantoll

Status: newopen
Summary: JQuery UI menu select event not firing due to mouseHandled flag reset bugMenu: select event not firing due to mouseHandled flag reset bug

comment:4 Changed 7 years ago by Jörn Zaefferer

The mouseHandled flag is global, not per instance, so destroying the menu during the select event prevents the document-click handler from resetting the flag. Based on the current code I can't tell why that flag is there, would've have to work through some git logs to figure that out.

For now, I see two options: Make the flag per instance, so an invalid stage in one menu doesn't affect any others. Or reset the flag on _destroy.

Both would address the issue at hand, though it would be preferable to figure out why the flag is there in the first place and just get rid of it. For now, what TJ suggests is a better solution, since reusing the menu makes a lot more sense.

comment:5 Changed 7 years ago by Jörn Zaefferer

Actually that was pretty easy to find: https://github.com/jquery/jquery-ui/commit/b8ad711deedab11da1f181b486ee67f259a3ef7c Originally this was to remove the stopImmediatePropagation calls: https://github.com/jquery/jquery-ui/commit/26d6952bd2b81de2ad2adb0bb77c1be6f2d717c2

I tend to make that flag instance specific. I don't see why that needs to be shared across menu instances. Will ping Kris about it.

comment:6 Changed 7 years ago by Kris Borchers

Resolution: fixed
Status: openclosed

Create mouseHandled flag per instance instead of globally. Fixes #8866 - Menu: select event not firing due to mouseHandled flag reset bug

Changeset: 5143b7f672bc668963cce7dcf5dd4e2970aad8e5

Note: See TracTickets for help on using tickets.