[TIMOB-24349] iOS: Ti.UI.AlertDialog Not Firing Events Consistently (run-on-main-thread)
GitHub Issue | n/a |
---|---|
Type | Bug |
Priority | Critical |
Status | Closed |
Resolution | Fixed |
Resolution Date | 2018-04-12T15:44:45.000+0000 |
Affected Version/s | n/a |
Fix Version/s | Release 7.1.1 |
Components | iOS |
Labels | ios, qe-6.2.0, run-on-main-thread |
Reporter | Kelly Corn |
Assignee | Hans Knöchel |
Created | 2017-01-25T21:55:48.000+0000 |
Updated | 2018-06-19T13:55:56.000+0000 |
Description
Alert dialog boxes do not always fire an event. The problem seems exacerbated if one listener is supposed to create another dialog box -- although it has been observed with only one alert dialog. It does not seem to matter which option the user selects.
In the sample app below, the expected behavior is to have text output whenever the user selects an option for dialog A. The actual output is sometimes missing text because the event isn't always firing. Usually, after missing several events, the app completely crashes.
*index.js*
var c = 0;
function doClick(e) {
c++;
var aDialog = Ti.UI.createAlertDialog({
cancel: 0,
buttonNames: ['Cancel', 'Ok'],
message: 'A',
});
aDialog.addEventListener('click', function(e) {
if (e.index == 1) {
Ti.API.info(c+' OK');
var bDialog = Ti.UI.createAlertDialog({
cancel: 0,
buttonNames: ['Cancel', 'Ok'],
message: 'B'
});
bDialog.show();
} else {
Ti.API.info(c+' Cancel.');
}
});
aDialog.show();
}
$.index.open();
*index.xml*
<Alloy>
<Window class="container">
<Button id="label" onClick="doClick">Hello, World</Button>
</Window>
</Alloy>
*Sample Output*
[DEBUG] : Application booted in 219.254971 ms
[INFO] : 1 Cancel.
[INFO] : 2 OK
[INFO] : 3 OK
[INFO] : 4 OK
[INFO] : 5 Cancel.
[INFO] : 6 Cancel.
[INFO] : 7 Cancel.
[INFO] : 8 OK
[INFO] : 9 OK
[INFO] : 10 OK
[INFO] : 11 Cancel.
[INFO] : 12 Cancel.
[INFO] : 13 Cancel.
[INFO] : 14 Cancel.
[INFO] : 17 OK
[INFO] : 18 Cancel.
[INFO] : 19 OK
[INFO] : 29 Cancel.
Attachments
File | Date | Size |
---|---|---|
index.js | 2017-01-25T21:55:04.000+0000 | 507 |
index.xml | 2017-01-25T21:55:05.000+0000 | 116 |
system.log | 2017-01-26T17:03:19.000+0000 | 262906 |
system-2017-01-27.log | 2017-01-27T18:25:46.000+0000 | 248462 |
Hey there, tested the issue with 20+ clicks and both main-thread enabled (
<property name="run-on-main-thread" type="bool">true</property>
) and not. Both worked fine for me. The tests have been done on the iOS 10.2 Simulator.Hello, I can also test the issue. Works for me too.
Also seeing this behavior on all SDKs after 5.3.1
[~cliff_stander] So it did not happen on 5.3.0 or 5.2.x? That would help greatly! *EDIT*: Just checked the commits, I can't see any related change to the alert dialog that might have caused it. If someone can provide an exact SDK version from which it happened (and probably a more detailed test-case), we have good chances to fix it! Maybe it only happens on iOS 9 devices? Or only sims/devices? More info is crucial for this one.
correct 5.3.1 and 5.2.x work but 5.4.0, 5.5.1, 6.0.1 all have this issue. It's intermittent, sometimes working many times in a row before failing, seems to only be iOS, Android seems unaffected.
Just happened to me with the sample code using iOS 8.4 Sim (ipad 2) and again seeing it happen on ipad 2 iOS 9.3
[~cliff_stander] Did you test this on iOS 10 or later? And using main-thread on/off?
ipad air iOS 10.1 just recreated, run-on-main-thread = true
The only AlertDialog related change in 5.4.0 is TIMOB-20550 (should not be related, only auto-layout). If you could log the full error when crashing, we could track it down easily.
I have not yet seen the crash, but will send it in if I do
I have modified the code to add clicks to the counter after saying ok to "A" so we can see when "B" fails, as Kelly stated it happens more on the second listener, in a 10. ipad air sim I had it fail 3 times out of 80 clicks 2/3 were on the second level message.
Here is a system.log with a crash.
Ok, the error helps a lot! Until I am able to reproduce it, can you try to modify the
TiUIAlertDialogProxy.m
methodfireClickEventWithAction
withinstead of
Hans, I made the change, and did not see much improvement. I am attaching a second log for you with another crash.
And you applied the change to
~/Library/Application Support/Titanium/mobilesdk/osx/6.0.1.GA/iphone/Classes/TiUIAlertDialog.m
+ did a fresh build? And did it help to not use main-thread (property above to false)?And next attempt: If you comment-out the
[self cleanup];
call, does that change something? The error suggests that a proxy reference has been GC'd from memory before sending out the event and it's required to assign thesource
property but also to delegate the event through the app lifecycle to deliver it to the assigned event listener. Just a little background, not too relevant for you guys I guess. But please to comment it out and see what happens. If it's that one, I have an idea to fix it.Hans- making the first change you suggested to TiUIAlertDialog.m and setting run-on-main-thread to false seems to have fixed the issue (my previous attempt was set to true). I went through 100 dialogs with no missed events and no crashes, and also tested our own app where the issue was quite visible - all seemed much better. Do you still need input on the deletion of [self cleanup]?
Hey [~kellycorn], sorry for the delay! Unfortunately I have still not been able to reproduce it, so I have to troubleshoot you some more ideas: Can you do one last thing and replace the
fireClickEventWithAction
method with [this one](https://gist.github.com/hansemannn/5512e4534f2c3f7fd19a7c986e7c1cf0)? It will basically ensure that the click event is fired before the cleanup is initiated. I have also attached the log of 81 click-tests that all passed. Thank you![~hknoechel] I tried your solution, it crashed me sometimes and event failed as well. As [~kellycorn] has verified that event in case of Kroll Thread (run_on_main-thread = false) is working fine. I checked memory allocation of TiUIAlertViewProxy for main thread and kroll thread. Heap size decrease very frequently in case of kroll thread and decrease after long time in case of main thread. I observed that when large size of heap is getting released, in that case event got missed. So I released heap forcefully in case of main thread using -
inside -(void)show:(id)args method of TIUIAlertViewProxy.m as mentioned [here](https://gist.github.com/vijaysingh-axway/af115c27a92b63f77808126ebea40535). [~hknoechel] and [~kellycorn] can you guys please verify with your use cases.
Vijay- I verified your change to release the heap and had 100 / 100 events fire successfully! (note: I am now using SDK 6.1.1)
Thanks [~kellycorn]. PR: https://github.com/appcelerator/titanium_mobile/pull/9181
Verified as fixed, using the sample app provided, AlertDialog events fired successfully 100 / 100 times. Tested on both Simulator and Device. Tested On: iPhone 7 10.3.2 Device & Simulator Mac OS Sierra (10.12.5) Ti SDK: 6.2.0.v20170630052324 Appc NPM: 4.2.9 App CLI: 6.2.2 Xcode 8.3.3 Node v4.6.0 *Closing ticket.*
I am still experiencing this issue on SDKs 6.2.0.GA and 6.3.0.GA with
run-on-main-thread = true
on iOS 11.1 simulator. If I setrun-on-main-thread = false
then the click listener is fired twice.I am dealing with the same issue on SDKs 7.0.0.GA. Problem is fixed with run-on-main-thread=false the click listener is fired correctly. But that is no option for me. I need to run my app on main thread.
[~vijaysingh] Can you revisit this ticket and write a stress-test?
I tried with test case mentioned in this ticket and unable to reproduce the issue. [~appsol] [~dbenhenni] Can you please share your test cases? Thanks!
Still seeing this issue on 7.0.1.GA, only happens when
run-on-main-thread = true
, [~hknoechel] the weirdest thing is, there is no crash and no log for this, just silently don't fire the listener[~reymundolopez] Is this issue happening with test case mentioned in this ticket? I am not able to reproduce at my end. Can you please share your test case? Thanks!
In my case it is an optionDialog. The result is the same. Here is a quick example code. Click several times and see what happens.
Reopening ticket. [~dbenhenni] Please format your code. Also, I am wondering how the above code even compiles, as
includes a syntax error around "msg" that represents a variable without
" + msg + "
.I guess fix similar to Ti.UI.AlertDialog will fix the problem of Ti.UI.OptionDialogue also. [~reymundolopez] [~appsol] Can you guys please confirm if this issue is happening in Ti.UI.OptionDialog or Ti.UI.AlertDialog to you? And please share your test cases. Thanks!
The following pull request fixes the Ti.UI.OptionDialog to have the same behavior as the Ti.UI.AlertDialog. It has been tested with a few hundred tries and working fine. PR (master): https://github.com/appcelerator/titanium_mobile/pull/9803 PR (7_1_X): https://github.com/appcelerator/titanium_mobile/pull/9992 Test-Case 1/2 (ran 100 times)
Test-Case 2/2 (alert from background thread):
[~vijaysingh] [~hknoechel] I'm having the problem with an
Ti.UI.AlertDialog
on Ti SDK 7.0.1, I'll put a small code to reproduce it for you guys.Moving out to 7.2.0 due to missing reproducible test-cases. We are trying to prevent providing fixes for issues that cannot be reproduced.
[~reymundolopez]Ping for test case. Thanks!
[~reymundolopez] Another ping. We'd consider this for the earliest next release, but need feedback on the patch and your test-case if it does not work with the patch.
[~hknoechel] and [~vijaysingh] sorry for not getting the test case, we decided to solve it by using a custom control, I tried every suggestion posted in this ticket and nothing seems to improve it. Also, I was unable to have a simple code where I can reproduce it, it's only happening in one of our main apps (so far), also I added some logs to the SDK, and seems like the click event of the control is fired, but the function added to the listener is never fired, kinda weird.
We're seeing the same thing. Problem must have started when we moved our ios app to the main thread (we had to do that to interoperate with a critical third party library). Unfortunately for us, it's affecting our normally very polite "rate our app" prompt. Not only can the user not rate our app, but they can't permanently dismiss the prompt. It will hit them every time they run the app. :-( We are also unable to reproduce in a simple app.js. And we don't have time to tear the entire app apart -- even with a binary search, it would take days to isolate the cause. We will probably write a simple AlertDialog replacement in JS.
I have been watching this thread with interest for a while. I have the same issues on both the AlertDialog and OptionDialog. But I have never been able to produce a reliable test case either. It just happens randomly - the alert popup appears, you choose an option ... and nothing happens. So you repeat the process a second time and it fires perfectly. "Most" times it fires correctly the first time. Similar to @Reymundo I have also been considering creating a custom control to just move past this issue, but I was holding off, hoping that this ticket would be solved. I don't have any other information to add that hasn't already been shared in the above comments.
This seems to be a pretty good start on a JS module: http://www.kiteplans.info/2014/11/01/titanium-allouy-ios-android-module-custom-alert-module-to-replace-titanium-ui-alertdialog/
The thing Jason Priebe said sounds interesting. I also had to turn on the "run-on-main-thread" property after an earlier update, because a critical third party library didn't work any more.
Why is the fix for this bug getting pushed back again and again? This seems like a critical bug. It is inconsistent, so a developer might not notice that one of his dialogs isn't working deep inside the application. Also, it's a main thread issue, and main thread is increasingly more important, with Hyperloop and looking forward to TiSDK 8, which I believe will be main-thread-only.
[~jpriebe] It is not pushed back, it is in pending review. The sprint is just adjusted. You can try the patch by replacing the files from [the pull request](https://github.com/appcelerator/titanium_mobile/pull/9803/files) with the ones in your local SDK. Make a backup before and replace them in
Thanks, Hans. I misunderstood all the JIRA messages I was seeing. Good to see you guys gave this priority. I'll try not to get too concerned about sprint reassignments in the future! :P
Thanks for that @Hans. Replacing those files in my SDK seems to have fixed this issue for me. :+1:
And for me. So much better. Finally. Thank you.
Yey! Thanks guys. 7.1.1 is expected for the next few weeks, so the manual patch can soon be replaced.
Verified changes are present in SDK builds 7.1.1.v20180419111054 & 7.2.0.v20180501064243
Hi guys, this problem persists on 7.2.0GA and 7.3.0build20180525122818 Tested on iPhone X, three clicks(clicked on cancel(1) and logs 1,0,0) Test code:
Hi guys, I tried to set run-on-main-thread to false and alertDialog is acting normally. As is stated above, It's not a good and it can cause another problems, so please look at it again. Thanks