Opened 10 years ago

Closed 10 years ago

#10649 closed defect (fixed)

[regression] TooltipDialog: loads href content immediately (before open)

Reported by: Sam Foster Owned by: bill
Priority: high Milestone: 1.4.1
Component: Dijit Version: 1.4.0
Keywords: Cc:
Blocked By: Blocking:

Description

If you load http://archive.dojotoolkit.org/nightly/dojotoolkit/dijit/tests/test_TooltipDialog.html and dont touch any of the buttons, you'll see in firebug net tab (or whatever you prefered way of monitoring XHR request is) the last request is to getResponse.php?delay=..

The test page has one widget instance on it with a href property - which is supposed to lazy-load that url when you first open it. The same page for 1.3.2 doesnt have this behavior.

The issue seems to be that in ContentPane's startup, there's this test to determine if _onShow should be called:

if(this._isShown() || this.preload){
  this._onShow();
}

The TooltipDialog? inherits its startup and _onShown method from CP. This line seems to be the culprit:

else if("open" in this){
  return this.open;		// for TitlePane, etc.

..TTD doesnt have an "open" property on its prototype. I'd attach a patch simply adding one, but when you do, there are further bugs waiting in line: when you then try to show the TTD (e.g. by clicking on the "Test slowloading HREF TooltipDialog?" button, you get an exception:

Error undefined running custom onLoad code: this.openDropDown is not a function

.. and that I've not tracked down yet.

Change History (6)

comment:1 Changed 10 years ago by Sam Foster

Owner: set to Sam Foster
Status: newassigned

Ok I found the loading issue too, it was just a missing "this" in form/Button.js' loadDropDown, where it connects to the dropDown's onLoad. Incoming fix (to trunk)

comment:2 Changed 10 years ago by Sam Foster

Resolution: fixed
Status: assignedclosed

(In [21198]) Adding open property to TooltipDialog? so its inline with TitlePane? etc., and critically so ContentPane?'s isShown (and anything else that needs to know its an open-able widget) does the right thing. Also (bug hiding behind this one) adding missing 'this' to fix the callback in Button's loadDropDown. Fixes #10649

comment:3 Changed 10 years ago by Sam Foster

(In [21199]) Rolling back r21198, missed an update before that commit and giving the fix more thought. Refs #10649

comment:4 Changed 10 years ago by Sam Foster

Resolution: fixed
Status: closedreopened

The missing 'this' in Button.js is tracked and fixed in #10650. The main _isShown issue described here is re-opened.

comment:5 Changed 10 years ago by bill

Milestone: tbd1.4.1
Owner: changed from Sam Foster to bill
Status: reopenednew

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);

Nathan isn't sure why it was removed though and I don't see a reason either.

There are two ways to fix this regression:

  • make ContentPane.isShown() return false when ContentPane.domNode.parentNode is undefined (as documented above)
  • 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 a widget not attached to the DOM tree on startup(), might be unforeseen issues.

comment:6 Changed 10 years ago by bill

Resolution: fixed
Status: newclosed
Summary: REGRESSION: TooltipDialog loads href content immediately (before open)[regression] TooltipDialog: loads href content immediately (before open)

(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 #10649 (but wrote the wrong number in the checkin comment!), and refs #9356 (the code which prevents the orphaning was removed in [19305] but I'm not sure why), !strict.

Note: See TracTickets for help on using tickets.