Opened 6 years ago

Last modified 3 years ago

#16515 assigned defect

dojo/Stateful breaks postscript() inheritance

Reported by: Peter Jekel Owned by: Kris Zyp
Priority: high Milestone: 1.15
Component: Core Version: 1.8.2
Keywords: Cc:
Blocked By: Blocking:

Description

Because each dojo module can only have one postscript() method dojo/Stateful breaks the postscript inheritance chain by not calling this.inherited(arguments).

The dojo/Stateful postscript method should read like:

    postscript: function(/*Object?*/ params){
      // Automatic setting of params during construction
      if (params){ this.set(params); }
      this.inherited(arguments);
    },

Change History (11)

comment:1 Changed 6 years ago by bill

Milestone: tbd1.9
Owner: set to bill
Status: newassigned

Indeed. I was thinking that postScript() was automatically chained, but apparently it isn't. I tested in firebug via:

var A = dojo.declare(null, {
   postscript: function(){
      console.log("A.postscript");
   }
});

var B = dojo.declare(A, {
   postscript: function(){
      console.log("B.postscript");
      this.inherited(arguments);
   }
});

new B();

Doesn't work without the this.inherited() call.

So dojo/Stateful is missing this.inherited() and so is dojox/charting/_BidiSupport and dojox.grid.cells.RowIndex (although I wouldn't worry about the last one since it's not a mixin, plus which its deprecated code).

Shouldn't the this.inherited() call come first?

In the interests of expediency I'll check this in.

comment:2 Changed 6 years ago by Peter Jekel

Executing the statement "this.inherited(arguments)" first is probably better because it will quarentee that whatever functionality one puts in the postscript() methods is executed in the same order as the chained constructors.

comment:3 Changed 6 years ago by bill

Owner: changed from bill to Kris Zyp

Agreed.

Unfortunately adding the this.inherited() call breaks the regression test, on the serialize() test. JSON.stringify(aStatefulInstance) now includes "_inherited" in the result.

I'm not sure if we should consider that a bug or not, since any non-trivial class created with dojo.declare() has an _inherited property. Kris?

comment:4 Changed 6 years ago by bill

In [30264]:

add note about why _WidgetBase.postscript() doesn't call this.inherited(), refs #16515 !strict.

comment:5 Changed 6 years ago by bill

Milestone: 1.91.10

Bumping this ticket since we are past the deadline for the 1.9RC. The fix can be put into 1.9.1 too, if desired.

comment:6 Changed 5 years ago by Kris Zyp

So would adding this.inherited() only break serialization, or would it break the widget initialization too (it sounds like that is what is implied by [30264])?

comment:7 Changed 5 years ago by bill

It just breaks serialization. It won't affect widgets because widgets will never call Stateful.postScript() anyway.

comment:8 Changed 5 years ago by Kris Zyp

Priority: undecidedhigh

comment:9 Changed 5 years ago by dylan

Milestone: 1.101.11

comment:10 Changed 4 years ago by dylan

Milestone: 1.111.12

comment:11 Changed 3 years ago by dylan

Milestone: 1.131.15

Ticket planning... move current 1.13 tickets out to 1.15 to make it easier to move tickets into the 1.13 milestone.

Note: See TracTickets for help on using tickets.