Opened 10 years ago

Closed 10 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 10 years ago.
14361.patch (1.5 KB) - added by Damien Mandrioli 10 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 10 years ago by mpcarl

Attachment: list.html added

comment:1 Changed 10 years ago by bill

Milestone: tbd

Changed 10 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 10 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 10 years ago by cjolif

Milestone: tbd1.8

comment:4 Changed 10 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 10 years ago by ykami

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

comment:6 in reply to:  4 Changed 10 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 10 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 10 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 10 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.