Opened 9 years ago

Closed 9 years ago

#11653 closed defect (invalid)

LightboxNano 'node is undefined' on first click, in reset function

Reported by: Nick Fenwick Owned by: dante
Priority: high Milestone: tbd
Component: Dojox Version: 1.5
Keywords: Cc:
Blocked By: Blocking:

Description

Dojo 1.5.0.

Go to http://archive.dojotoolkit.org/nightly/dojotoolkit/dojox/image/tests/test_LightboxNano.html and click on any image. You get 'node is undefined'.

This is because _reset() is called in _load() in LightboxNano?.js, and reset calls dojo.destroy() on this._img and this._bg, when both are undefined at that point.

			d.forEach([this._img, this._bg], function(n){
				d.destroy(n);
				n = null;
			});

I'm putting a temporary hack into my local dojo:

d.forEach([this._img, this._bg], function(n){
  if (n) {
    d.destroy(n);
    n = null;
  }
});

Perhaps there's a better fix?

Change History (4)

comment:1 Changed 9 years ago by Adam Peller

Owner: changed from Adam Peller to dante

comment:2 Changed 9 years ago by Nick Fenwick

phiggins has claimed on #dojo that this bug is invalid, he doesn't experience the error. I loaded the nightly testpage to re-test, and got no error. However, on refreshing the page and trying again, I got the error. So, perhaps there is a race condition going on?

I've no time to investigate just now.

comment:3 Changed 9 years ago by Nick Fenwick

I still think this is valid. Perhaps I confused myself by leaving Firebug off while re-testing. I've given up on the idea of a race condition. There is a default 5 second delay before the large image is preloaded, but that code doesn't touch this._img or this._bg, and is nothing to do with the error I experience.

If you load that nightly test page with firebug off and click an image, no errors appear because the browser is squishing the 'undefined' error. If you load with Firebug on and click an image, I get the error every time.

This seems perfectly sensible, looking at the code.

  • onclick is dojo.connect'd to this._load(),
  • almost the first thing _load() does is call this._reset()
  • _reset() calls dojo.destroy() passing this._img and this._bg, which have never been referenced or created and are therefore null
  • dojo.destroy looks like this:
129 node = byId(node);
130 try{
131     var doc = node.ownerDocument; 

The "node.ownerDocument" fails because 'node' is null (or undefined)... it was null when it was passed into dojo.destroy().

When reset() passes this._img and this._bg, it does so by using dojo.forEach.. a quick test shows me that forEach will pass null or undefined elements to the callback, so it's going to call dojo.destroy(undefined) when they are undefined.

Can anyone confirm or deny?

comment:4 Changed 9 years ago by dante

Resolution: invalid
Status: newclosed
Note: See TracTickets for help on using tickets.