wordpress-mobile / WordPress-Android

WordPress for Android
http://android.wordpress.org
GNU General Public License v2.0
2.92k stars 1.3k forks source link

Crash: InflateException: Binary XML file line #26: Error inflating class org.wordpress.android.l... #9905

Closed sentry-io[bot] closed 3 years ago

sentry-io[bot] commented 4 years ago

Sentry Issue: https://sentry.io/share/issue/136cee38cb79491db456071ee0c686a0/

NotFoundException: Unable to find resource ID #0x7f0800bb
    at org.wordpress.android.util.widgets.WPTextInputLayout.<init>(WPTextInputLayout.java:20)
    at org.wordpress.android.login.widgets.WPLoginInputRow.init(WPLoginInputRow.java:67)
    at org.wordpress.android.login.widgets.WPLoginInputRow.<init>(WPLoginInputRow.java:58)
    at org.wordpress.android.login.LoginBaseFormFragment.createMainView(LoginBaseFormFragment.java:100)
    at org.wordpress.android.login.LoginBaseFormFragment.onCreateView(LoginBaseFormFragment.java:106)
...
(60 additional frame(s) were not displayed)

NotFoundException: Drawable (missing name) with resource ID #0x7f0800bb
NotFoundException: File res/drawable-v21/design_password_eye.xml from drawable resource ID #0x7f0800bd
    at org.wordpress.android.util.widgets.WPTextInputLayout.<init>(WPTextInputLayout.java:20)
    at org.wordpress.android.login.widgets.WPLoginInputRow.init(WPLoginInputRow.java:67)
    at org.wordpress.android.login.widgets.WPLoginInputRow.<init>(WPLoginInputRow.java:58)
    at org.wordpress.android.login.LoginBaseFormFragment.createMainView(LoginBaseFormFragment.java:100)
    at org.wordpress.android.login.LoginBaseFormFragment.onCreateView(LoginBaseFormFragment.java:106)
...
(49 additional frame(s) were not displayed)

NotFoundException: Drawable org.wordpress.android:drawable/design_password_eye with resource ID #0x7f0800bd
InvocationTargetException: None
    at org.wordpress.android.login.widgets.WPLoginInputRow.init(WPLoginInputRow.java:67)
    at org.wordpress.android.login.widgets.WPLoginInputRow.<init>(WPLoginInputRow.java:58)
    at org.wordpress.android.login.LoginBaseFormFragment.createMainView(LoginBaseFormFragment.java:100)
    at org.wordpress.android.login.LoginBaseFormFragment.onCreateView(LoginBaseFormFragment.java:106)
...
(37 additional frame(s) were not displayed)

InflateException: Binary XML file line #21: Error inflating class org.wordpress.android.util.widgets.WPTextInputLayout
InflateException: Binary XML file line #21: Binary XML file line #21: Error inflating class org.wordpress.android.util.widgets.WPTextInputLayout
InvocationTargetException: None
    at org.wordpress.android.login.LoginBaseFormFragment.createMainView(LoginBaseFormFragment.java:100)
    at org.wordpress.android.login.LoginBaseFormFragment.onCreateView(LoginBaseFormFragment.java:106)
...
(24 additional frame(s) were not displayed)

InflateException: Binary XML file line #26: Error inflating class org.wordpress.android.login.widgets.WPLoginInputRow
InflateException: Binary XML file line #26: Binary XML file line #26: Error inflating class org.wordpress.android.login.widgets.WPLoginInputRow

Binary XML file line #26: Binary XML file line #26: Error inflating class org.wordpress.android.login.widgets.WPLoginInputRow
designsimply commented 4 years ago

30-day impact: ~73 per day Users affected: 698 in the last 30d Last seen in: 12.7.1 (latest release at the time of this report)

https://sentry.io/share/issue/9eaadccac3be4c259943d72c46427792/

marecar3 commented 4 years ago

It seems that we are dealing with the same issue as WOOCOMMERCE is dealing with.

Thanks to @anitaa1990 and @0nko's comment: https://github.com/woocommerce/woocommerce-android/issues/1141 we saved some time here.

We need to make some decision what would be next steps in solving this issue.

Suggestion from the comment is to add this: https://developer.android.com/guide/app-bundle/sideload-check Into our application, in order to not allow users to install an app without google play.

elibud commented 4 years ago

I think @daniloercoli may have some insights too.

daniloercoli commented 4 years ago

Restrict to Google Play only devices is super sad.

Not sure if we can fix this issue here by adding run-time checks about the availability of Google Play Services, like it was done here https://github.com/wordpress-mobile/WordPress-Android/pull/9831

marecar3 commented 4 years ago

Summary

This one is really difficult to be resolved as it's hard to find a scenario which will crash the application.

Based on the latest reports from the sentry I am not sure that the cause of this problem is because our users have rooted devices.

Screen Shot 2019-07-22 at 5 44 49 PM

Both WPAndroid and WooComerce are using the same version of material-components -> https://github.com/material-components/material-components-android/releases/tag/1.0.0

Not sure if that is a potential problem, but as I can see the version 1.0 is using style to include drawables

Screen Shot 2019-07-22 at 5 57 53 PM

And newest version: 1.1.0-alpha08 is using quite a different approach.

https://github.com/material-components/material-components-android/blob/fa0be1485f6edb6dbd3f92e604a7ff15651a3f7d/lib/java/com/google/android/material/textfield/PasswordToggleEndIconDelegate.java#L81

Not sure if it's safe to use the newest library (regarding our UI) but maybe if we update it, it will affect to crashes disappear?

@daniloercoli @hypest @maxme @mzorz Any thoughts?

mzorz commented 4 years ago

Been following this interesting issue here!

In 1.1.0-alpha-104 they've added support for a custom end icon for the TextInputLayout (https://github.com/material-components/material-components-android/commit/56859415e88224b9fa039f641eaaf7953a699e5d) so my reasoning is: if the cause of the crash is the resource not being available for whatever cause, we can make sure it is available by packaging the eye resource ourselves and referencing that by using the custom end icon capability.

That said, the issues described about sideloading IIUC wouldn't be totally circumvented by this, but given the crashes are not caused by missing WP resources at all I'm pretty confident packaging the resource ourselves should help alleviate the problem.

We'd be targeting an alpha product that could have other consequences, but it may be worth a try. Wdyt @marecar3 ? Would you like to try that?

marecar3 commented 4 years ago

Hey @mzorz thanks for the information. I am on it.

designsimply commented 4 years ago

90-day impact: ~113 per day Users affected in the last 90 days: 2700 https://sentry.io/share/issue/9eaadccac3be4c259943d72c46427792/

Moving this back to the Prioritized Android list in the Groundskeeping project and noting that the open PR needs a review or a nudge.

designsimply commented 4 years ago

90-day impact: ~167 per day Users affected in the last 90 days: 3600 https://sentry.io/share/issue/9eaadccac3be4c259943d72c46427792/

designsimply commented 4 years ago

90-day impact: ~189 per day Users affected in the last 90 days: 3200 https://sentry.io/share/issue/9eaadccac3be4c259943d72c46427792/

designsimply commented 4 years ago

Users affected in the last 90 days: 2400 https://sentry.io/share/issue/9eaadccac3be4c259943d72c46427792/

The open PR for this needs follow-up.

designsimply commented 4 years ago

Users affected in the last 90 days: 1900 https://sentry.io/share/issue/9eaadccac3be4c259943d72c46427792/

(Also see internal reference: paqN3M-7F-p2)

designsimply commented 4 years ago

Users affected in the last 90 days: 1200 https://sentry.io/share/issue/9eaadccac3be4c259943d72c46427792/

elibud commented 4 years ago

Users affected in the last 90 days: 1200

Not good. @oguzkocer I believe you are in groundskeeping next week? Can you please take a look at this one?

oguzkocer commented 4 years ago

@elibud I'd be happy to check it out, however there is already a PR for it with quite a bit discussion. From what I understand, it looks like there is still an issue with AppCompat and we already reported the issue. Following up on that discussion with the people that are already involved may be the faster way to resolution. Having said that, if you do want me to take over, follow up etc, I'd be happy to do that as well.

Here is the PR in question: https://github.com/wordpress-mobile/WordPress-Android/pull/10279

elibud commented 4 years ago

Following up on that discussion with the people that are already involved may be the faster way to resolution.

Got it, thank you @oguzkocer. @khaykov @marecar3 any updates there?

marecar3 commented 4 years ago

Hey @elibud I have responded to the latest question from Google and also I have asked them if they have some proposal which can be used to overcome the issue, however, we didn't get an answer from their side. (Almost week ago the last email was sent)

marecar3 commented 4 years ago

Maybe we shouldn't wait for them 😕 @oguzkocer do you maybe how some other approach on your mind? This one isn't easy to reproduce so not sure what could be an alternative solution for it? Thanks.

oguzkocer commented 4 years ago

I have gone through the discussions across several issues and had a look at the changes in the PR. I can't help but feel like due to the confusing nature of the issue and uncertainty about the android material library update, we might have lost sight of the main issue. First of, here is my understanding of the situation:

Please correct me if I am wrong about any of these assumptions and ignore the rest of the comment in that case.


With these assumptions in mind here are my suggestions:

  1. Open a separate PR and set our own drawable and ship it as soon as possible. Our main priority should be to get rid of the crash which hopefully this change will accomplish. Even if it doesn't, it'll give us more clues about the issue. If it does fix the issue, one extra drawable is really nothing to be concerned about especially considering the time and energy spent on these issues so far.
  2. Open a new issue to update the material library or add a new comment in the existing PR. Summarize what the situation is and refer to both this issue and the original PR. Clearly state what the blocker is and what steps should be taken once the blocker is gone. These steps should include trying to revert the separate PR from step 1. We still don't want to leave an extra drawable in the code if it's no longer necessary.
  3. Close this issue. We can re-open it if the crash is not fixed yet.

Hope all this makes sense and let me know if you have any questions or concerns @elibud @marecar3.

marecar3 commented 4 years ago

Hey @oguzkocer,

Thanks for sharing your thoughts!

The main problem for me was how can we use our own drawable for a password as they are using their own drawable - which is crashing the app, in the constructor of the component :

https://github.com/material-components/material-components-android/blob/cd59e98f7e2185ddb075ff0fc91f29765d562968/lib/java/com/google/android/material/textfield/TextInputLayout.java#L344

@oguzkocer maybe I am missing something but I don't see which approach can be used in order to override the code from the constructor? Thanks!

oguzkocer commented 4 years ago

@marecar3 I might be missing something as well, but can we not do something like this:

<org.wordpress.android.util.widgets.WPTextInputLayout
    android:layout_centerVertical="true"
    app:theme="@style/LoginTheme.EditText"
    android:id="@+id/input_layout"
    android:layout_width="match_parent"
    android:layout_height="wrap_content"
    android:layout_toEndOf="@+id/icon"
    app:passwordToggleDrawable="drawable/selector_password_visibility"
    android:importantForAccessibility="no">

When I check the decompiled code for TextInputLayout I don't see the same constructor, but even in the version you linked, wouldn't this just use the drawable we give bypassing the missing resource? There is not a whole lot of documentation about R.styleable but my understanding is that it'd work like this. What do you think?

marecar3 commented 4 years ago

Hey, @oguzkocer thanks for the great suggestion!

Let's do it in that way and see what will happen. Here is the new PR: https://github.com/wordpress-mobile/WordPress-Android/pull/11172

cc: @elibud @designsimply

oguzkocer commented 4 years ago

Unfortunately the solution in #11172 did not work for all users. We have one user crashing multiple times in 14.1: https://sentry.io/share/issue/a7681d396e284f1990a5f3005f031b7b/

It's very early for 14.1, so we'll need to keep track to see if it's still a common issue or if the solution somewhat worked and now it's more isolated.

The device that crashed is itel it1508 with Android 5.1. I tried creating an emulator with the same resolution and Android version, but couldn't reproduce the issue.

cc @marecar3 @designsimply

marecar3 commented 4 years ago

Hey @oguzkocer,

Thanks for tracking this was. I was also wondering did we manage to solve it.

I saw this error in the stack trace: android.content.res.NotFoundException: File res/drawable/selector_password_visibility.xml from drawable resource ID #0x7f08025f

This means that the app can't find a selector that was created in a recent PR fix. Not sure why it couldn't find the resource, I will google it.

marecar3 commented 4 years ago

It seems that users can also experience a side effect of the fix.

I assume that there is also a case when Android can't find a drawable reference and somehow it connects other drawable instead.

Here is the issue: https://github.com/wordpress-mobile/WordPress-Android/issues/11307

oguzkocer commented 4 years ago

Thanks for looking into this again @marecar3! The side effect is pretty weird, I honestly don't understand what's going on with these drawables in certain phones. Also, the fact that it's happening for a password field is making me feel uneasy.

designsimply commented 4 years ago

Zero events have been tracked for this crash in 14.6.1 since it was released 9d ago on Apr 22.

~~Number of users affected in the last 90d: 534 https://sentry.io/share/issue/06479f2a38234c8abad668f173724b9f/~~

Number of users affected in the last 90d from May 19: 384 https://sentry.io/share/issue/9eaadccac3be4c259943d72c46427792/

@oguzkocer the latest version we're seeing this crash happen for in the last 90d is 14.0, which makes me feel it's no longer happening in current releases, but is it still worth keeping the issue open because it's related to the password field (which you mentioned in your latest comment here back in Feb)?

oguzkocer commented 4 years ago

@designsimply The issue number you used for that query is incorrect and there has been 10 events in 14.6.1. You can also see this information in the tags section of the event and verify that it has been happening in pretty much every release.

designsimply commented 3 years ago

@oguzkocer thanks for catching that! :bow:

On a closer look, I see that I got the event number correct for the 14.6.1 query but used the wrong link for the number of users affected in the last 90d.

I fixed the error in the comment above, and here's another look at the numbers drilling into the 14.7 release:

Zero events have been tracked for this crash in 14.7 since it was released 15d ago on May 4.

Events in the last 90d: 23,000 Users affected in the last 90d: 384 Limited to: WPAndroid 14.0 and below https://sentry.io/share/issue/9eaadccac3be4c259943d72c46427792/

The total number of issues is still high, but if you look at the Tags section of the Sentry issue, you can see that it's only happening to version 14.0 and below of the WP Android app. In this case do you think it's okay to close or should we look into trying to resolve this for older versions as well? Note that the number of users affected is probably in the middle for the crashes we're tracking for WPAndroid in GitHub at the moment, which would lead me to prioritize this as medium. wdyt?

oguzkocer commented 3 years ago

@designsimply That sounds reasonable and when in doubt I almost always prefer to resolve the crash in Sentry since it'll just come back as regression if it's not actually fixed. However, I believe we have several issues for this crash in Sentry, so let me look through them in my next crash monitoring session to just verify that it's not happening in the newer releases. If that's the case, I'll close this and resolve the crashes in Sentry.

oguzkocer commented 3 years ago

As mentioned in my previous comment, we have a lot of different issues in Sentry for this same crash and unfortunately some of it seems to be still happening in the latest releases. I've merged relevant issues and there seems to be a significant downward trend in the number of crashes in the latest releases, but somehow some of these crashes are ending up in the crash reported in https://github.com/wordpress-mobile/WordPress-Android/issues/11966 which is a completely unrelated issue. So working on this issue right now is tricky and the impact of it is unclear. My proposed plan of action is:

If we are not planning to work on this issue in the next couple weeks, which I don't think we should because the numbers are kind of low in Sentry and it's hard to fix this crash, then I think this plan would work. If we want to fix the issue as soon as possible, I think whoever is working on this needs to do a little bit of investigation which I'd be happy to help with.

cc @designsimply @yaelirub (as triager for the week, I think you might want to know that the linked issue is related)

marecar3 commented 3 years ago

Hey @oguzkocer @khaykov @designsimply 👋 I have opened a new PR which we did have in the past but we didn't merge it because of the material library version. Now that we have good version of the material library, maybe we can try with this fix?

https://github.com/wordpress-mobile/WordPress-Android/pull/12000/

grndvl1 commented 9 months ago

I am wondering as I am getting the same crash only periodically by users that we have 100K users per day, we maybe get only 1 per day. Only thing I see is that the line number in our code for XML points to "android:importantForAccessibility="no" on the image file. Could it be something with that attribute?