-
Notifications
You must be signed in to change notification settings - Fork 472
Closes #8122: Do not redirect to market app if app is already installed #8126
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
private fun isPackageInstalled(packageName: String): Boolean { | ||
return try { | ||
context.packageManager.getPackageInfo(packageName, PackageManager.GET_ACTIVITIES) | ||
true | ||
} catch (e: PackageManager.NameNotFoundException) { | ||
false | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe can share this logic as an extension function as we are doing the same on other components see
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'll check if there's a good way to share this. Thanks,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the extension function. Unfortunately support-ktx
depends on support-utils
causing a circular dependency so I couldn't replace the Browsers.kt
call.
* | ||
* @param packageName The name of the package to check for. | ||
*/ | ||
fun PackageManager.isPackageInstalled(packageName: String, flags: Int): Boolean { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Can we add a test for this :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'll add it. Thanks
…lready installed
bors r=Amejia481 |
Build succeeded: |
@rocketsroger Is it possible that this change introduced the regression: mozilla-mobile/fenix#14843? |
I don't think so. If anything it should help with the problem. I'll self assign and look into the other issue when I have a chance. Thanks |
Pull Request checklist
After merge