Fix android.resource Uris that point to a vector drawable fail to load #51
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
pkpdeveloper
force-pushed
the
android_resource_uri_vector_load_bug_fix
branch
4 times, most recently
from
c57eb92
to
797fb3d
Compare
audkar
reviewed
Aug 20, 2019
| if (data.scheme == ContentResolver.SCHEME_ANDROID_RESOURCE) { | ||
| val resourceId = extractResourceId(data) | ||
| resourceId?.let { | ||
| val drawable = context.getDrawableCompat(resourceId) |
There was a problem hiding this comment.
If I understand correctly, this code part assumes that resourceId belongs to same applications. However Uri can point to resourceId which belongs to different application:
"android.resource:https://{pkgName}/{resourceId}"
There was a problem hiding this comment.
Thanks, @audkar for pointing this issue. I got the solution from Glide source code
I will do the changes ASAP 😊
pkpdeveloper
force-pushed
the
android_resource_uri_vector_load_bug_fix
branch
3 times, most recently
from
4941a03
to
6040e5d
Compare
|
Implementation logic seems correct and aligned with platform one (private method used for |
pkpdeveloper
force-pushed
the
android_resource_uri_vector_load_bug_fix
branch
3 times, most recently
from
3abac39
to
5bb3cd4
Compare
pkpdeveloper
force-pushed
the
android_resource_uri_vector_load_bug_fix
branch
from
5bb3cd4
to
a38df6b
Compare
|
Thanks for the contribution. We're working on a bigger refactor of the mappers + fetchers here, which might conflict with this PR. I'll circle back and review this PR after the one I linked is merged. |
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.
Hi @colinrtwhite,
Please review and merge fix for android.resource Uri related issue. also added test cases for that.
I have tried many solutions only this one works as expected.
Please let me know for further improvements or any discussion.
Cheers 🍻 for creating this awesome library 👏 ❤️