[Android 11+] Correctly open URLs in browser and fix opening downloads and external players #9850
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Stypox
force-pushed
the
fix-api33-links-again3
branch
from
88b6c1d to
f329b32
Compare
6 tasks
AudricV
changed the title
AudricV
requested changes
Feb 24, 2023
There was a problem hiding this comment.
Changes looks good overall to me, except the following ones which needs to be solved.
Please split the sentences in Javadocs into "visible" multiple lines, i.e. lines that can be seen in the Javadoc preview, especially in the ones you changed in ShareUtils. This would make their reading a lot better.
Could you also reword your commit message to summarize better what you have done here, so we are not really forced to look at the pull request in the future to understand better your changes? Thanks in advance!
app/src/main/java/org/schabi/newpipe/util/external_communication/KoreUtils.java
Outdated
Show resolved
Hide resolved
app/src/main/java/org/schabi/newpipe/util/external_communication/KoreUtils.java
Outdated
Show resolved
Hide resolved
app/src/main/java/org/schabi/newpipe/util/external_communication/ShareUtils.java
Outdated
Show resolved
Hide resolved
2 tasks
- Fix misconfiguration in manifest ('http|https|market' is not valid)
- Split ShareUtils functions taking a boolean parameter into pairs of functions with better names and less runtime checks
- Move all Kore-related functions to KoreUtils
- Remove the toast_no_player string
Stypox
force-pushed
the
fix-api33-links-again3
branch
from
f329b32 to
5972e50
Compare
|
@AudricV done, thanks for the review :-) |
But such method is not available before API 33
Stypox
force-pushed
the
fix-api33-links-again3
branch
from
5972e50 to
be4f3d9
Compare
|
SonarCloud Quality Gate failed. |
AudricV
approved these changes
Feb 25, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is it?
Description of the changes in your PR
queriesfield in the manifest. I thought I could usehttp|https|marketto specify three schemas at once, but actually it's not possible. Hence, the permission was not actually given by the Android system andgetDefaultAppPackageNamewould not work. Now I removedhttpsandmarketas we never actually query those packages, but we only queryhttpinopenUrlInBrowser.booleanparameter toopenUrlInBrowser, now there are two functions:openUrlInBrowser, which enforces the opened app is a browsergetDefaultAppPackageNameand just hardcode the one important line of code insideopenUrlInBrowser. This is to make sure that function is not used by accident anywhere else, since we don't wantresolveActivitychecks anywhere (tryOpenIntentInAppshould be used instead, which uses atry catchinstead).openUrlInApp, which allows any app to be openedbooleanparameter to decide whetheropenIntentInAppshould show a toast, now there are two functions:openIntentInAppandtryOpenIntentInAppKoreUtilsand deduplicate code for trying to open a Kore intent and showing a failure toast.toast_no_playerstring ("No app installed to play this file"), because it was basically a duplicate ofno_app_to_open_intent("No app on your device can open this").For more info check out the javadocs in
ShareUtilsFixes the following issue(s)
startActivitywithout querying for default app #9817 and Fix opening downloads and streams on external players on Android 11+ and refactor external intents #9833APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.
Due diligence