Opened 11 years ago

Closed 11 years ago

#7650 closed defect (fixed)

Declaration: can't really create/override methods

Reported by: bill Owned by: bill
Priority: high Milestone: 1.3
Component: Dijit Version: 1.2beta
Keywords: Cc:
Blocked By: Blocking:

Description

The Declaration code lets you declare a class with methods:

<table dojoType="dijit.Declaration" widgetClass="demo.Table" ...>
	<script type="dojo/method" event="onSort" args="index">
		...
	</script>
</div>

Unfortunately methods declared this way are implemented as connections, rather than as real methods in the declared class' prototype. This leads to many issues including:

  • can't return a value
  • can't override an existing method
  • i suspect this.inherited() doesn't work

It also breaks the deferredConnect code in Widget.js that checks whether an event handler method has been redefined from a sentinel value:

for(attr in this._deferredConnects){
	if(this[attr] !== dijit._connectOnUseEventHandler){
		delete this._deferredConnects[attr];	// redefined, probably dojoAttachEvent exists
	}
}

... last issue is leading to JS errors in mail demo when clicking an item in the inbox, since an unwanted connection is getting setup from this.domNode.onclick --> this.onClick.

Change History (5)

comment:1 Changed 11 years ago by bill

Resolution: fixed
Status: newclosed

(In [15238]) Refactor Declaration code so that methods become real methods. Also do dojo/connects on prototype methods rather than using fake mixin trick.

Also updated demo.Table example to declare onClick method so that code in Widget looking for override to onClick works.

Fixes #7650 !strict

comment:2 Changed 11 years ago by Juho Manninen

Resolution: fixed
Status: closedreopened

Changeset [15238] breaks preamble - see my test case on Dijit.Declaration

comment:3 Changed 11 years ago by bill

Milestone: 1.21.2.1

Ah, yeah I see your point; looks like the current code in Declaration.js just ignores a <script type="dojo/method" ...> declaration of preamble (when it should be overriding the definition in the base class).

Probably can just remove the line

preambles = dojo.query("> script[type='dojo/method'][event='preamble']", src).orphan(),

from Declaration.js as it doesn't seem necessary to special case preamble.

comment:4 Changed 11 years ago by Adam Peller

Milestone: 1.2.11.2.2

comment:5 Changed 11 years ago by bill

Milestone: 1.2.21.3
Resolution: fixed
Status: reopenedclosed

Maine - thanks for the test case.

Fixed in [15603].

Note: See TracTickets for help on using tickets.