TIMOB-17016-Since Android restore fragments on orientation change, make ...

[salachi]

Added empty constructor so that Android can restore the state. Since Android restore fragments on orientation change, make sure the properties are set correctly

https://jira.appcelerator.org/browse/TIMOB-17016

[hieupham007]

the activity should already be set with the proxy. You just need to call proxy.getActivity() to get the activity

[salachi]

I had tried this but the activity that is returned from the TabGroupProxy is the main application activity and not the window activity. The window activity is stored in the variable tabGroupActivity and is exposed through getWindowActivity but it is protected method.

[hieupham007]

Functional looks good, just a minor comment.

[hieupham007]

tabTag needs to be updated for remaining tabs when a tab is removed. If you run this test case, click delete tab2, then add tab4, then open and close window 2, it will crash.

var tabWin1 = Ti.UI.createWindow({title : 'TW1',backgroundColor:'blue'});
var tabWin2 = Ti.UI.createWindow({title : 'TW2',backgroundColor:'red'});
var tabWin3 = Ti.UI.createWindow({title : 'TW3',backgroundColor:'green'});
var tabWin4 = Ti.UI.createWindow({title : 'TW4',backgroundColor:'yellow'});


var tab1 = Ti.UI.createTab({window : tabWin1,title : 'Tab 1'});
var tab2 = Ti.UI.createTab({window : tabWin2,title : 'Tab 2'});
var tab3 = Ti.UI.createTab({window : tabWin3,title : 'Tab 3'});
var tab4 = Ti.UI.createTab({window : tabWin4,title : 'Tab 4'});


var tabGroup = Ti.UI.createTabGroup({tabs : [tab1,tab2,tab3]});

var win2 = Ti.UI.createWindow({title: 'Window 2'});

var openWindowButton = Ti.UI.createButton({widht:Ti.UI.SIZE,height:Ti.UI.SIZE,title:'Open window2'});
var deleteTabButton = Ti.UI.createButton({top: 10, title: 'Delete Tab2'});
var addTabButton = Ti.UI.createButton({bottom: 10, title: 'Add Tab4'});
tabWin1.add(deleteTabButton);
tabWin1.add(openWindowButton);
tabWin1.add(addTabButton);

var closeWindowButton = Ti.UI.createButton({widht:Ti.UI.SIZE,height:Ti.UI.SIZE,title:'Close window2'});
win2.add(closeWindowButton);

openWindowButton.addEventListener('click',function(e){win2.open();});
deleteTabButton.addEventListener('click', function(e){tabGroup.removeTab(tab2)});
addTabButton.addEventListener('click', function(e){tabGroup.addTab(tab4)});
closeWindowButton.addEventListener('click',function(e){win2.close();});

tabGroup.open();

[salachi]

Updated the PR to use incremental index to generate the tag name

[salachi]

Moved the code that assign tag value to proxy

[hieupham007]

Functionally test failed. Still crashes occasionally with the test case I posted.

[hieupham007]

Any update on this?

[mokesmokes]

I strongly suggest you take care of this one first: #5651
and then revisit this issue, if required, since scrollable tabs change a lot of the tab group code.

[salachi]

Hieu,
I tried many times on my Nexus and Galaxy Note but I can't reproduce the crash with the PR. It will crash if I click 'Delete Tab2' multiple time but it is a programming error. Also, should I wait for #5651 to be merged and then try to fix this?

[mokesmokes]

I would prefer if #5651 be merged first since it is a large PR and much of the fragment handling changes. I confirmed the bug exists there too, unfortunately. In my opinion it will be less overall effort to merge the scrollable tabs and then fix this issue.

[hieupham007]

Ok, I can reproduce this crash effectively now. First, uninstall your app, then do a fresh install. Once you run my test case, click on 'Tab 2', then 'Tab 3' so they're all loaded. Then click on 'Tab 1', then tap on 'Delete Tab2', then 'Open window2', and 'Close window2', app will crash with this error:

E/TiApplication(17533): Caused by: java.lang.NullPointerException
E/TiApplication(17533): at ti.modules.titanium.ui.widget.tabgroup.TiUIActionBarTab$TabFragment.onCreateView(TiUIActionBarTab.java:39)
E/TiApplication(17533): at android.support.v4.app.Fragment.performCreateView(Fragment.java:1500)
E/TiApplication(17533): at android.support.v4.app.FragmentManagerImpl.moveToState(FragmentManager.java:938)
E/TiApplication(17533): at android.support.v4.app.FragmentManagerImpl.moveToState(FragmentManager.java:1115)
E/TiApplication(17533): at android.support.v4.app.FragmentManagerImpl.moveToState(FragmentManager.java:1097)
E/TiApplication(17533): at android.support.v4.app.FragmentManagerImpl.dispatchActivityCreated(FragmentManager.java:1895)
E/TiApplication(17533): at android.support.v4.app.FragmentActivity.onStart(FragmentActivity.java:566)
E/TiApplication(17533): at org.appcelerator.titanium.TiBaseActivity.onStart(TiBaseActivity.java:1077)
E/TiApplication(17533): at android.app.Instrumentation.callActivityOnStart(Instrumentation.java:1164)
E/TiApplication(17533): at android.app.Activity.performStart(Activity.java:5114)
E/TiApplication(17533): at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2153)
E/TiApplication(17533): ... 11 more

[salachi]

Fixed the crash issue after the tab is removed.

[mokesmokes]

@hieupham007 where is your test case app? unable to find it, thanks.

[hieupham007]

@mokesmokes, It was in one of my comments in this PR, but I posted it in the ticket.

[hieupham007]

CR + FR looks good to me.