Opened 10 years ago
Closed 5 years ago
#11762 closed defect (patchwelcome)
Calling this.inherited (arguments) on the Expando results in currentSize not defined message.
Reported by: | spidey2099 | Owned by: | dante |
---|---|---|---|
Priority: | high | Milestone: | 1.13 |
Component: | DojoX Layout | Version: | 1.5 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description
There is a use case in my application where a resize is called on an expando. When this occurs - an error message to the effect that currentSize is not defined occurs.
Looking at the code - it was found that the resize method has changed it's arguments!!! So now there is a second argument currentSize which must be provided.
And if this argument is not provided, the expando is not created completely and things like splitters become detached from it and the page in which the expando exists cannot be built.
Attachments (1)
Change History (12)
comment:1 Changed 10 years ago by
comment:2 follow-up: 4 Changed 10 years ago by
Component: | General → Dojox |
---|---|
Owner: | changed from anonymous to dante |
are we talking about ExpandoPane? or the base classes? Aren't those arguments marked as optional?
comment:3 Changed 10 years ago by
Component: | Dojox → DojoX Layout |
---|
comment:4 Changed 10 years ago by
Replying to peller:
are we talking about ExpandoPane? or the base classes? Aren't those arguments marked as optional?
Not as far as I can tell. Anyway here is the resize method from dojo/dojox/layout/ExpandoPane.js
*
resize: function(/* Object? */newSize, /*Object?*/ currentSize){
summary: we aren't a layout widget, but need to act like one: newSize: Object The size object to resize to currentSize: Object The size of my domNode that has already been set (by BorderCon?
tainer)
if(!this._hasSizes){ this._startupSizes(newSize); }
compute size of container (ie, size left over after title bar) this._contentBox = {
w: newSize && "w" in newSize ? newSize.w : currentSize.w, h: (newSize && "h" in newSize ? newSize.h : currentSize.h) - this._tit
leHeight
}; dojo.style(this.containerNode, "height", this._contentBox.h + "px");
if(newSize){
dojo.marginBox(this.domNode, newSize);
}
this._layoutChildren();
},
*
comment:5 Changed 10 years ago by
Sorry for the split response. Here is the resize method from dojo/dojox/layout/ExpandoPane.js in release 1.4.2
resize: function(/* Object? */psize){ // summary: we aren't a layout widget, but need to act like one: // // psize: Object // The size object optionally passed to us by our parent. if(!this._hasSizes){ this._startupSizes(psize); } // compute size of container (ie, size left over after title bar) // it looks like two marginBox calls, but sometimes psize comes in with only one member var size = (psize && psize.h) ? psize : dojo.marginBox(this.domNode); this._contentBox = { w: size.w || dojo.marginBox(this.domNode).w, h: size.h - this._titleHeight }; dojo.style(this.containerNode, "height", this._contentBox.h + "px"); this._layoutChildren(); },
comment:6 Changed 10 years ago by
Given the ambiguity of our documentation format, it's hard to tell since it's there twice, but the "?" in Object? means it's optional. Where's the inherited call and how do you reproduce the problem?
comment:7 Changed 10 years ago by
Okay - I see what you are saying. The call I am making is: this.inherited(arguments) However, I suspect that something is adding the first arg(newSize) to the arguments and the code looks for the newSize parameter and if present it will process both the newSize and the currentSize - which in this case is undefined.
Question is: if I pass an empty arguments list to the expando, will it resize correctly?
Changed 10 years ago by
Attachment: | ExpandoPane.js added |
---|
Fix for currentSize being null when children of the expando call resize()
comment:8 Changed 10 years ago by
I have found that when children of the expando call resize, the currentSize can be undefined. The code is not defensive about this: it assumes that currentSize will always be there. I have tried passing in an empty arguments list and the method does not handle it.
So I added a defensive measure to protect against currentSize being undefined:
// compute size of container (ie, size left over after title bar) if (currentSize === undefined || currentSize === null) { currentSize = dojo.marginBox(this.domNode); }
The complete Expando.js with the change is attached.
This change works and the problem is fixed in my application. Please let me know what you think.
comment:9 Changed 10 years ago by
Since ExpandoPane is always a child of BorderContainer, it's assuming that it's always passed in a non-null currentSize parameter. I'll update the contradictory documentation. FWIW this currentSize stuff is more trouble than it's worth and will be removed in the future.
I don't understand what you wrote about about "when children of the expando call resize()". Is ExpandoPane's child a ContentPane? And the ContentPane is calling resize on it's children? And that's when the problem happens?
comment:10 Changed 10 years ago by
comment:11 Changed 5 years ago by
Milestone: | tbd → 1.12 |
---|---|
Resolution: | → patchwelcome |
Status: | new → closed |
If there's interest in making a change to this, it needs an updated pull request, see our contributing guidelines at https://github.com/dojo/dojo/blob/master/CONTRIBUTING.md
I'm marking this as patchwelcome for now, given that it's been 5 years.
P.S. Maybe I missed it but if a fundamental method's arguments change, shouldn't we wrap it with the original method and invoke the new method call from the old one. Basically we can use the wrapper pattern to be backward compatible.