Skip to content

Vector drawable can't be used as error drawable #1267

New issue
New issue
Closed
Closed
Vector drawable can't be used as error drawable#1267
Assignees
sjudd
Labels
bugenhancement
Milestone
 
4.3
@jakubkrolewski

Description

@jakubkrolewski
jakubkrolewski
opened on Jun 14, 2016 · edited by TWiStErRob
  1. Create a vector drawable in xml
  2. Refer the drawable id in the error method
  3. Run the app on Android < 5.0
  4. Make sure main image is not loaded and the error drawable is displayed
  5. The app crashes with:
android.content.res.Resources$NotFoundException: File res/drawable/no_image_placeholder.xml from drawable resource ID #0x7f020080
   at android.content.res.Resources.loadDrawable(Resources.java:3376)
   at android.content.res.Resources.getDrawable(Resources.java:1872)
   at com.bumptech.glide.request.GenericRequest.getErrorDrawable(GenericRequest.java:409)
   at com.bumptech.glide.request.GenericRequest.setErrorPlaceholder(GenericRequest.java:399)
   at com.bumptech.glide.request.GenericRequest.onException(GenericRequest.java:548)
   at com.bumptech.glide.load.engine.EngineJob.handleExceptionOnMainThread(EngineJob.java:183)
   at com.bumptech.glide.load.engine.EngineJob.access$200(EngineJob.java:22)
   at com.bumptech.glide.load.engine.EngineJob$MainThreadCallback.handleMessage(EngineJob.java:204)
   at android.os.Handler.dispatchMessage(Handler.java:98)
   at android.os.Looper.loop(Looper.java:146)
   at android.app.ActivityThread.main(ActivityThread.java:5653)
   at java.lang.reflect.Method.invokeNative(Native Method)
   at java.lang.reflect.Method.invoke(Method.java:515)
   at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1291)
   at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1107)
   at dalvik.system.NativeStart.main(Native Method)
Caused by: org.xmlpull.v1.XmlPullParserException: Binary XML file line #2: invalid drawable tag vector
   at android.graphics.drawable.Drawable.createFromXmlInner(Drawable.java:986)
   at android.graphics.drawable.Drawable.createFromXml(Drawable.java:930)
   at android.content.res.Resources.loadDrawable(Resources.java:3372)
   at android.content.res.Resources.getDrawable(Resources.java:1872) 
   at com.bumptech.glide.request.GenericRequest.getErrorDrawable(GenericRequest.java:409) 
   at com.bumptech.glide.request.GenericRequest.setErrorPlaceholder(GenericRequest.java:399) 
   at com.bumptech.glide.request.GenericRequest.onException(GenericRequest.java:548) 
   at com.bumptech.glide.load.engine.EngineJob.handleExceptionOnMainThread(EngineJob.java:183) 
   at com.bumptech.glide.load.engine.EngineJob.access$200(EngineJob.java:22) 
   at com.bumptech.glide.load.engine.EngineJob$MainThreadCallback.handleMessage(EngineJob.java:204) 
   at android.os.Handler.dispatchMessage(Handler.java:98) 
   at android.os.Looper.loop(Looper.java:146) 
   at android.app.ActivityThread.main(ActivityThread.java:5653) 
   at java.lang.reflect.Method.invokeNative(Native Method) 
   at java.lang.reflect.Method.invoke(Method.java:515) 
   at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1291) 
   at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1107) 
   at dalvik.system.NativeStart.main(Native Method)

The problem is that GenericRequestgets resources from the application context, not the provided context. All the AppCompat classes (like AppCompatActivity) can return the VectorEnabledTintResourcesclass, which supports vector drawable. Unfortunately, there's no AppCompatApplication, so the Applicationclass always returns just plain Resourcesclass which doesn't support vector drawables before Android 5.0.

Glide Version:
3.7.0

Integration libraries:
okhttp3-integration:1.4.0

Device/Android Version:
GT-P5210 Android 4.4.2

Activity

TWiStErRob

TWiStErRob commented on Jun 14, 2016

@TWiStErRob
TWiStErRob
on Jun 14, 2016 · edited by TWiStErRob
Collaborator

A simple workaround would be to get the drawable yourself: .error(ContextCompat.getDrawable(getContext(), R.drawable.no_image_placeholder)).

Tip: you can save the request builder into a field and reuse it in an adapter to prevent continuously calling these (see Glide.with(...).from*).

TWiStErRob
added
bug
on Jun 14, 2016
TWiStErRob

TWiStErRob commented on Jun 14, 2016

@TWiStErRob
TWiStErRob
on Jun 14, 2016 · edited by TWiStErRob
Collaborator

I took a deeper look, checked the flow of Context from user to GenericRequest and it seems to me that if we are not paranoid in RequestManager the wrapped activity context would flow nicely to where you need it. The problem is that it would require a full analysis of other escape paths of that context to see if they're not kept once the Activity is destroyed.

You said there's no AppCompatApplication, but you could simply create your own. Wouldn't it "just work" if you copied AppCompatActivity.getResources method into your Application class?

@sjudd what's your take on this?

TWiStErRob
assigned
sjudd
on Jun 14, 2016
jakubkrolewski

jakubkrolewski commented on Jun 14, 2016

@jakubkrolewski
jakubkrolewski
on Jun 14, 2016 · edited by TWiStErRob
Author

The AppCompatApplication (full source below) is what I'm using right now, so the bug is no a blocker for me. However, the provided Contextcan be in fact some custom instance of ContextWrapper, so getting resources from it directly would be a more bulletproof solution.

/**
 * Based on {@link android.support.v7.app.AppCompatActivity}
 */
public class AppCompatApplication extends MultiDexApplication {

    private Resources resources;

    @Override
    public void onConfigurationChanged(Configuration newConfig) {
        super.onConfigurationChanged(newConfig);
        if (resources != null) {
            DisplayMetrics newMetrics = super.getResources().getDisplayMetrics();
            resources.updateConfiguration(newConfig, newMetrics);
        }
    }

    @Override
    public Resources getResources() {
        if (resources == null && VectorEnabledTintResources.shouldBeUsed()) {
            resources = new VectorEnabledTintResources(this, super.getResources());
        }
        return resources == null ? super.getResources() : resources;
    }
}
sjudd

sjudd commented on Jun 14, 2016

@sjudd
sjudd
on Jun 14, 2016
Collaborator

Themes are a sign we ought to use the provided context to load placeholders, not the application. We will have to be careful to avoid leaking activities unnecessarily, but that already can happen (if you use something like SimpleTarget) and we do try to guard against it by cancelling/restarting requests when activities and fragments are destroyed/re-created, so hopefully it won't be too bad.

sjudd
added
enhancement
on Aug 9, 2016
sjudd
added this to the 4.3 milestone on Oct 7, 2017
sjudd

sjudd commented on Oct 18, 2017

@sjudd
sjudd
on Oct 18, 2017
Collaborator

So I think I have a more complete fix for this in 4.3 which will allow automatic application of themes and handle custom ContextWrappers for error/fallback/placeholder. The original issue I think was actually fixed a while ago in bd9e265.

sjudd
added a commit that references this issue on Oct 18, 2017

Pass a non-application Context into Glide’s Requests.

bbb25af
sjudd
closed this as completedin bbb25afon Oct 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugenhancement

Type

No type

Projects

No projects

Milestone

Relationships

None yet

    Development

    No branches or pull requests

      Participants

      @sjudd@TWiStErRob@jakubkrolewski

      Issue actions
        Vector drawable can't be used as error drawable · Issue #1267 · bumptech/glide