Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#13369 closed defect (wontfix)

Dijit id is not set before the call of postMixInProperties

Reported by: Gu Yi, He Owned by:
Priority: low Milestone:
Component: Dijit Version: 1.6.1
Keywords: dijit id postMixinProperties Cc: Adam Peller
Blocked By: Blocking:

Description

In _WidgetBase, from line 189 to 201.

if(this.srcNodeRef && (typeof this.srcNodeRef.id == "string")){ this.id = this.srcNodeRef.id; }
		if(params){
			this.params = params;
			dojo._mixin(this, params);
		}
		this.postMixInProperties();

		// generate an id for the widget if one wasn't specified
		// (be sure to do this before buildRendering() because that function might
		// expect the id to be there.)
		if(!this.id){
			this.id = dijit.getUniqueId(this.declaredClass.replace(/\./g,"_"));
		}

If the user does not specify an id for the widget when creating it, The id is not available in the call of postMixInProperties.
Suggest to put the assignment statement before the postMixInProperties.

Change History (9)

comment:1 Changed 8 years ago by bill

Priority: highestlow
severity: criticalnormal

Hmm, well I suppose I could move the id generation to before postMixInProperties() although is there a bug with the current code?

The workaround to this "issue" is to not access the id from postMixInProperties(). If you have code that needs the id, you could put it in buildRendering().

comment:2 Changed 8 years ago by Gu Yi, He

Yep. At line 676 in dijit.Tree, It tries to set up a cookie with the id prefixed in the postMixInProperties. If no id is provided, the trees will share the same state cookie which may cause the inconsistency issue.

if(!this.cookieName){
    this.cookieName = this.id + "SaveStateCookie";
}

comment:3 Changed 8 years ago by bill

OK. The Tree/id thing was addressed in #12735. To get persistence for a Tree you need to set an explicit id, otherwise it would be using a temporary id to save Tree state and that's bad because the id could easily change. (Or a new Tree widget could receive the same id as an old Tree widget.)

Was there anything else broken?

comment:4 Changed 8 years ago by Gu Yi, He

No, at present. I can live with the Tree case. Agree with you.

comment:5 Changed 8 years ago by bill

Resolution: wontfix
Status: newclosed

OK, well this one is a toss up. There might be some reason to access the id from postMixinProperties(), but I haven't seen one so far, and changing the code to define id earlier would actually break the change from [24462]. So, I'm going to mark this as wontfix for now, but if a good reason comes up later then I'll change it.

comment:6 Changed 8 years ago by Adam Peller

Yes, I think it was #12735 we were really concerned about. Thanks for the fix.

comment:7 Changed 8 years ago by Adam Peller

Could there still be a collision problem with Trees running prior to 1.7 which had "SaveStateCookie?" recorded as a cookieName without an id?

comment:8 Changed 8 years ago by Adam Peller

sorry... I made the comment on #12735 where it belongs

comment:9 Changed 8 years ago by Adam Peller

Cc: Adam Peller added; apeller@… removed
Note: See TracTickets for help on using tickets.