[TIMOB-2433] Android: OptionMenu MenuItem's click callbacks don't persist w/ tabbed window switching

Description

Attachments

Comments

1. Don Thorp 2011-04-15

   Robby, I'm going to be changing the menu API for 1.5 this weekend if I can get it finished. It would be a great help if you could attach a non-proprietary test case that I can use to verify what you're seeing now. The underlying problem is a menu is directly tied to an Android Activity and in 1.4.X and earlier it's almost impossible to always guess the correct one. In 1.5.X menu will move to Ti.Android.currentActivity so that it's always associated with the correct one.

2. Robby 2011-04-15

   Don, this is a tough one to reproduce. I spent some time on developing a test case but they worked rather consistently. I then took my code and tried to chip away at it until I could identify the problem. The code I have constructs a tableview, search bar, then makes a HTTP request. After the HTTP request returns, it calls a function that puts several labels and a view (a notification circle) on each tableviewrow object, then calls setData on the tableview. Rather direct, and it works in Android 1.4 and on iOS fine, but I need 1.5.x as I have a few custom Android modules.

   The problem I'm actually seeing with that window is that the menu button normally doesn't work at all (that, or sometimes it will work, but the onclick handlers for the children wont). By selectively removing parts, it would seem to be somewhat of a race condition: If I keep everything but the rendering of the labels, it works fine, every time. Then, I try something like the following (I'm rendering 3 labels and a view on each tableview row overall):

   Render label 1: works
   render label 2: works
   render view and label 3: works
   render label 1 & 2: works
   render label 1 & 3, and view: works
   render label 1 & 3, and view: works
   render label 1, 2, 3, and view: fails (rendering appears fine, but the menu button doesn't work)

   So the elements render alone fine, but if I try ALL of them, it fails. I have my debug up to trace, and all I see is:

   error: [TRACE] W/InputManagerService( 61): Window already focused, ignoring focus gain of: com.android.internal.view.IInputMethodClient$Stub$Proxy@45065610

   When I try to press the menu button.

   Again, this works fine on 1.4 (just tested it now again on the 1.4 build from Oct 15th).... I can send you the relevant proprietary code if needed. I'm also available to speak more on this over Lighthouse or the phone as well. I can't seem to find a workaround to this either, and can't do the initial release on Android until the menus work unfortunately, so I'm your man on this issue :).

   If you're planning a redesign of the 1.5 menu system, it may be best to just move forward with that and see if that magically fixes the problem. There's a very good chance it would. I've been running into plenty of menu weirdness in 1.5 under Android.

   1.4.2 where it works: version=1.4.2 timestamp=10/11/10 19:02 githash=425bc37
   1.5 where it fails: version=1.5.0 timestamp=11/24/10 04:54 githash=c0aff27

   This is under Windows 7 64bit.

3. Robby 2011-04-15

   Oops, meant to say the 1.4.2 build from Oct 11th, not the 15th.

4. Don Thorp 2011-04-15

   Robby, do you have a sample I can test with. I want to verify this works with the new menu code.

5. Robby 2011-04-15

   Hi Don,

   I migrated to the new code based off of android activities, using the commit you made as an example. Looks very promising so far. A few issues:

   1. In the menu callbacks, I had an alert() statement to test. Before this migration, I used to get an alert box pop up, now the text is printed to the console without the box popping up (just like would have happened if I had done Ti.API.info()). Is this a bug?

   2. I believe you have a bug in your menu.add() code similar to #274. I have code in one of my windows that adds to the menu from a xhr.onload callback (i.e. not the UI thread), and I get:

      [TRACE] E/KrollCallback( 472): (kroll$5) [262,145110] Error evaluating source, invocation: [callMethod Network.HTTPClient.(anonymous) org.appcelerator.titanium.kroll.KrollCallback@452efb28], message: Wrapped android.view.ViewRoot$CalledFromWrongThreadException: Only the original thread that created a view hierarchy can touch its views. (app://win_games.js#797)
      [TRACE] E/KrollCallback( 472): org.mozilla.javascript.WrappedException: Wrapped android.view.ViewRoot$CalledFromWrongThreadException: Only the original thread that created a view hierarchy can touch its views. (app://win_games.js#797)
      [TRACE] E/KrollCallback( 472): at org.mozilla.javascript.Context.throwAsScriptRuntimeEx(Context.java:1781)
      [TRACE] E/KrollCallback( 472): at org.appcelerator.kroll.KrollMethod.call(KrollMethod.java:77)
      [TRACE] E/KrollCallback( 472): at org.mozilla.javascript.Interpreter.interpretLoop(Interpreter.java:1711)
      [TRACE] E/KrollCallback( 472): at script(app://win_games.js:797)
      [TRACE] E/KrollCallback( 472): at script(file:///android_asset/Resources/shared.js:413)
      [TRACE] E/KrollCallback( 472): at org.mozilla.javascript.Interpreter.interpret(Interpreter.java:854)
      [TRACE] E/KrollCallback( 472): at org.mozilla.javascript.InterpretedFunction.call(InterpretedFunction.java:164)
      [TRACE] E/KrollCallback( 472): at org.mozilla.javascript.ContextFactory.doTopCall(ContextFactory.java:426)
      [TRACE] E/KrollCallback( 472): at org.mozilla.javascript.ScriptRuntime.doTopCall(ScriptRuntime.java:3161)
      [TRACE] E/KrollCallback( 472): at org.mozilla.javascript.InterpretedFunction.call(InterpretedFunction.java:162)
      [TRACE] E/KrollCallback( 472): at org.appcelerator.titanium.kroll.KrollCallback.callSync(KrollCallback.java:109)
      [TRACE] E/KrollCallback( 472): at org.appcelerator.titanium.kroll.KrollCallback$1.run(KrollCallback.java:133)
      [TRACE] E/KrollCallback( 472): at android.os.Handler.handleCallback(Handler.java:587)
      [TRACE] E/KrollCallback( 472): at android.os.Handler.dispatchMessage(Handler.java:92)
      [TRACE] E/KrollCallback( 472): at android.os.Looper.loop(Looper.java:123)
      [TRACE] E/KrollCallback( 472): at org.appcelerator.titanium.kroll.KrollHandlerThread.run(KrollHandlerThread.java:73)
      [TRACE] E/KrollCallback( 472): Caused by: android.view.ViewRoot$CalledFromWrongThreadException: Only the original thread that created a view hierarchy can touch its views.
      [TRACE] E/KrollCallback( 472): at android.view.ViewRoot.checkThread(ViewRoot.java:2802)
      [TRACE] E/KrollCallback( 472): at android.view.ViewRoot.requestLayout(ViewRoot.java:594)
      [TRACE] E/KrollCallback( 472): at android.view.View.requestLayout(View.java:8125)
      [TRACE] E/KrollCallback( 472): at android.view.View.requestLayout(View.java:8125)
      [TRACE] E/KrollCallback( 472): at android.view.ViewGroup.removeAllViews(ViewGroup.java:2255)
      [TRACE] E/KrollCallback( 472): at com.android.internal.view.menu.IconMenuView.updateChildren(IconMenuView.java:338)
      [TRACE] E/KrollCallback( 472): at com.android.internal.view.menu.MenuBuilder.onItemsChanged(MenuBuilder.java:908)
      [TRACE] E/KrollCallback( 472): at com.android.internal.view.menu.MenuBuilder.addInternal(MenuBuilder.java:379)
      [TRACE] E/KrollCallback( 472): at com.android.internal.view.menu.MenuBuilder.add(MenuBuilder.java:393)
      [TRACE] E/KrollCallback( 472): at org.appcelerator.titanium.proxy.MenuProxy.add(MenuProxy.java:64)
      [TRACE] E/KrollCallback( 472): at org.appcelerator.titanium.proxy.MenuProxyBindingGen$6.invoke(MenuProxyBindingGen.java:264)
      [TRACE] E/KrollCallback( 472): at org.appcelerator.kroll.KrollMethod.call(KrollMethod.java:46)
      [TRACE] E/KrollCallback( 472): ... 12 more
      

   Besides those issues, menu loading and functionality SEEM to be much improved!! Let me know if you address these issues and I'll give her another go.

   Robby

6. Don Thorp 2011-04-15

   Robby, more changes were pushed. Without and example I can verify that this is fixed.

7. Robby 2011-04-15

   Don,

   Just tried it with the nightly from today: version=1.5.0 timestamp=12/09/10 01:18 githash=43358e5

   Item #1 from above seems to be fixed. Below is a testcase that isolates #2 from above. (I also used it to verify item #1 from above was fixed.)

      Ti.API.info("@ win_test.js");
      var win = Ti.UI.currentWindow;
      win.backgroundColor = "#b5aea5";
      
      
      var activity = Ti.Android.currentActivity;
      activity.onCreateOptionsMenu = function(e) {
        var aMenu = e.menu;
        var test1 = aMenu.add({
          title: 'Test1',
        });
        test1.addEventListener('click', function() {
          alert("alert: Test1");
        });
        
        var test2 = aMenu.add({
          title: 'Test2',
        });
        test2.addEventListener('click', function() {
          alert("alert: Test2");
          Ti.API.info("Ti.API.info: Test2");
        });
        
        //Now, make a request and add a menu item from a non-UI thread...
        var request = Ti.Network.createHTTPClient({
          timeout: 15000 //15 seconds
        });
        
        request.onload = function(e) {
          Ti.API.info('HTTPClient onload, status: ' + this.status);
          Ti.API.info("Adding additional menu item (this will fail)");
          
          var test3 = aMenu.add({
            title: 'Test3',
          });
          test3.addEventListener('click', function() {
            alert("alert: Test3");
          });
        };
        
        var url = "http://www.google.com/";
        Ti.API.info('Making GET call to: ' + url);
        request.open("GET", url, true);
        request.send();
      }
      
      
      var displayName = Ti.UI.createLabel({
        top: 0,
        left: 0,
        font:{fontSize: 20, fontWeight:'bold'},
        height: 30,
        width: 320,
        color:'#000000',
        text:"Testing 123"
      });
      win.add(displayName);
      

8. Don Thorp 2011-04-15

   There is a simple workaround and this is really more how it should be done. Since there is an easy workaround, moving out of M05. It's not worth the risk. Basically you should modify your menu in onPrepareOptionsMenu for the state.

      Ti.API.info("@ win_test.js");
      var win = Ti.UI.currentWindow;
      win.backgroundColor = "#b5aea5";
      
      var xhrComplete = false;
      
      var activity = Ti.Android.currentActivity;
      activity.onCreateOptionsMenu = function(e) {
        var aMenu = e.menu;
        var test1 = aMenu.add({
          title: 'Test1',
        });
        test1.addEventListener('click', function() {
          alert("alert: Test1");
        });
        
        var test2 = aMenu.add({
          title: 'Test2',
        });
        test2.addEventListener('click', function() {
          alert("alert: Test2");
          Ti.API.info("Ti.API.info: Test2");
        });
        
        //Now, make a request and add a menu item from a non-UI thread...
        var request = Ti.Network.createHTTPClient({
          timeout: 15000 //15 seconds
        });
        
        request.onload = function(e) {
          Ti.API.info('HTTPClient onload, status: ' + this.status);
          Ti.API.info("Adding additional menu item (this will fail)");
          setTimeout(function() {
              xhrComplete = true;
          }, 5000);    
        };
        
        var url = "http://www.google.com/";
        Ti.API.info('Making GET call to: ' + url);
        request.open("GET", url, true);
        request.send();
      }
      
      activity.onPrepareOptionsMenu = function(e) {
          var aMenu = e.menu;
          
          if (aMenu.size() < 3 && xhrComplete) {
              var test3 = aMenu.add({
                title: 'Test3',
               });
              test3.addEventListener('click', function() {
                alert("alert: Test3");
              });
          }   
      };
      
      
      var displayName = Ti.UI.createLabel({
        top: 0,
        left: 0,
        font:{fontSize: 20, fontWeight:'bold'},
        height: 30,
        width: 320,
        color:'#000000',
        text:"Testing 123"
      });
      win.add(displayName)
      

9. Robby 2011-04-15

   Great, thanks Don! I'll give this a try this weekend and let you know how it works out.

   The new menu system seems to work much better!

10. Don Thorp 2011-04-15

    Was put in wrong bucket during move from 1.5.1.

11. Don Thorp 2011-04-15

    Please triage.

12. Opie Cyrus 2011-04-15

    (from [f4238600ef496a4059347f3c5208bae27ed5427a]) [#2433 state:fixed-in-qa] Modify MenuProxy kroll calls to run on UI thread

    Modified MenuProxy to have all Kroll calls that modify the menu execute on the UI thread and resolve the "Only the original thread that created a view hierarchy can touch its views" error.
    https://github.com/appcelerator/titanium_mobile/commit/f4238600ef496a4059347f3c5208bae27ed5427a"> https://github.com/appcelerator/titanium_mobile/commit/f4238600ef49...

13. Opie Cyrus 2011-04-15

    Please test the fix with the attached test app. This is basically the pasted code above from Robby.

    1) Click "open window"
    2) Launch the menu and you should see 3 tabs shown in the menu.

    Thanks
    Opie

14. Thomas Huelbert 2011-04-15

    [INFO] Titanium SDK version: 1.6.0 (01/10/11 08:25 3452f06), G1 (1.6) droid 1 (2.2.1) sim 2.1. lag on third tab appearing due to network issues.

JSON Source