Opened 8 years ago

Closed 8 years ago

#14361 closed defect (invalid)

Uncaught TypeError when using mobile ListItem

Reported by: mpcarl Owned by: ykami
Priority: high Milestone: 1.8
Component: DojoX Mobile Version: 1.7.0
Keywords: Cc:
Blocked By: Blocking:

Description

I am getting the following error when using mobile ListItems? and a RoundRectList?.

Uncaught TypeError?: Cannot read property 'className' of null dom-class.js:283 removeClass dom-class.js:283 declare.deselect ListItem?.js:242 declare.select ListItem?.js:233

I think the problem us due to the fact that I am reusing the list and programmatically removing and adding list items. This seems like a common use case when using Lists to traverse a hierarchy of data. A simple null check might be all that is needed.

A sample page is attached.

Attachments (2)

list.html (1.8 KB) - added by mpcarl 8 years ago.
14361.patch (1.5 KB) - added by Damien Mandrioli 8 years ago.
When removing a list item from its click handler, a NPE is raised. Fix: Add domNode checking - Damien Mandrioli (IBM, CCLA)

Download all attachments as: .zip

Change History (11)

Changed 8 years ago by mpcarl

Attachment: list.html added

comment:1 Changed 8 years ago by bill

Milestone: tbd

Changed 8 years ago by Damien Mandrioli

Attachment: 14361.patch added

When removing a list item from its click handler, a NPE is raised. Fix: Add domNode checking - Damien Mandrioli (IBM, CCLA)

comment:2 Changed 8 years ago by cjolif

dmandrioli, ideally if we could start to more consistently have doh tests for the bugs we fix (and and that can easily be covered by a unit testing (i.e. not strange visual issues)) it would be great to help increase the stability. Can you provide that with your patch? Thanks.

comment:3 Changed 8 years ago by cjolif

Milestone: tbd1.8

comment:4 Changed 8 years ago by ykami

Is this really a valid use case? I guess anything can happen if you destroy the widget itself from its handler. If you still want to do that way, you should be able to do like this:

item.onClick = function(e) {
    updateView(other);
    return false;
};

Adding unnecessary error check code could increase the code size.

comment:5 Changed 8 years ago by ykami

btw, sorry somehow I missed this ticket until now..

comment:6 in reply to:  4 Changed 8 years ago by Damien Mandrioli

ykami, returning false in the handler does not work. The explanation is in aspect.js. I'm not aspect.js expert, but there is probably a good reason to return not just the result of the handler but the following expression (handlerResult || undefined) which can't be false.

I agree the use case is not very common...

Thanks.

Is this really a valid use case? I guess anything can happen if you destroy the widget itself from its handler. If you still want to do that way, you should be able to do like this:

item.onClick = function(e) {
    updateView(other);
    return false;
};

Adding unnecessary error check code could increase the code size.

comment:7 Changed 8 years ago by ykami

dmandrioli, have you tried exactly as I suggested? I guess you are still connecting to the handler rather than overriding it.

comment:8 in reply to:  7 Changed 8 years ago by Damien Mandrioli

My mistake, your guess is right. No idea if it's better to add some marginal robustness with this patch. I guess the workaround is enough.

comment:9 Changed 8 years ago by ykami

Resolution: invalid
Status: newclosed

OK, then I think we can close this. I think it is an invalid usage. Even dijit widgets can be broken if you destroy a widget from its own handler. (I tested a couple of them) Let's not add unnecessary code.

Note: See TracTickets for help on using tickets.