Show warning if module is not installed on internal storage

[pylerSM]

FLAG_FORWARD_LOCK is hidden flag in ApplicationInfo. So use anyway?

[rovo89]

FLAG_FORWARD_LOCK is hidden flag in ApplicationInfo. So use anyway?

Depends on whether paid modules stored in /mnt/asec can be identified without it. If FLAG_FORWARD_LOCK implies FLAG_EXTERNAL_STORAGE, then no need to check it. Otherwise, I think we have to check it (we could copy the constant definition, no need for reflection I think).

I'm also wondering whether this needs a separate string? For apps on the SD card, the user should find out rather easily how to move it back. For forward locked apps, it's pretty complicated and probably needs the developer's help. So maybe have two strings:

"This module cannot be loaded because it's installed on the SD card, please move it to internal storage." "This module is forward-locked, please contact the module's developer."

(when separating these two, I would actually get rid of the negation, i.e. name the method and string isInstalledOnExternalStorage)

[rovo89]

Oh, and one minor personal preference: isInstalledOnInternalStorage() could be reduced to a single line:

public boolean isInstalledOnInternalStorage() {
    return (app.flags & ApplicationInfo.FLAG_EXTERNAL_STORAGE) == 0;
}

(or != 0 if we go for isInstalledOnExternalStorage)

[pylerSM]

Tested, we should look on forward lock also.

[pylerSM]

Added check for forward locked modules. Should I fix something else, @rovo89?

[rovo89]

Looks very good now, thanks! One thing I'm not sure about: Do we need to escape apostrophes in strings?

And sorry for being very nitpicky sometimes, but the blank line before isInstalledOnExternalStorage() shouldn't have any blanks. I have enabled indicators for leading and trailing spaces in my editor to spot such things.

Anyway, this is something I can correct when merging this (I would combine it into a single commit with you as author anyway), this is just for the future. ;)

There's one more thing that came to my mind. The version check is also performed when writing the module list: https://github.com/rovo89/XposedInstaller/blob/master/src/de/robv/android/xposed/installer/util/ModuleUtil.java#L203 I guess we should do the same for the two new checks. This would make sense if an already enabled module is moved to external storage later. I assume it triggers a broadcast handled by Xposed Installer (would be good to test it!), so this would remove the module out of the modules.list file and thus avoid the "file not found" messages.

[pylerSM]

• Fixed apostrophes for newly added strings
• Removed spaces

Now I can look in ModuleUtil.

And sorry for being very nitpicky sometimes That's OK. Code readability is important for such a big project.

[pylerSM]

So condition like this?

if (module.minVersion > installedXposedVersion || module.minVersion < MIN_MODULE_VERSION || module.isInstalledOnExternalStorage() || module.isForwardLocked()) continue;

or added second condition after original code?

if (module.isInstalledOnExternalStorage() || module.isForwardLocked()) continue;

[rovo89]

I'd go for the second condition.

[pylerSM]

Ok, I will rewrite it.

BTW (off topic) https://github.com/rovo89/XposedInstaller/blob/cdb3f47895f5543eca0ff94861f5a98b343213c6/src/de/robv/android/xposed/installer/InstallerFragment.java#L902 --> executing "reboot recovery" doesn't work on some devices?

[rovo89]

executing "reboot recovery" don't work on some devices?

Yes. Xperia devices it was I think.

[pylerSM]

I disabled my xposed module using "pm disable pkg" and after reboot it was loaded normally and module perfomed all modifications.

I think we should handle even this state, when app is disabled. I will update this PR.

[rovo89]

This should already be handled.

[pylerSM]

This should already be handled.

I tested it some minutes ago and even if module is disabled (pm disable), it still works.

[pylerSM]

I see no "getApplicationEnabledSetting" in your code yet. So how do you check if module is disabled by system disable feature?

[rovo89]

Check ModuleUtil around line 71. I use the enabled flag in application info. Such modules are not considered installed, so they should not be shown in the module fragment and should be removed from the file. Maybe you rebooted before the receiver could rewrite the file?

[pylerSM]

I disabled module via pm, waited for some minutes & then rebooted. After reboot I couldn't find module in menu and xposed modules list, but module features were working.

Afaik, it is strange. if you reenable module using pm, "module update notification" is shown (receiver got action) But if you disable it using pm, nothing is shown (no action or what? file with xposed modules list is not updated).

Now I know patches to show UI about this is useless (since modules are hidden). We need just this. https://github.com/rovo89/XposedInstaller/pull/254

[rovo89]

This has been working, so please let's not blow up this feature request with "fixes" for possible bugs. I don't have the time to merge this at the moment, but once I have, I will only merge the parts for SD card/forward locked.

The handling for disabled modules is a different issue. It should work like this: When you execute "pm disable", it will trigger the PackageChangeReceiver with ACTION_PACKAGE_CHANGED. That will call reloadSingleModule(). As the module is not enabled then, it will return null and will also trigger an immediate reload of the module list UI. If the module had been enabled before, it will be deactivated and the module list will be updated, which should show a toast message. So my suggestion is that you follow this control flow and see where it stops unexpectedly (maybe simply add some more toasts for testing). If you find something, please create a new issue or pull request for it.

[darkyuhoo01]

Releases only Be notified only for new releases, or when participating or @mentioned