ContentPane: destroys contents on refresh even if href is ""

[David M]

Browser type: FF, IE, Chrome Browser version: FF4, IE7, Chrome 10 OS and OS Version: Windows XP, all All steps necessary to reproduce the issue:

Put anything in a dijit ContentPane, cause the pane to be refreshed. If you have a dockable FloatingPane, a refresh will be triggered when it's minimized/restored. Also see attached test.html for example to reproduce.

An email address of the person filing the bug or a way to get a hold of him/her: dmiddlecamp@…

Notes:

Previous functionality allowed the creation of a dijit ContentPane with any nodes / etc inside it. The Refresh function should be smart enough not to attempt an XHR request if HREF is "". This causes errors in Firefox 4, and IE 7, because href is "", which is an "invalid argument" in IE7, and causes the content to be replaced with "Loading..." in all browsers. Could we add a simple

if (this._href !== "") { return; }

or something in the Refresh function in ContentPane? to avoid this?

[David M]

suggested patch

[bill]

Thanks, BTW you can and should register you email in trac, to get notifications.

Sounds like you are saying this is a regression from 1.5?

[David M]

sorry about that, tried to add my email after the fact, but I can't seem to modify these. Yes, this is a dijit regression from 1.5

[bill]

@dmiddlecamp - I meant that you should click "preferences" in the upper right, and put your email there.

[bill]

@dmiddlecamp - I tried your test case. It doesn't actually work, since the parser isn't getting called, but after fixing that, I see the failure. However, you shouldn't be calling refresh() on a ContentPane without an href. () So I tried dojox/layout/tests/test_FloatingPane.html. You said that the refresh() is triggered when a dockable pane is minimized. But I tried minimizing/restoring the "un-closable, dockable" pane and the _load() method is never called.

Also, I tried this against 1.5/ and 1.4/, and those also fail when calling refresh() on a pane w/no href. So this doesn't seem like a regression, although you said that "previous functionality allowed...". Which version were you talking about?

So, can you show a test case where refresh() is getting called when it shouldn't be?

[David M]

Basically the call stack is:

*User clicks DockNode? -->dojox.layout._DockNode.restore

-->if(!this.paneRef.isLoaded){ this.paneRef.refresh(); }

-->dijit.layout.ContentPane?.refresh

I'll revise my example so this is more obvious. This is a major regression from dojo 1.5 to dojo 1.6 for my app. This only started after we upgraded to 1.6, maybe something just isn't setting 'isLoaded' like it used to. Regardless, ContentPane? is supposed to take in dom nodes when built, and probably shouldn't destroy its content if href isn't set.

[David M]

still working on example, but this is from ContentPane?.js:

line 237, in _setContentAttr:

clear href so we can't run refresh and clear content refresh should only work if we downloaded the content this._set("href", "");

seems to imply to me that the ContentPane? should have a guard in the refresh function preventing it from destroying content if href is not set or ""...

[David M]

example to reproduce 12550

[David M]

I added a newer version of test.html that reproduces this bug, I had trouble getting it to work properly in cross domain mode (all in one file), but it works normally if you have a build locally. Just minimize and restore the floatingpane, and you'll see the issue.

[bill]

OK thanks, I see it, thanks for the test case.

Turns out ContentPane does have guard code, here:

if(this.href){
	if(!this._xhrDfd && // if there's an href that isn't already being loaded
		(!this.isLoaded || this._hrefChanged || this.refreshOnShow)
	){
		return this.refresh();	// If child has an href, promise that fires when the load is complete
	}
}

It's just outside of the refresh() method. The FloatingPane code should be calling _onShow(), not calling refresh() directly. I'll make that change.

If we run into other code that's calling refresh() even though there's no href, I'll add the guard code into refresh() as you suggested.

[bill]

(In [24214]) Fix problem when refresh() is called even when there's no href. Fixes #12550 on 1.6/ branch, !strict.

[bill]

(In [24215]) Fix problem when refresh() is called even when there's no href. Fixes #12550 on trunk, !strict.