Opened 11 years ago

Closed 10 years ago

Last modified 9 years ago

#5528 closed defect (fixed)

StackContainer: nested StackContainers always preload first child

Reported by: tk Owned by: bill
Priority: high Milestone: 1.4
Component: Dijit Version: 1.0
Keywords: StackContainer TabContainer preload Cc: shawn@…
Blocked By: Blocking:

Description (last modified by bill)

IF you have nested stackContained based widgets with children set to preload=false and display: none; the first child widget is always preloaded regardless of if its parent is actually displayed or not.

Attached is a small patch that works with StackContainer? based (SplitContainer? would need a seperate patch with this method) widgets, but maybe this patch will drive an idea for a more centerally located patch that will fix all Container widgets.

Attachments (1)

sc.diff (1.1 KB) - added by tk 11 years ago.
allows nested stackContainers to not preload firstChild until the parent is selected.

Download all attachments as: .zip

Change History (17)

Changed 11 years ago by tk

Attachment: sc.diff added

allows nested stackContainers to not preload firstChild until the parent is selected.

comment:1 Changed 11 years ago by bill

Milestone: 1.2
Owner: set to bill

Sounds like the testcase is something like:

<div dojoType="dijit.layout.StackContainer">
    <div dojoType="dijit.layout.ContentPane">  selected tab </div>
    <div dojoType="dijit.layout.StackContainer id="unselectedChildStackContainer">
          <div dojoType="dijit.layout.ContentPane" href=...>...</div>
    </div>
</div>

(could replace StackContainer? with TabContainer? in the above example and would be same issue)

I guess there's an architectural problem.

On page load, startup() is called on every widget. In this case, unselectedChildStackContainer.startup() calls unselectedChildStackContainer._showChild() which calls ContentPane?._loadCheck(), thus causing a premature load. Clearly, the child StackContainer? is doing stuff on startup that it shouldn't be doing until it's shown.

We already have code to solve a similar problem. widgetX.resize() isn't called until widgetX is shown. This prevents the problem where we try to do JS sizing on a hidden div. One might think to defer the StackContainer?._showChild() call until StackContainer?.resize() is called, but things are complicated because StackContainer? has both doLayout=true and doLayout=false options, and resize() isn't called when doLayout=false.

I guess StackContainer? needs an onShow() method too, and then *if* the StackContainer? is hidden on initialization, it won't try to show any child until onShow() is called. See ContentPane?._loadCheck() for example of testing whether or a widget is hidden.

Marking for 1.2 for now but could be fixed either sooner or later depending on the complexity of the code change.

comment:2 Changed 11 years ago by tk

That sounds fairly good. Including a more complete test case.

<div dojoType="dijit.layout.StackContainer">
    <div dojoType="dijit.layout.ContentPane">  selected tab </div>
    <div dojoType="dijit.layout.StackContainer id="unselectedChildStackContainer">
          <div dojoType="dijit.layout.ContentPane" preLoad="false" style="display: none;" href=...>..... content that *must* not preload but does anyways .....</div>
          <div dojoType="dijit.layout.ContentPane" preLoad="false" style="display: none;" href=...>..... content that *must* not preload and does not .....</div>
    </div>
    <div dojoType="dijit.layout.ContentPane" preLoad="false" style="display: none;" href=...>..... content that *must* not preload and doesnt .....</div>
</div>

comment:3 Changed 11 years ago by guest

This bug is of particular interest to me as I have an application that hides all the sub tabs of a TabContainer? until the first tab is completed accurately. The subtabs require certain data in the database and the http session. Since the data is not there, this bug loads those screens and cause my server to throw 500 errors. I tried the patch above, but it doesn't seem to work (Dojo 1.0.2, FF 2.0.12, and Windows Vista). In order to get a "quick fix" going, I did the following in the _showChild method of StackContainer?.js and it worked for me:

_showChild: function(/*Widget*/ page){
		if(page.declaredClass=="dijit.layout.ContentPane" && !page.preload && !this._started){
			return;
		}
		....current dojo code.....
}

This fix will only work for ContentPane? children due to the declaredClass name being hardcoded. My code uses a TabContainer? and so your mileage may vary. I have a TabContainer? nested within a LayoutContainer?. The very first ContentPane? in my TabContainer? is loaded properly because the layout method gets called on it. The layout method then calls _showChild once more, but this second time around "this.started" will be true causing the function not to return. Hope this helps people looking for a "quick fix". Again mileage may vary.

comment:4 Changed 11 years ago by tk

Note: the patch attached is only for TabContainer?->TabContainer? or StackContainer?->StackContainer? derivatives....

You should be fine doing TabContainer?->child1 (loads right away) TabContainer?->childNot1->preLoad=false;

That part of TabContainers? works fine.... however, you MUST set ContentPane?.domNode.style.display="none" for preLoad=false to work... (as is documented)

comment:5 Changed 11 years ago by guest

tk:

I still believe there is a bug here with nested Containers:

The following nested TabContainer? fails to recognize the preload=false and style="display:none" combination, despite not being visible. So this following example loads subTab1.

		<div dojoType="dijit.layout.LayoutContainer" id="main" style="height:100%;width:100%;">
			<div dojoType="dijit.layout.TabContainer" id="tabs" layoutAlign="client">
				<div dojoType="dijit.layout.ContentPane" title="Tab1" 
					extractContent="true" preventCache="true" href="http://localhost:8080/asomepage.html">
				</div>
				<div id="tabDoesntLoad" dojoType="dijit.layout.ContentPane" title="Tab2"
					extractContent="true" preventCache="true" href="http://localhost:8080/asomepage.html">
				</div>
				<div dojoType="dijit.layout.ContentPane" title="Tab w SubTabs" >
					<div dojoType="dijit.layout.TabContainer" id="subtabs" style="margin-top:20px;" >
						<div id="subTab1" dojoType="dijit.layout.ContentPane" title="Pre-Loads And Should Not" preload="false" style="display:none;"
							extractContent="true" preventCache="true" href="http://localhost:8080/asomepage.html">
						</div>
						<div dojoType="dijit.layout.ContentPane" title="subtabDoesntLoadAsExpected"
							extractContent="true" preventCache="true" href="http://localhost:8080/asomepage.html">
						</div>
					</div>
				</div>
			</div>
		</div>

The above is likely because the nested TabContainer? isn't checking to see if it's parent widget container is hidden.

I am sorry if I am missing something and cluttering this ticket, but my solution fixed the above issues for me. Thanks.

comment:6 Changed 11 years ago by bill

Description: modified (diff)
Milestone: 1.21.3
Summary: Nested StackContainers always preload first childStackContainer: nested StackContainers always preload first child

Punting this since I want to fix it w/an architectural change:

  • move _isShown() from ContentPane to LayoutWidget
  • make _onShow() method in !_LayoutWidget similar to ContentPane's _loadCheck() that is called whenever a layout widget is shown.
  • factor out startup() code into !_onFirstShow() or some such method, that's called from startup() if _isShown() is true, and otherwise is called from _onShow().

Note that TitlePane has an _onShow method so that name would need to be changed.

PS: the above example doesn't work w/tk's code because you have an intermediate ContentPane. If you want it to work then the getParent() call would need to go up another level in this case.

comment:7 Changed 11 years ago by bill

Milestone: 1.32.0

comment:8 Changed 10 years ago by bill

(In [15979]) test case, refs #5528

comment:9 Changed 10 years ago by bill

(In [15981]) Automated tests for AccordionContainer?, refs #5528, #7553.

comment:10 Changed 10 years ago by bill

Milestone: 2.01.4
Status: newassigned

I'm about to checkin a fix for this along with #9794.

comment:11 Changed 10 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [20000]) Make nested ContentPanes defer loading of href and resizing of child widgets until the ContentPane is shown. Specifically addressing complicated cases like nested TabContainers or TabContainer --> BorderContainer --> ContentPane. When ContentPane is a child of a layout container it's hard to efficiently determine whether it's shown or not, but we know that resize() will be called the first time it's shown (perhaps on page load), so if resize() _hasn't_ been called then defer loading of href and resizing of child widgets.

comment:12 Changed 10 years ago by Nathan Toone

(In [20007]) Refs #5528 #9794 - changelist r20000 means that resize does not get called for already-selected (default) values - so we need to ensure that _transition is called for these items

comment:13 Changed 10 years ago by Nathan Toone

(In [20009]) Refs #5528, #9794 - If we have an open value, we should use that if we have been resized - so that we honor at least that flag, since it is fast to check !strict

comment:14 Changed 10 years ago by bill

(In [20099]) Various TabContainer? href-loading and children resizing fixes, centered around ContentPane?'s role as a layout widget:

  • Fix doLayout=false TabContainer? (broken by recent checkins for #5528, #9794)
  • Fix problem where resizing the TabContainer? would refresh a pane with refreshOnShow=true. It's only supposed to refresh when the pane is hidden then shown again. (fixes #8969 !strict)

Differentiated between onShow(), which indicates that the ContentPane? is being shown, and resize(), which, when the ContentPane? is in a layout widget hierarchy, occurs both when the ContentPane? is shown but also when it changes size.

It's complicated because a ContentPane? being made visible is indicated in different ways depending on the situation:

Refs #5528, #9794 !strict

Also added a bunch of tests including one for a TitlePane? with a single layout child (refs #8197).

comment:15 Changed 10 years ago by bill

(In [20905]) Update comments for isContainer and isLayoutContainer flags for how they are currently being used (specifically since ContentPane defines them now, even though it's not a typical layout widget).

Refs #5528, #9794 !strict

comment:16 Changed 9 years ago by bill

(In [20926]) Fix comment for getParent(), refs #5528, #9794 !strict

Note: See TracTickets for help on using tickets.