Titanium JIRA Archive
Titanium SDK/CLI (TIMOB)

[TIMOB-24349] iOS: Ti.UI.AlertDialog Not Firing Events Consistently (run-on-main-thread)

GitHub Issuen/a
TypeBug
PriorityCritical
StatusClosed
ResolutionFixed
Resolution Date2018-04-12T15:44:45.000+0000
Affected Version/sn/a
Fix Version/sRelease 7.1.1
ComponentsiOS
Labelsios, qe-6.2.0, run-on-main-thread
ReporterKelly Corn
AssigneeHans Knöchel
Created2017-01-25T21:55:48.000+0000
Updated2018-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

FileDateSize
index.js2017-01-25T21:55:04.000+0000507
index.xml2017-01-25T21:55:05.000+0000116
system.log2017-01-26T17:03:19.000+0000262906
system-2017-01-27.log2017-01-27T18:25:46.000+0000248462

Comments

  1. Hans Knöchel 2017-01-26

    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.
  2. Sharif AbuDarda 2017-01-26

    Hello, I can also test the issue. Works for me too.
  3. Mike Stancliffe 2017-01-26

    Also seeing this behavior on all SDKs after 5.3.1
  4. Hans Knöchel 2017-01-26

    [~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.
  5. Mike Stancliffe 2017-01-26

    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.
  6. Mike Stancliffe 2017-01-26

    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
  7. Hans Knöchel 2017-01-26

    [~cliff_stander] Did you test this on iOS 10 or later? And using main-thread on/off?
  8. Mike Stancliffe 2017-01-26

    ipad air iOS 10.1 just recreated, run-on-main-thread = true
  9. Hans Knöchel 2017-01-26

    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.
  10. Mike Stancliffe 2017-01-26

    I have not yet seen the crash, but will send it in if I do
  11. Mike Stancliffe 2017-01-26

    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.
        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');
           c++;
           var bDialog = Ti.UI.createAlertDialog({
           cancel: 0,
           buttonNames: ['Cancel', 'Ok'],
           message: 'B'
        });
        				
         bDialog.addEventListener('click', function(e) {
        if (e.index == 1) {
        Ti.API.info(c+' OK - b');
        } else {
        Ti.API.info(c+' Cancel - b');
        }});
        				
        bDialog.show();
        } else {
         Ti.API.info(c+' Cancel.');
        }
        });
        aDialog.show();
        }
        $.index.open();
        
  12. Kelly Corn 2017-01-26

    Here is a system.log with a crash.
  13. Hans Knöchel 2017-01-27

    Ok, the error helps a lot! Until I am able to reproduce it, can you try to modify the TiUIAlertDialogProxy.m method fireClickEventWithAction with
        TiThreadPerformOnMainThread(^{
            [self fireEvent:@"click" withObject:event];
        }, YES);
        
    instead of
        [self fireEvent:@"click" withObject:event];
        
  14. Kelly Corn 2017-01-27

    Hans, I made the change, and did not see much improvement. I am attaching a second log for you with another crash.
  15. Hans Knöchel 2017-01-27

    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)?
  16. Hans Knöchel 2017-01-27

    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 the source 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.
  17. Kelly Corn 2017-01-27

    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]?
  18. Hans Knöchel 2017-06-22

    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!
  19. Vijay Singh 2017-06-29

    [~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 -
        #ifndef TI_USE_KROLL_THREAD
            KrollContext *krollContext = [self.pageContext krollContext];
            [krollContext forceGarbageCollectNow];
        #endif
        
    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.
  20. Kelly Corn 2017-06-29

    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)
  21. Vijay Singh 2017-06-30

    Thanks [~kellycorn]. PR: https://github.com/appcelerator/titanium_mobile/pull/9181
  22. Harry Bryant 2017-06-30

    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.*
  23. Muhammad Qasim 2017-11-09

    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 set run-on-main-thread = false then the click listener is fired twice.
  24. dbenhenni 2018-01-24

    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.
  25. Hans Knöchel 2018-01-24

    [~vijaysingh] Can you revisit this ticket and write a stress-test?
  26. Vijay Singh 2018-01-26

    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!
  27. Reymundo López 2018-02-05

    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
  28. Vijay Singh 2018-02-06

    [~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!
  29. dbenhenni 2018-02-06

    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.
        // -- confirm function -------------------------------------------------------
        UI.confirm = function(msg, callback, selectedButton, buttons) {
        	var opts = {
        		options : buttons,
        		selectedIndex : ( selectedButton ? selectedButton : null ),
        		//destructive : 1,  <- tested with and without destructive. No difference.
        		title : msg
        	};
        	
        	var dialog = Ti.UI.createOptionDialog(opts);
        	
        	var f = function(e) {
        		console.log("!! UI.Confirm -> Click for: "+msg+" callback -> "+(callback ? "yes" : "no"));
        		dialog.removeEventListener("click", f);
        		if (callback) {callback(e);}
        	};
        	dialog.addEventListener('click', f);
        	dialog.show();
        };
        
        // -- Calling the confirmation function -----------------
        UI.confirm("actions", function(e) {
             console.log(e.source.options[e.index] + " clicked);
        }, 0, ['click A','click B','click C']);
        
  30. Hans Knöchel 2018-02-06

    Reopening ticket. [~dbenhenni] Please format your code. Also, I am wondering how the above code even compiles, as
            console.log("!! UI.Confirm -> Click for: "msg" callback -> "+(callback ? "yes" : "no"));
        
    includes a syntax error around "msg" that represents a variable without " + msg + ".
  31. Vijay Singh 2018-02-06

    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!
  32. Hans Knöchel 2018-02-06

    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)
        var count = 0;
        
        var win = Ti.UI.createWindow({
          backgroundColor: '#fff'
        });
        
        var btn = Ti.UI.createButton({
         title: 'Trigger'
        });
        
        btn.addEventListener('click', function() {
          var dialog = Ti.UI.createAlertDialog({
            title: 'Test',
            message: 'Hello',
            buttonNames: ['OK', 'Cancel'],
            cancel: 1
          });
          dialog.addEventListener('click', function() {
            Ti.API.warn((++count) + ' CLICKED!');
          });
          dialog.show();
        });
        
        win.add(btn);
        win.open();
        
    Test-Case 2/2 (alert from background thread):
        /**
         * This file is used to validate iOS test-cases. It is ran using the Xcode
         * project in titanium_mobile/iphone/iphone/Titanium.xcodeproj.
         *
         * Change the below code to fit your use-case. By default, it included a button
         * to trigger a log that is displayed in the Xcode console.
         */
        var win = Ti.UI.createWindow({
            backgroundColor: '#fff'
        });
        
        var btn = Ti.UI.createButton({
            title: 'Trigger',
            width: 300,
            height: 45,
            top: 100,
            color: '#fff',
            backgroundColor: '#9B1C1F',
            borderRadius: 10
        });
        
        btn.addEventListener('click', function() {
            btn.setTitle('Loading ...');
            btn.setEnabled(false);
        
            var url = 'http://www.appcelerator.com';
            var client = Ti.Network.createHTTPClient({
                onload: function(e) {
                    btn.setTitle('Trigger');
                    btn.setEnabled(true);
        
                    Ti.UI.createAlertDialog({
                        message: 'Success',
                        buttonNames: ['OK']
                    }).show();
                },
                onerror: function(e) {
                    btn.setTitle('Trigger');
                    btn.setEnabled(true);
        
                    Ti.UI.createAlertDialog({
                        message: 'Error',
                        buttonNames: ['OK']
                    }).show();
                },
                timeout: 5000 // in milliseconds
            });
            client.open('GET', url);
        
            client.send();
        });
        
        win.add(btn);
        win.open();
        
  33. Reymundo López 2018-02-06

    [~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.
  34. Hans Knöchel 2018-02-16

    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.
  35. Vijay Singh 2018-02-28

    [~reymundolopez]Ping for test case. Thanks!
  36. Hans Knöchel 2018-03-12

    [~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.
  37. Reymundo López 2018-03-12

    [~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.
  38. Jason Priebe 2018-03-12

    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.
  39. Matthew Delmarter 2018-03-12

    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.
  40. Jason Priebe 2018-03-12

    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/
  41. dbenhenni 2018-03-13

    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.
  42. Jason Priebe 2018-04-08

    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.
  43. Hans Knöchel 2018-04-09

    [~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
        ~/Library/Application Support/Titanium/mobilesdk/osx/7.1.0.GA/iphone/Classes/
        
  44. Jason Priebe 2018-04-09

    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
  45. kosso 2018-04-10

    Thanks for that @Hans. Replacing those files in my SDK seems to have fixed this issue for me. :+1:
  46. Matthew Delmarter 2018-04-12

    And for me. So much better. Finally. Thank you.
  47. Hans Knöchel 2018-04-12

    Yey! Thanks guys. 7.1.1 is expected for the next few weeks, so the manual patch can soon be replaced.
  48. Eric Wieber 2018-05-01

    Verified changes are present in SDK builds 7.1.1.v20180419111054 & 7.2.0.v20180501064243
  49. Marian Kucharcik 2018-06-19

    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:
        var alertDialog = Ti.UI.createAlertDialog({
        	title:"Please click on cancel",
        	message:"First time logs 1, second time logs 0",
        	buttonNames:["Button", "Cancel"],
        	
        });
        versionAlertDialog.addEventListener("click", function(e)
        {
        	Ti.API.log(e.index); 
        }); 
        
  50. Marian Kucharcik 2018-06-19

    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

JSON Source