Opened 9 years ago

Closed 9 years ago

#11836 closed defect (fixed)

[patch][cla] Form widgets interfere with dijit.Menu

Reported by: avoidscorn Owned by: Douglas Hays
Priority: high Milestone: 1.6
Component: Dijit Version: 1.5
Keywords: Cc:
Blocked By: Blocking:

Description

Tested on Firefox 3.6.10/Linux and Chromium 6.0.472.62/Linux.

If you bind a dijit.Menu to a form widget and then right-click on the form widget, the form widget will steal focus from the menu popup and close it as soon as you mouse-up.

The culprit is dijit._FormWidget._onMouseDown, which attempts to restore focus to the form widget even if the user mouse-ups somewhere else. This works fine unless the focus was on a popup that just opened.

The attach test case patch adds a form widget to test_Menu.js.

I'm attaching two possible fixes:

  1. ignoreRightClick.patch: dijit._FormWidget ignores right clicks for focus-grabbing purposes. This will not fix the issue for leftClickToOpen=true menus. However those menus don't work for form widgets anyway, because form widgets almost always capture left clicks, so it's a moot point.
  2. detectPopup.patch: dijit._FormWidget checks to see if a popup opened before it tries to grab focus. In theory, this will work for leftClickToOpen=true menus.

Another alternative is to make dijit.Menu try and stop mousedown/mouseup events from being detected by the form widget, but that seems risky.

Attachments (6)

testCase.patch (1.4 KB) - added by avoidscorn 9 years ago.
test_Menu.html patch
ignoreRightClick.patch (814 bytes) - added by avoidscorn 9 years ago.
_FormWidget.js patch to ignore right clicks
detectPopup.patch (998 bytes) - added by avoidscorn 9 years ago.
_FormWidget.js patch to detect popups
ignoreRightClick2.patch (832 bytes) - added by avoidscorn 9 years ago.
Uses dojo.mouseButtons.isLeft to detect left-clicks
robotTestCase.patch (2.0 KB) - added by avoidscorn 9 years ago.
tests/robot/Menu_mouse.html patch
testCase2.patch (1.4 KB) - added by avoidscorn 9 years ago.
Display "dijit.form._FormWidget" instead of "dijit._FormWidget"

Download all attachments as: .zip

Change History (13)

Changed 9 years ago by avoidscorn

Attachment: testCase.patch added

test_Menu.html patch

Changed 9 years ago by avoidscorn

Attachment: ignoreRightClick.patch added

_FormWidget.js patch to ignore right clicks

Changed 9 years ago by avoidscorn

Attachment: detectPopup.patch added

_FormWidget.js patch to detect popups

comment:1 Changed 9 years ago by avoidscorn

Oops, dijit._FormWidget should be dijit.form._FormWidget.

comment:2 Changed 9 years ago by bill

ignoreRightClick.patch seems safe, except dojo.mouseButtons.isLeft() is the portable way to test mouse buttons. (You didn't say you tested your patch on IE, I bet it doesn't work there.) You just need to add a section to Menu_mouse.html (the robot test) to automate the test you added to test_Menu.html.

I looked up the reason for the whole _onMouseDown() handler. It's from #5434 and also #6888, although it no longer seems to be necessary for getting buttons on Chrome to focus when mouseup happens outside the button. But it is still necessary for ValidationTextBox widgets to get focus when the user clicks the right section, where the validation icon is.

comment:3 Changed 9 years ago by Douglas Hays

Owner: set to Douglas Hays

Changed 9 years ago by avoidscorn

Attachment: ignoreRightClick2.patch added

Uses dojo.mouseButtons.isLeft to detect left-clicks

Changed 9 years ago by avoidscorn

Attachment: robotTestCase.patch added

tests/robot/Menu_mouse.html patch

Changed 9 years ago by avoidscorn

Attachment: testCase2.patch added

Display "dijit.form._FormWidget" instead of "dijit._FormWidget"

comment:4 Changed 9 years ago by avoidscorn

I believe the new patches address your concerns.

comment:5 Changed 9 years ago by Douglas Hays

Milestone: tbd1.6

avoidscorn, do you have a signed CLA on file with Dojo?

comment:6 Changed 9 years ago by avoidscorn

Yes, I have a CLA on file.

comment:7 Changed 9 years ago by Douglas Hays

Resolution: fixed
Status: newclosed

(In [23303]) Fixes #11836. Patch from avoidscorn (CLA). _FormWidget onMouseDown now only consumes a left mousedown. Added automated contextmenu test.

Note: See TracTickets for help on using tickets.