Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#16948 closed defect (fixed)

[regression] _this.domNode undefined in : dojox/mobile/scrollable.js

Reported by: mc007ibi Owned by: Adrian Vasiliu
Priority: undecided Milestone: 1.9
Component: DojoX Mobile Version: 1.9.0b1
Keywords: Cc:
Blocked By: Blocking:

Description

I discovered this by switching from stable 1.8 to trunk. Its crashing in line 208 :

win.global.addEventListener("scroll", function(e){

if(_this.domNode.style.display === 'none'){ return; }

Please a simple sanity check there. Thank you.

Attachments (1)

patch16948.patch (1.3 KB) - added by Adrian Vasiliu 6 years ago.
Fixes regression in some cases of DOM destruction - Adrian Vasiliu (IBM, CCLA)

Download all attachments as: .zip

Change History (8)

comment:1 Changed 7 years ago by Eric Durocher

Owner: changed from Eric Durocher to Adrian Vasiliu
Status: newassigned

comment:2 Changed 7 years ago by Adrian Vasiliu

The piece of code your refer to is from scrollable's init() method, which documentation states "At least domNode and containerNode have to be given.". Also, before the line of code you refer to, there are a couple of other "unprotected" usages of this.domNode inside this init(). Hence, to be sure we do the right fix, it would be helpful if you could provide information about the way you use scrollable.js, ideally a test case allowing to reproduce.

comment:3 Changed 7 years ago by Adrian Vasiliu

@mc007ibi, we are still waiting for information about how to reproduce.

comment:4 Changed 6 years ago by Adrian Vasiliu

In the meantime a case when this hurts has been found: dojox/mobile/tests/test_StoreCarousel-resize.html with the following scenario:

launch for instance on iPad, select the last item of the carousel, then change the orientation (landscape/portrait).

The attached patch fixes it by adding the "sanity check" plus the removal of the listener in scrollable.cleanup() (which is called by _ScrollableMixin.destroy()).

Changed 6 years ago by Adrian Vasiliu

Attachment: patch16948.patch added

Fixes regression in some cases of DOM destruction - Adrian Vasiliu (IBM, CCLA)

comment:5 Changed 6 years ago by cjolif

Summary: _this.domNode undefined in : dojox/mobile/scrollable.js[regression] _this.domNode undefined in : dojox/mobile/scrollable.js

comment:6 Changed 6 years ago by Eric Durocher

Resolution: fixed
Status: assignedclosed

In [31334]:

fixes #16948, refs #16363. Fix regression caused by "scroll" listener added to fix #16363, and which was not removed on view destroys. Thanks Adrian Vasiliu (IBM, CCLA). !strict

comment:7 Changed 6 years ago by Eric Durocher

Milestone: tbd1.9
Note: See TracTickets for help on using tickets.