feat: Add installer status dialog #1473

oSumAtrIX · 2023-11-08T03:01:14Z

This PR shows a native dialogue after the installation finishes. It can handle all status flags. Business logic may move.

Todo

Handle errors such as:
- Installing regularly while a mount installation is currently active (In this case, uninstall the mount installation)
- Mount errors such as wrong base APK or no base APK present (In this case, show an appropriate dialog)

Review

Check if https://github.com/ReVanced/revanced-manager/pull/1473/files#diff-bd497764717bba99312a9c9db9f60eaa4c5d090266194189db6855574572564bR55 is placed in a suitable location
When uninstallation is necessary, first the uninstallation prompt is shown, then the installation prompt
All strings are chosen appropriately
All icons are chosen appropriately
Showing a SUCCESS status dialog is wanted
All TODOs in this PR are considered or done
uiSafe has been used correctly (In some places, it seems it should be used but it seems out of scope for this PR)

Warning

On installation, the install button is not greyed out, and no progress is visible, so it appears as if nothing happened.

KobeW50

Suggestion for the "invalid" error description

app/src/main/res/values/strings.xml

KobeW50

String changes in error dialog

app/src/main/res/values/strings.xml

app/src/main/java/app/revanced/manager/ui/viewmodel/InstallerViewModel.kt

app/src/main/java/app/revanced/manager/ui/component/InstallerDialog.kt

app/src/main/java/app/revanced/manager/ui/viewmodel/InstallerViewModel.kt

app/src/main/java/app/revanced/manager/ui/component/InstallerDialog.kt

KobeW50 · 2023-11-14T06:21:48Z

Is this also for when you update the Manager?

app/src/main/java/app/revanced/manager/ui/viewmodel/InstallerViewModel.kt

CnC-Robert · 2023-11-17T12:24:39Z

Is this also for when you update the Manager?

No it's only for installing patched apps

Ushie · 2023-11-17T12:40:50Z

ReVanced Manager Updates will also use this, but this PR focuses on patched apps

oSumAtrIX · 2023-12-07T15:22:44Z

@ Axelen123 Currently, the extra status message is not used, but I don't know which extra status messages exist so that I can implement them. For example, if an existing installation is present, but the patched app mismatches signatures, then the dialogue will say:

This flag is returned from installation, and there is no way to differentiate between an APK being invalid, for example, or a signature mismatch.

Furthermore, when uninstalling, the dialogue may also be shown. But once again, there is no way to differentiate between installations, uninstallations or the status message, so the dialogue may say that the installation has failed while the uninstallation failed.

oSumAtrIX · 2023-12-07T15:24:49Z

msedge_XcwCJ412Tb.mp4

(The recording is cut off at the end, it shows the success dialogue when installation succeeded)

Axelen123 · 2023-12-09T13:30:20Z

@ Axelen123 Currently, the extra status message is not used, but I don't know which extra status messages exist so that I can implement them. For example, if an existing installation is present, but the patched app mismatches signatures, then the dialogue will say:

This flag is returned from installation, and there is no way to differentiate between an APK being invalid, for example, or a signature mismatch.

Not sure why Android says the app is invalid. Android should give you CONFLICT on signature mismatch. What is the content of the EXTRA_STATUS_MESSAGE here?

Furthermore, when uninstalling, the dialogue may also be shown. But once again, there is no way to differentiate between installations, uninstallations or the status message, so the dialogue may say that the installation has failed while the uninstallation failed.

Isn't it possible to make a separate dialog or enum for uninstalls?

oSumAtrIX · 2023-12-09T13:34:52Z

Isn't it possible to make a separate dialog or enum for uninstalls?

Yes, but ideally we would have the existing Flags enum have variants of the install and uninstall strings.

Ushie · 2023-12-10T20:41:17Z

Will root install be handled in this?

app/src/main/java/app/revanced/manager/ui/viewmodel/InstallerViewModel.kt

app/src/main/java/app/revanced/manager/ui/component/InstallerStatusDialog.kt

app/src/main/java/app/revanced/manager/ui/viewmodel/InstallerViewModel.kt

app/src/main/java/app/revanced/manager/ui/component/InstallerStatusDialog.kt

app/src/main/java/app/revanced/manager/ui/screen/InstallerScreen.kt

app/src/main/java/app/revanced/manager/ui/viewmodel/InstallerViewModel.kt

app/src/main/java/app/revanced/manager/ui/component/InstallerStatusDialog.kt

app/src/main/java/app/revanced/manager/ui/viewmodel/InstallerViewModel.kt

app/src/main/java/app/revanced/manager/ui/component/InstallerStatusDialog.kt

app/src/main/java/app/revanced/manager/ui/viewmodel/InstallerViewModel.kt

app/src/main/res/values/strings.xml

oSumAtrIX · 2024-01-23T00:18:06Z

@Axelen123 I have updated the PR description with up to date information.

app/src/main/java/app/revanced/manager/ui/viewmodel/PatcherViewModel.kt

Axelen123 · 2024-01-23T17:00:41Z

app/src/main/java/app/revanced/manager/ui/viewmodel/PatcherViewModel.kt

+                UninstallService.APP_UNINSTALL_ACTION -> {
+                    val pmStatus = intent.getIntExtra(
+                        UninstallService.EXTRA_UNINSTALL_STATUS,
+                        PackageInstaller.STATUS_FAILURE
+                    )
+
+                    // TODO: This is unused and may be completely removed everywhere.
+                    // intent.getStringExtra(UninstallService.EXTRA_UNINSTALL_STATUS_MESSAGE)
+
+                    if (pmStatus != PackageInstaller.STATUS_SUCCESS) {
+                        installerStatusDialogModel.packageInstallerStatus = pmStatus
                    }
                }


No logic for reattempting an installation that required uninstalling the installed app is present here. Is this intended?

I am not sure if I understood correctly. If uninstallation fails, the appropriate dialog will be shown. If uninstallation succeeds, but the installation afterwards fails, the appropriate dialog will also be shown.

Axelen123 · 2024-01-23T17:01:57Z

app/src/main/java/app/revanced/manager/ui/viewmodel/PatcherViewModel.kt

-                        app.toast(app.getString(R.string.install_app_fail, extra))
+                    }
+
+                    installerStatusDialogModel.packageInstallerStatus = pmStatus


@PalmDevs should the dialog be shown for successful installs?

If not, after installation the only thing that changes is the button from "Install" to "Open"

If the package installer dialog is hidden after the user clicks Install (some apps do that), yes. Otherwise, maybe not considering the package manager dialog already shows the same thing.

The popup is hidden once the user clicks install.

Should this be marked as resolved?

PalmDevs · 2024-01-23T17:10:42Z

Please send a recording of the latest changes or link me to them, thanks 👍

oSumAtrIX · 2024-01-23T19:15:27Z

@PalmDevs

explorer_IlN3nldlMq.mp4

PalmDevs

A few string changes, need opinions.

app/src/main/res/values/strings.xml

PalmDevs

Strings and UI, LGTM.

oSumAtrIX · 2024-01-24T11:00:02Z

I'll need some help regarding the pending TODOs in the PR body. I am unsure where to get context about root installations when doing regular installation. The idea is, that other dialogs are added in case root installations exist or a root installation can't be done.

Axelen123

I have nothing else to suggest that isn't already being discussed.

Axelen123 · 2024-01-27T23:34:15Z

@oSumAtrIX

When uninstallation is necessary, first the uninstallation prompt is shown, then the installation prompt

Not sure this is within the scope of the PR. One of the cases requires checking the signature of the installed app, which would require functionality that isn't currently present in revanced library.

Mount errors such as wrong base APK or no base APK present (In this case, show an appropriate dialog)

Isn't this already handled?

oSumAtrIX · 2024-01-27T23:46:24Z

Not sure this is within the scope of the PR. One of the cases requires checking the signature of the installed app, which would require functionality that isn't currently present in revanced library.

The dialog model has two functions for installing and uninstalling apps.
The installer returns a result at which the dialog shows the corresponding dialogue. If the signature mismatches, it will suggest to uninstall the app using the models uninstall function and attempting the installation again, which, the dialog will handle the result of:

explorer_IlN3nldlMq.mp4

What I meant with that message is the issue shown in the recording which shows that the installer runs before the uninstaller. The cause is not known and its unclear if it is an issue outside my env.

Isn't this already handled?

The dialog currently ONLY handles Android installer status. It has no context about root installations and root installers also don't return any status code like the Android installer. As it is right now, the dialog is only handling Android installations.

oSumAtrIX linked an issue Nov 8, 2023 that may be closed by this pull request

feat: handle install failure #1472

Open

4 tasks

oSumAtrIX self-assigned this Nov 8, 2023

TheAabedKhan added the ReVanced Manager Compose Regarding the Compose rewrite of ReVanced Manager label Nov 8, 2023

KobeW50 reviewed Nov 9, 2023

View reviewed changes

app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved

oSumAtrIX requested review from Axelen123 and Ushie November 10, 2023 00:49

oSumAtrIX marked this pull request as ready for review November 10, 2023 00:49

KobeW50 reviewed Nov 10, 2023

View reviewed changes

app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved

app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved

Axelen123 requested changes Nov 10, 2023

View reviewed changes

CnC-Robert requested changes Nov 17, 2023

View reviewed changes

app/src/main/java/app/revanced/manager/ui/viewmodel/InstallerViewModel.kt Outdated Show resolved Hide resolved

oSumAtrIX requested review from CnC-Robert and Axelen123 December 7, 2023 15:18

Ushie requested changes Dec 10, 2023

View reviewed changes

Axelen123 requested changes Dec 14, 2023

View reviewed changes

oSumAtrIX requested review from Axelen123 and Ushie December 21, 2023 15:08

Axelen123 requested changes Dec 25, 2023

View reviewed changes

oSumAtrIX requested a review from Axelen123 December 26, 2023 23:28

Axelen123 requested changes Dec 30, 2023

View reviewed changes

oSumAtrIX force-pushed the feat/installer-dialog branch from 4d4d682 to 962e68d Compare January 23, 2024 00:08

oSumAtrIX requested a review from Axelen123 January 23, 2024 00:14

feat: Add installer status dialog

b1a821d

oSumAtrIX force-pushed the feat/installer-dialog branch from 962e68d to b1a821d Compare January 23, 2024 00:24

oSumAtrIX changed the title ~~feat: Show native dialog for installations~~ feat: Add installer status dialog Jan 23, 2024

Axelen123 requested changes Jan 23, 2024

View reviewed changes

app/src/main/java/app/revanced/manager/ui/viewmodel/PatcherViewModel.kt Outdated Show resolved Hide resolved

app/src/main/java/app/revanced/manager/ui/viewmodel/PatcherViewModel.kt Outdated Show resolved Hide resolved

Axelen123 reviewed Jan 23, 2024

View reviewed changes

Axelen123 requested a review from PalmDevs January 23, 2024 17:02

PalmDevs suggested changes Jan 24, 2024

View reviewed changes

app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved

app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved

app/src/main/res/values/strings.xml Show resolved Hide resolved

app/src/main/res/values/strings.xml Show resolved Hide resolved

oSumAtrIX added 2 commits January 24, 2024 11:14

Adjust strings

d147cd6

Log extra status message

8270999

oSumAtrIX requested review from Axelen123 and PalmDevs January 24, 2024 10:20

PalmDevs approved these changes Jan 24, 2024

View reviewed changes

oSumAtrIX requested a review from PalmDevs January 25, 2024 17:52

Axelen123 approved these changes Jan 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add installer status dialog #1473

feat: Add installer status dialog #1473

oSumAtrIX commented Nov 8, 2023 •

edited by Axelen123

KobeW50 left a comment •

edited

KobeW50 left a comment

KobeW50 commented Nov 14, 2023

CnC-Robert commented Nov 17, 2023

Ushie commented Nov 17, 2023

oSumAtrIX commented Dec 7, 2023

oSumAtrIX commented Dec 7, 2023

Axelen123 commented Dec 9, 2023

oSumAtrIX commented Dec 9, 2023

Ushie commented Dec 10, 2023 •

edited

oSumAtrIX commented Jan 23, 2024

Axelen123 Jan 23, 2024

oSumAtrIX Jan 23, 2024 •

edited

Axelen123 Jan 23, 2024

oSumAtrIX Jan 23, 2024 •

edited

PalmDevs Jan 24, 2024

oSumAtrIX Jan 25, 2024

Axelen123 Jan 27, 2024

PalmDevs commented Jan 23, 2024

oSumAtrIX commented Jan 23, 2024 •

edited

PalmDevs left a comment

PalmDevs left a comment

oSumAtrIX commented Jan 24, 2024

Axelen123 left a comment

Axelen123 commented Jan 27, 2024

oSumAtrIX commented Jan 27, 2024

feat: Add installer status dialog #1473

Are you sure you want to change the base?

feat: Add installer status dialog #1473

Conversation

oSumAtrIX commented Nov 8, 2023 • edited by Axelen123

Todo

Review

KobeW50 left a comment • edited

Choose a reason for hiding this comment

KobeW50 left a comment

Choose a reason for hiding this comment

KobeW50 commented Nov 14, 2023

CnC-Robert commented Nov 17, 2023

Ushie commented Nov 17, 2023

oSumAtrIX commented Dec 7, 2023

oSumAtrIX commented Dec 7, 2023

Axelen123 commented Dec 9, 2023

oSumAtrIX commented Dec 9, 2023

Ushie commented Dec 10, 2023 • edited

oSumAtrIX commented Jan 23, 2024

Axelen123 Jan 23, 2024

Choose a reason for hiding this comment

oSumAtrIX Jan 23, 2024 • edited

Choose a reason for hiding this comment

Axelen123 Jan 23, 2024

Choose a reason for hiding this comment

oSumAtrIX Jan 23, 2024 • edited

Choose a reason for hiding this comment

PalmDevs Jan 24, 2024

Choose a reason for hiding this comment

oSumAtrIX Jan 25, 2024

Choose a reason for hiding this comment

Axelen123 Jan 27, 2024

Choose a reason for hiding this comment

PalmDevs commented Jan 23, 2024

oSumAtrIX commented Jan 23, 2024 • edited

PalmDevs left a comment

Choose a reason for hiding this comment

PalmDevs left a comment

Choose a reason for hiding this comment

oSumAtrIX commented Jan 24, 2024

Axelen123 left a comment

Choose a reason for hiding this comment

Axelen123 commented Jan 27, 2024

oSumAtrIX commented Jan 27, 2024

oSumAtrIX commented Nov 8, 2023 •

edited by Axelen123

KobeW50 left a comment •

edited

Ushie commented Dec 10, 2023 •

edited

oSumAtrIX Jan 23, 2024 •

edited

oSumAtrIX Jan 23, 2024 •

edited

oSumAtrIX commented Jan 23, 2024 •

edited