#12550 closed defect (fixed)
ContentPane: destroys contents on refresh even if href is ""
Reported by: | David M | Owned by: | bill |
---|---|---|---|
Priority: | high | Milestone: | 1.6.1 |
Component: | Dijit | Version: | 1.6.0 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description (last modified by )
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: [email protected]…
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?
Attachments (2)
Change History (12)
Changed 10 years ago by
Attachment: | contentPane_12550.patch added |
---|
comment:1 Changed 10 years ago by
Component: | General → Dijit |
---|---|
Description: | modified (diff) |
Summary: | dijit.layout.ContentPane - destroys contents on refresh even if href is "" → [regression] ContentPane: destroys contents on refresh even if href is "" |
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?
comment:2 Changed 10 years ago by
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
comment:3 Changed 10 years ago by
Milestone: | tbd → 1.6.1 |
---|
@dmiddlecamp - I meant that you should click "preferences" in the upper right, and put your email there.
comment:4 Changed 10 years ago by
Milestone: | 1.6.1 → tbd |
---|---|
Summary: | [regression] ContentPane: destroys contents on refresh even if href is "" → ContentPane: destroys contents on refresh even if href is "" |
@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?
comment:5 Changed 10 years ago by
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.
comment:6 Changed 10 years ago by
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 ""...
comment:7 Changed 10 years ago by
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.
comment:8 Changed 10 years ago by
Milestone: | tbd → 1.6.1 |
---|---|
Owner: | set to bill |
Status: | new → assigned |
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.
comment:9 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
suggested patch