Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#10652 closed defect (fixed)

[regression] focus on empty or hidden menus throws exceptions

Reported by: Adam Peller Owned by:
Priority: high Milestone: 1.4.1
Component: Dijit Version: 1.4.0
Keywords: Cc: Nathan Toone
Blocked By: Blocking:

Description

Yeah, sounds a bit odd, but it used to be a no-op before. _KeyNavContainer.focusChild used to check if(widget{...} and since r20776 it does not.

Change History (7)

comment:1 Changed 9 years ago by Adam Peller

Summary: [regression] focus on empty or hidden menus throw exceptions[regression] focus on empty or hidden menus throws exceptions

comment:2 Changed 9 years ago by bill

Cc: Nathan Toone added

This started in [19305] with the conversion of DropDownButton and ComboButton to extend _HasDropDown. After that checkin, the drop down widget's DOM node is initially disconnected from the DOM tree (from <body>), which confuses ContentPane.startup(), making it load before it's supposed to.

Specifically [19305] removed this line from DropDownButton.startup() which attaches the drop down's DOM to <body>:

dijit.popup.prepare(this.dropDown.domNode);

Not sure why that line was removed.

There are two ways to fix this regression:

  • make ContentPane.isShown() return false when ContentPane.domNode.parentNode is undefined
  • attach the drop down to <body> initially

Since the drop down is attached to <body> when it is shown, and also after it's hidden (after being shown), it seems consistent to attach it to <body> initially too. Plus, it worries me a bit to have startup() called on a widget that it isn't attached to the DOM tree, although the widget shouldn't be doing sizing since it's display:none.

comment:3 Changed 9 years ago by bill

Resolution: fixed
Status: newclosed

(In [21203]) Don't orphan the drop down widget from the DOM tree; it causes TooltipDialog to prematurely load it's href (before the widget is visible), and also possibly other obscure problems, like if the app has dojo.query() code that runs on page load, and expects to find nodes in the drop down widget.

Fixes #10652, refs #9356 (the code which prevents the orphaning was removed in [19305] but I'm not sure why), !strict.

comment:4 Changed 9 years ago by Nathan Toone

I'm not positive why I removed that either...it seems to be needed.

I would suggest adding back in

dijit.popup.prepare(this.dropDown.domNode)

comment:5 Changed 9 years ago by Nathan Toone

Or that... ;)

comment:6 Changed 9 years ago by Adam Peller

(In [21204]) on 1.4 branch: Tolerate empty menus or all hidden menus with no focusable items. Fixes #10652

comment:7 Changed 9 years ago by bill

Note: above comments from me, and [21203], were really for #10649, I got the ticket numbers confused.

Note: See TracTickets for help on using tickets.