Opened 10 years ago

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

Download all attachments as: .zip

Change History (13)

Changed 10 years ago by avoidscorn

Attachment: testCase.patch added

test_Menu.html patch

Changed 10 years ago by avoidscorn

Attachment: ignoreRightClick.patch added

_FormWidget.js patch to ignore right clicks

Changed 10 years ago by avoidscorn

Attachment: detectPopup.patch added

_FormWidget.js patch to detect popups

comment:1 Changed 10 years ago by avoidscorn

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

comment:2 Changed 10 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 10 years ago by Douglas Hays

Owner: set to Douglas Hays

Changed 10 years ago by avoidscorn

Attachment: ignoreRightClick2.patch added

Uses dojo.mouseButtons.isLeft to detect left-clicks

Changed 10 years ago by avoidscorn

Attachment: robotTestCase.patch added

tests/robot/Menu_mouse.html patch

Changed 10 years ago by avoidscorn

Attachment: testCase2.patch added

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

comment:4 Changed 10 years ago by avoidscorn

I believe the new patches address your concerns.

comment:5 Changed 10 years ago by Douglas Hays

Milestone: tbd1.6

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

comment:6 Changed 10 years ago by avoidscorn

Yes, I have a CLA on file.

comment:7 Changed 10 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.