Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ios): TableView "sectionCount" property crash #12193

Merged
merged 3 commits into from
Oct 21, 2020

Conversation

jquick-axway
Copy link
Contributor

@jquick-axway jquick-axway commented Oct 20, 2020

JIRA:
https://jira.appcelerator.org/browse/TIMOB-13903

Unit Test Note:
RecylerView PR #12029 is making heavy changes to the ti.ui.tableview.test.js script. In order to avoid a huge amount of conflicts, we should re-enable all iosBroken unit tests via that PR instead of this one.
(I've already verified this PR's change fixes the unit tests.)

Test:

  1. Build and run the below on iOS.
  2. Verify an alert displays "sectionCount: 2" on app startup.
var section1 = Ti.UI.createTableViewSection({ headerTitle: "Colors" });
section1.add(Ti.UI.createTableViewRow({ title: "Red" }));
section1.add(Ti.UI.createTableViewRow({ title: "Green" }));
section1.add(Ti.UI.createTableViewRow({ title: "Blue" }));

var section2 = Ti.UI.createTableViewSection({ headerTitle: "Fruit" });
section2.add(Ti.UI.createTableViewRow({ title: "Apples" }));
section2.add(Ti.UI.createTableViewRow({ title: "Bananas" }));
section2.add(Ti.UI.createTableViewRow({ title: "Peaches" }));

var window = Ti.UI.createWindow();
var tableView = Ti.UI.createTableView({
	data: [section1, section2],
});
window.add(tableView);
window.addEventListener("open", function() {
	alert("sectionCount: " + tableView.sectionCount);
});
window.open();

@build
Copy link
Contributor

build commented Oct 20, 2020

Messages
📖

💾 Here's the generated SDK zipfile.

📖 ✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖

✅ All tests are passing
Nice one! All 10091 tests are passing.
(There are 975 skipped tests not included in that total)

Generated by 🚫 dangerJS against 5af24ef

{ //TODO: Shouldn't this be in the main thread, too?
return [sections count];
return NUMUINTEGER((sections != nil) ? sections.count : 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the only required change, and the rest of the lines/changes are not necessary. Why not just leave this method as returning NSUInteger as before and avoid the need to explicitly call [NSNumber unsignedIntegerValue] to get one in the above changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't crashing because sections was nil.
(That's just a safety mechanism I added just in case.)

It was crashing because the calling code here expected the property's return type to be an NSObject derived type, but it received an NSUInteger which is a typedef'ed unsigned int. That's why I changed it to NSNumber, which is the same return type our TiUIListViewProxy uses for sectionCount here.

@ssaddique
Copy link
Contributor

FR: Pass

Test Environment
SDK Ver: this PR
OS Ver: 10.15.6
Xcode Ver: Xcode 12.0.1
Appc NPM: 5.0.0
Appc CLI: 8.1.1
Ti CLI Ver: 5.2.5
Alloy Ver: 1.15.2
Node Ver: 10.19.0
NPM Ver: 6.13.6
Emulator: iPhone 8 (13.5), iPhone 11 Pro (14.0)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants