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(android): prevent showing base activity #13889

Closed
wants to merge 17 commits into from

Conversation

m1ga
Copy link
Contributor

@m1ga m1ga commented Jul 28, 2023

fixes #13791

var win1 = Ti.UI.createWindow({
	backgroundColor: "green"
})
win1.addEventListener("click", function() {
	var win2 = Ti.UI.createWindow({
		backgroundColor: "red"
	})
	win2.open();
})
win1.open();

Steps to reproduce

  • open the app (green window)
  • click to open the second window (red)
  • close the red window and close the green window right away
  • you will see the root activity

more infos in the ticket

Update

I've changed the PR a bit. Instead of checking the stuff when closing the window I'll do it when adding a new activity. If we have more then one and the first one is still TiRootActivity (the launch screen) then it will be removed. This also helps you add <application android:enableOnBackInvokedCallback="true"/> in your tiapp.xml to enable Androids new predictive back feature. This way it will show the dashboard instead of the icon!

NOTE:

It will only run in a production build. It looks like LiveView needs it so it can restart the app without loading the whole app again.

If you test it make sure to build a dist-playstore app!

@m1ga m1ga marked this pull request as draft January 28, 2024 19:07
@m1ga m1ga marked this pull request as ready for review May 23, 2024 10:45
@m1ga m1ga added the bug label May 23, 2024
@hansemannn
Copy link
Collaborator

@m1ga We are currently testing this, but needed some other adjustments done by @prashantsaini1 to support Gradle 8 properly.

@@ -190,6 +190,24 @@ public static boolean firstOnActivityStack()
public static void addToActivityStack(Activity activity)
{
if (activity != null) {
if (!isDevelopment && activityStack.size() > 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like this hack might be a symthome of not using the best possible solution. According to both Apple and Google, apps should have the same core-functionality between dev and prod

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only need it for Liveview as that needs the first activity not to be closed to restart the app. But we don't have a way to check for Liveview, that's why I've used dev/prod to do the check

Copy link
Collaborator

Choose a reason for hiding this comment

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

Alright, then @prashantsaini1 will know what to do. It's still odd to have this check, but looking at Liveview it may make sense.

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 works without it but Liveview will look a bit different as you'll see the home screen and then it will launch again. I don't like the dev/prod difference either 😄 Let's see what Prashant will find 👍

@prashantsaini1
Copy link
Contributor

prashantsaini1 commented Nov 29, 2024

So I checked this PR today a bit more and found following observations:

  1. The code changes in this PR will cause more issues as the root-activity is closed as soon as a window is added to stack.
  2. Root activity is the base activity of a titanium app and should only be closed when we really want to terminate the app. This will also stop sending newintent event used to share content from other apps to the titanium app (as is the case in our app). Root activity is generally closed automatically in app when no windows are open.
  3. However, I checked the SDK code and found that this issue is caused by an incorrect setting of exitOnClose property defaults to false at this line of code, whereas the docs says here that it's true by default for the first window launched, but it's overridden by the current window count which may be in accurate in native back press handling until this point (as mentioned in below point also).
  4. I also noted that this behaviour is not happening when we close window programmatically using window.close(), SDK code tells me that this is probably due to that windows references are removed properly in this scenario and Titanium knows the activity stack upfront, however, just relying on back press delegates it to internal Android SDK and then Titanium comes to know about an activity being closed later, which is why in my tests I found that even pressing back button twice, the window count in Titanium SDK was 2, whereas there was no window at all.

So to fix @m1ga issue, you can just set exitOnClose: true on your first window and it would work as expected:

 var win1 = Ti.UI.createWindow({
	exitOnClose: true,
	backgroundColor: "green"
})

@prashantsaini1
Copy link
Contributor

prashantsaini1 commented Nov 29, 2024

@m1ga You can revert rest of the changes in this PR, and add below code after this line fixes it.

// Set default value on first window proxy also if not already set above.
setProperty(TiC.PROPERTY_EXIT_ON_CLOSE, true);

@m1ga
Copy link
Contributor Author

m1ga commented Nov 29, 2024

I would need to test that.

But it still leaves us with the issue that when you use android:enableOnBackInvokedCallback="true" you'll end up with the base activity as it is still running and not the home screen icons.

Root activity is generally closed automatically in app when no windows are open.

From my old tests it looked more like a race condition because when you close the windows slowly/normally it works fine; only when you close both fast and you close the root window in the time frame that the internal "close" animation was still running it will end up with the base activity still open. So my guess was more that it was still running some code before it cleared itself from the stack. That's why my solution was to have it not even there anymore so when you close the root window there is no chance to end up at the base activity. As a side effect the enableOnBackInvokedCallback was running.

@prashantsaini1
Copy link
Contributor

I also found the same behaviour that windows closing flow runs in a race condition, which is why I didn't prefer any other solution.

@m1ga
Copy link
Contributor Author

m1ga commented Dec 4, 2024

@prashantsaini1 I've added the line in this PR: #14150

That should fix the closing + showing the base activity issue. Then we can look at the enableOnBackInvokedCallback at a different time.

@m1ga m1ga closed this Dec 4, 2024
@m1ga m1ga deleted the 230729_android_window_close branch December 16, 2024 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android: closing two windows quickly won't close app
3 participants