Titanium JIRA Archive
Titanium SDK/CLI (TIMOB)

[TIMOB-20389] iOS: Listview delete event sends wrong itemId

GitHub Issuen/a
TypeBug
PriorityCritical
StatusClosed
ResolutionFixed
Resolution Date2016-02-15T05:31:22.000+0000
Affected Version/sn/a
Fix Version/sRelease 5.2.0
ComponentsiOS
Labelslistview, regression
ReporterMartijn Kooij
AssigneeHans Knöchel
Created2016-02-07T20:31:40.000+0000
Updated2016-02-17T22:56:46.000+0000

Description

In TiUIListView.m code has been refactored a couple of months ago (?) to fire edit events in 1 place. For delete this is causing quite a big problem because now the order of events has changed. TiUIListView.m, around line 1045 - (void)tableView:(UITableView *)tableView commitEditingStyle:(UITableViewCellEditingStyle)editingStyle forRowAtIndexPath:(NSIndexPath *)indexPath Before the change: - A reference to the deleted item was stored - The actual delete was performed on the listview - The event for the JS client was build and sent using the previously stored itemId After the change: - The item is actually deleted from the listview. - The fireEditEventWithName method is called to trigger the JS event. The problem: The fireEditEventWithName gets a reference to the item (and thus the itemId) at a section index and itemindex, but since the item was already deleted it gets the wrong or no item. If my findings are correct you have created quite a big invisible bug deleting wrong items all over the world...
//FirstView Component Constructor
function FirstView() {
	//create object instance, a parasitic subclass of Observable
	var self = Ti.UI.createView();

	var listView = Ti.UI.createListView({
		editing: true,
		allowsSelection: true
	});
	var sections = [];
	
	var fruitSection = Ti.UI.createListSection({ headerTitle: 'Fruits'});
	var fruitDataSet = [
	    {properties: { itemId: "apple", title: 'Apple', canEdit: true}},
	    {properties: { itemId: "banana", title: 'Banana', canEdit: true}},
	];
	fruitSection.setItems(fruitDataSet);
	sections.push(fruitSection);
	
	var vegSection = Ti.UI.createListSection({ headerTitle: 'Vegetables'});
	var vegDataSet = [
	    {properties: { itemId: "carrots", title: 'Carrots', canEdit: true}},
	    {properties: { itemId: "potatoes", title: 'Potatoes', canEdit: true}},
	];
	vegSection.setItems(vegDataSet);
	sections.push(vegSection);
	
	listView.sections = sections;
	self.add(listView);
	
	var fishSection = Ti.UI.createListSection({ headerTitle: 'Fish'});
	var fishDataSet = [
	    {properties: { itemId: "cod", title: 'Cod', canEdit: true}},
	    {properties: { itemId: "haddock", title: 'Haddock', canEdit: true}},
	];
	fishSection.setItems(fishDataSet);
	listView.appendSection(fishSection);
	
	listView.addEventListener("delete", function(e) {
		alert(e.itemId);
	});

	return self;
}

module.exports = FirstView;

Attachments

FileDateSize
Screen Shot 2016-02-11 at 3.12.50 PM.png2016-02-11T09:15:03.000+000034336
Screen Shot 2016-02-11 at 3.13.05 PM.png2016-02-11T09:15:10.000+000036650
Screen Shot 2016-02-11 at 3.13.36 PM.png2016-02-11T09:15:18.000+000035518
Screen Shot 2016-02-11 at 3.13.50 PM.png2016-02-11T09:15:25.000+000038800

Comments

  1. Jebun Naher 2016-02-09

    Hello, Thanks for sharing your findings. It would be much helpful if you can attach any test code that shows this behavior. Thanks.
  2. Martijn Kooij 2016-02-10

    Added a JS sample. Just delete any item and the alert will either show no item Id at all, or even worse the wrong item Id. IMHO certain bugs are worth looking into without waiting for a user sample... Not sure how many users have implemented this of course, but seems pretty critical to me. I am just "lucky" that I had not updated my app in a while.
  3. Sharif AbuDarda 2016-02-11

    Hello, I tried your sample code for test, I did not found the problem. Deleting an item shows the alert of right item id. See the attachments. First screen shot shows the listview. Second one shows the alert 'apple' when I delete item 'apple'. Third one shows the alert 'carrots' when I delete item 'carrots'. And the last one shows 'haddock' when I delete 'haddock' item. I tested with SDK 5.1.2.GA. Also IMHO, It is always better to provide sample code of an issue. It helps clearly understanding the customer problem which leads to faster customer support. Thanks.
  4. Martijn Kooij 2016-02-11

    Terribly sorry, forgot to mention that I was working using bleeding edge... Appears to have been introduced in the 5.2.x branche. The change was introduced in the commit for "[TIMOB-7735] Support UITableView insert style" on Oct. 27 https://github.com/appcelerator/titanium_mobile/commit/567e29a54280a21c4504b2f29ec718b592835436
  5. Martijn Kooij 2016-02-11

    Since 5.2.x has not yet been released this of course isn't as huge an issue as I stated earlier... It's quite a terrible and invisible bug (no unit tests on this?), but it does not yet apply to live client applications (except for people like me who tend to prefer nightly builds...)
  6. Sharif AbuDarda 2016-02-11

    Please update to latest SDK 5.1.2.GA. Closing this issue. Thanks.
  7. Martijn Kooij 2016-02-11

    NOOOOO! Don't close it! I mean... The bug causing code has been introduced almost 4 months ago an no one noticed it. Not sure if you have placed the issue on a different list, but it needs to be fixed before you release 5.2.x right???
  8. Sharif AbuDarda 2016-02-11

    The issue is not occurring in SDK 5.1.2.GA. Update your SDK 5.1.2.GA. Download [link](http://builds.appcelerator.com.s3.amazonaws.com/index.html#5_1_X). Also 5.2.x beta is available.
  9. Martijn Kooij 2016-02-11

    1. I understand that this is not in the 5.1.2. 2. I am worried that if you close this issue, no-one will fix it before 5.2.x is released. 5.2.x DOES contain this issue.
  10. Sharif AbuDarda 2016-02-11

    Hello, I have tested this in SDK 5.2.0.v20160114021251. The issue occurs. Moving the ticket for father processing. Thanks.
  11. Hans Knöchel 2016-02-12

    Hi [~martijnkooij], I'm very thankful for you to discover this issue! It' definitely critical and your code analysis is correct. The problem is, that we now received the itemId after the cell was deleted so it's the wrong one. I'll setup a fix right now and we will patch the 5.2.x asap!
  12. Hans Knöchel 2016-02-12

    PR (master): https://github.com/appcelerator/titanium_mobile/pull/7713 PR (5_2_X): https://github.com/appcelerator/titanium_mobile/pull/7714
  13. Eric Wieber 2016-02-17

    Verified fixed, using: MacOS 10.11.3 (15D21) Studio 4.5.0.201602170821 Ti SDK 5.2.0.v20160216202526 Appc NPM 4.2.3-2 Appc CLI 5.2.0-265 Alloy 1.7.33 Xcode 7.2 (7C68) Deleting an item from a listview returns the correct itemid. Tested using the provided code.

JSON Source