Opened 12 years ago

Closed 5 years ago

#3905 closed defect (fixed)

Patch dojo.parser, OR stop putting widget code in event callbacks such as onShow, onRezised etc

Reported by: mumme Owned by: bill
Priority: low Milestone: 2.0
Component: Dijit Version: 0.9
Keywords: parser, markup, dijit events Cc:
Blocked By: Blocking:

Description (last modified by bill)

Connecting to onShow in markup using TitlePane?, StackContainer? TabContainer? etc breaks href load.

This is just a symptom of a bigger problem. Say a designer wants to know when a TitlePane? shows

<div dojoType='dijit.TitlePane'  
    href='somepage'
    open='false'
    title='Foo'
    onShow='console.debug("opened")'
></div>

When dojo.parser parses this markup it replaces TitlePane?'s onShow function, which currently triggers the href download, with:

function(){console.debug("opened")}

So we either patch the parser to connect instead of replace OR stop using public functions in widget to widget calls (not sure that is possible in all cases).

I suspect it has been discussed extensively already, still code like this is still being added to dijit trunk. I searched trac but couldn't find any tickets filed about this yet. So I figured I open a ticket to make people aware of this.

/ Fredrik

Change History (13)

comment:1 Changed 12 years ago by bill

Owner: changed from bill to mumme

Right, doing onShow=... in the parser does a replace rather than a connect by design, so that you can define a function that returns a value, like

   <div dojoType=Graph f="return arguments[0]+1">

or something like that.

So we need to change onShow to _onShow and make onShow empty, like:

  onShow: function(){ // summary: user defined callback to ... }
  _onShow: function(){  do stuff and then call this.onShow(); }

Can you do this?

comment:2 Changed 12 years ago by mumme

Owner: changed from mumme to bill

Ok thanks for clearing that out, I suppose it's a question of what people use most, functions as events, or functions as mutators.

Yes I can for this particular case, it's the reason I only had _loadCheck in the first place in ContentPane?.

But I wont do Menu.js: onCancel, onItemHover, onItemUnHover, onItemClick, onOpen, onClose

nor Button.js onClick(2 places)

nor ValidationTextbox?.js onfocus, onblur, onkeyup (lowercased why??)

nor FilteringSelect?.js onkeyup (lowercased ??)

nor TimeTextbox? onfocus (lowercased ??)

nor Slider.js onMouseMove

nor ComboBox?.js onkeypress, onfocus, onblur, onmouseover, onmouseout (lowercased ??), onClose, onClick

nor Checkbox.js onClick

nor Textbox.js onfocus (lowercased ??)

nor Dialog.js onLoad, onOpen

These are just a quick scan, this problem is bigger than just changing 1 file, IMHO all widget creators must be aware of this and code accordingly.

I'm assigning this back to you Bill, you probably know how to assign issues in the files above.

/ Fredrik

comment:3 Changed 12 years ago by bill

Milestone: 0.91.1
  • Most of those functions should have _ in front of them (like Menu.js).
  • Button.js is not an issue because it's ok to override that function.
  • ValidationTextBox?, FilteringSelect?, TimeTextbox? is using syntax like dojoAttachEvent="onfocus" rather than dojoAttachEvent="onfocus:_onFocus" but that makes the functions seem public so we should probably change it

and so forth. Will fix for 1.1

comment:4 Changed 12 years ago by mumme

(In [9876]) Dont use public event methods in widget to widget infrastructure, public events like onShow might be overridden from markup by dojo.parser.

refs #3905

comment:5 Changed 12 years ago by Adam Peller

Bill, I wonder if using attributemap (#3058) cascading the parsed handler with the template handler, the way we do with class and style attributes, might help here.

comment:6 Changed 12 years ago by bill

Priority: highnormal
severity: criticalminor

comment:7 Changed 12 years ago by bill

PS: I marked this as low priority because the user can now easily connect to a method rather than overriding it by doing a <script type="dojo/connect" event="onShow"> ...

comment:8 Changed 12 years ago by bill

Milestone: 1.12.0

comment:9 Changed 12 years ago by alex

Milestone: 2.01.3

Milestone 2.0 deleted

comment:10 Changed 11 years ago by bill

Description: modified (diff)
Milestone: 1.32.0

comment:11 Changed 9 years ago by bill

The issue is that sometimes the specified function is intended to overwrite, rather than connecting. The two examples I've found are dijit.form.Form (onSubmit), and dijit.form.Button (onClick). In both cases the function returning false will cancel the form submit.

comment:12 Changed 7 years ago by bill

Priority: highlow

comment:13 Changed 5 years ago by bill

Resolution: fixed
Status: newclosed

This is effectively fixed in delite/deliteful, since events like show/hide are emitted as actual events, and there aren't callback methods. So, closing this as fixed in 2.0.

Note: See TracTickets for help on using tickets.