Fixed NPE in LoginUsernamePasswordFragment #10833

khaykov · 2019-11-22T07:24:24Z

After the user leaves LoginUsernamePasswordFragment for more than one hour and comes back, a global site refresh is triggered, which causes onSiteChanged in LoginUsernamePasswordFragment to propagate false "successful" login event, which causes a crash down the line.

To make the testing easier, change the throttle delay on-site refresh (here) to something reasonable, like 10 seconds.

To test:

Navigate to Site Picker and press the + icon to add new site.
Chose "Add self-hosted site" option.
In the Site address field input self-hosted site URL and press NEXT.
While on the screen with login and password, press the home button and minimize the app.
Wait 10 seconds (or whatever time you chouse for site fetch throttle.
Navigate back to the app, and notice that it's not crashing.

Part 2:

Input wrong login or password and press NEXT to produce an error.
Press the home button and minimize the app.
Navigate back to the app and input correct login credentials.
Make sure you can login and the site is added.

PR submission checklist:

I have considered adding unit tests where possible.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

peril-wordpress-mobile · 2019-11-22T07:24:30Z

	Messages
📖	This PR contains changes in the subtree `libs/login/`. It is your responsibility to ensure these changes are merged back into `wordpress-mobile/WordPress-Login-Flow-Android`. Follow these handy steps! WARNING: Make sure your git version is 2.19.x or lower - there is currently a bug in later versions that will corrupt the subtree history! `cd WordPress-Android` `git checkout issue/10832-fix-npe-in-login-fragment` `git subtree push --prefix=libs/login/ https://github.com/wordpress-mobile/WordPress-Login-Flow-Android.git merge/WordPress-Android/10833` Browse to https://github.com/wordpress-mobile/WordPress-Login-Flow-Android/pull/new/merge/WordPress-Android/10833 and open a new PR.

Generated by 🚫 dangerJS

peril-wordpress-mobile · 2019-11-22T07:29:53Z

You can test the changes on this Pull Request by downloading the APK here.

malinajirka

Great investigation @khaykov !! I've tried to carefully review and test the code and AFAICT it works like a charm. However, tbh it's quite complicated and since it's a vital part of the app, I'd feel more comfortable if one more developer reviewed it.
cc @planarvoid @develric

I'd also suggest keeping an eye on "failed login" tracks to make sure we didn't break anything.

planarvoid · 2019-11-25T13:50:45Z

...dPressLoginFlow/src/main/java/org/wordpress/android/login/LoginUsernamePasswordFragment.java

        super.onSaveInstanceState(outState);

        outState.putBoolean(KEY_LOGIN_FINISHED, mLoginFinished);
+        outState.putBoolean(KEY_LOGIN_FINISHED, mLoginStarted);


I think the key here is wrong, it should be KEY_LOGIN_STARTED

:D How come I missed this. Great catch !

jkmassel · 2019-12-03T06:32:35Z

We're freezing 13.8 today, so I'm moving this forward to 13.9. If you'd like it released as part of 13.8, please adjust the target from develop to release/13.8, and let me know once this PR has been merged – I'll be happy to ship a new release candidate!

planarvoid

There is one mistake when saving the state 👍

khaykov · 2019-12-11T01:53:40Z

I fixed the issue, so this one is ready for another round :)

planarvoid

thanks for the change 👍, it works well

sentry · 2019-12-19T09:36:18Z

Sentry issue: WORDPRESS-ANDROID-3C

cfc0675d21 Add title to the layout override for the site help dialog 63fc73efa0 Use wrap_content for image width so wider images will scale better 68b0c58437 Merge commit '8487e6f4cfee09193f48b803fcf1eb2252355022' into darkmode/login-theme-1 c826be2ccb Merge pull request #35 from wordpress-mobile/feature/login-style-changes-v2 1f6c6e4bca Gutenberg/integrate release 1.25.0 with dark mode (#11580) 1d668f54a5 Merge pull request #11537 from wordpress-mobile/fix/10930-email-error-dissapears-on-rotation fd6665242f store the res id instead of the boolean so it supports multiple errors. 858decb8b1 utilized runnable that's posted when the UI has been drawn. ab37113dbf removed clearing in text watcher. 3691e848b8 Fixed config change issues. fd0c8c97e8 Merge pull request #34 from wordpress-mobile/merge/WordPress-Android/11492 c134376ee6 Merge commit 'e80a69322fe65ad994bee1854a2343c2089af323' into issue/fix-login-subtree 526919648d Ignore onDiscoverySucceeded events if LoginBaseDiscoveryFragment is detached 775f096826 Revert "Feature/material theme and Dark Theme support (#11469)" (#11486) 65d5c8f67b Feature/material theme and Dark Theme support (#11469) e80a69322f Merge pull request #33 from wordpress-mobile/merge/WordPress-Android/11051 40824a4333 Merge commit '0c3930794ed0c77e4926d334674e85263ef2a651' into update_login_lib_with_signup_sheet_nav_bar_buttons_visibility_fix fec863d2f8 Resolved merge conflict by removing SignupBottomSheetDialog.java file. f2d8c102ad Merge branch 'develop' into issue/10908-navigation-bar-buttons-not-visible-with-signup-sheet 7e46000166 Fix validation in input of Email fccc72b9a4 Merge pull request #32 from wordpress-mobile/merge/WordPress-Android/11172 d634c91e9f Merge commit '371f14160a780fbd7797d71921859d0fee5764d5' into update-login-lib-with-password-toggle-fix cd81dd1ce9 Add custom selector for password button 8f444b2d13 Merge branch 'issue/10908-navigation-bar-buttons-not-visible-with-signup-sheet' of https://github.com/wordpress-mobile/WordPress-Android into issue/10908-navigation-bar-buttons-not-visible-with-signup-sheet b53dda144c Add null check for design_bottom_sheet layout 521d09f42c Update libs/login/WordPressLoginFlow/src/main/java/org/wordpress/android/login/widgets/WPBottomSheetDialogFragment.java 7178f5c123 Show full width navigation bar and restrict max width for large screens 8a768d3dd2 Partial fix for terms of service announcement bering read twice. 989fb1d5a2 Remove empty line after brace 2d5d733080 Extract parts of code from onCreateView to onViewCreated a24d5289ab Eliminate setRetainInstance(true) 2e71a00135 Remove dialog.setOnDismissListener cfff7a060d Avoid using a parameter in Fragment's constructor 8b09b2d836 Convert BottomSheetDialog to BottomSheetDialogFragment and update styles a87d595f7a Merge pull request #1796 from woocommerce/release/merge-3.3 f18c390619 Fix signup bottom sheet navigation bar buttons not visible issue 4f4657fb64 Add requested changes e407a7f304 Revert erroneously deleted code during merge 821d29b90b Revert erroneously deleted code during merge 188cc0ee64 Check if view is null before accessing property 1a4414eec7 Merge commit 'bd659986940153549bfc26a8f6a4104bf748fe3c' into hotfix/1778-npe d2b8135e1e Added logic to fetch the correct SiteModel from the local db for the incoming url d80e8e616d Remove whitespaces b18703520d Issue/10930 email address error is preserved on rotation 3421ca9534 Merge branch 'develop' into test a20471e8c1 Merge branch 'develop' of https://github.com/woocommerce/woocommerce-android into feature/refund-by-items-master 7757f052b7 Merge branch 'develop' into feature/edit-product-master f869be0bfc Merge branch 'feature/refund-by-items-master' into 0nko/refund-items-list ea896f6172 Merge pull request #10833 from wordpress-mobile/issue/10832-fix-npe-in-login-fragment 64678168e4 check if listener is null before handling discovery error 1de03a5522 check if listener is null before handling discovery error a6e0946af0 Update gradle plugin to version 3.5.3 a23213aa25 Fixed wrong key variable c89899f001 Updated Glide version to 4.10.0 in the login module 2dd431ccdd Updated Glide version in the login lib e3211148cd Merge pull request #10787 from wordpress-mobile/anitaa/woo-login-changes 00f6bd2e62 Added flag that checks if login has started. d5e8683cf6 Added period to login with site credentials eee4c53fa7 Fixed bug when the forgot password url in username password screen was not valid 2810b14e28 Upgrade gradle plugin to version 3.5.2 daf7384622 Merge pull request #1552 from woocommerce/login-lib-changes f75cbaaacd Merge commit '4e588147352f8fe88f1417a03a00114dd7d51640' into login-lib-changes b2d28c652d Add signup flow name parameter to signup auth email request git-subtree-dir: libs/login git-subtree-split: cfc0675d2167b938369cf1896083453a593e9b8e

…n-login-fragment Fixed NPE in LoginUsernamePasswordFragment

Added flag that checks if login has started.

6a5c323

khaykov added [Type] Bug [Type] Crash labels Nov 22, 2019

khaykov added this to the 13.8 milestone Nov 22, 2019

khaykov changed the title ~~Added flag that checks if login has started.~~ Fixed NPE in LoginUsernamePasswordFragment Nov 22, 2019

khaykov requested review from develric, malinajirka and planarvoid November 22, 2019 07:24

malinajirka self-assigned this Nov 23, 2019

malinajirka approved these changes Nov 23, 2019

View reviewed changes

malinajirka removed their assignment Nov 23, 2019

planarvoid reviewed Nov 25, 2019

View reviewed changes

jkmassel modified the milestones: 13.8, 13.9 Dec 3, 2019

planarvoid requested changes Dec 3, 2019

View reviewed changes

Fixed wrong key variable

dcda5be

planarvoid approved these changes Dec 11, 2019

View reviewed changes

planarvoid merged commit e2e6e29 into develop Dec 11, 2019

planarvoid deleted the issue/10832-fix-npe-in-login-fragment branch December 11, 2019 12:53

This was referenced Jan 27, 2020

Pull the latest changes from the LoginLibrary woocommerce/woocommerce-android#1853

Merged

Merge/word press android/11172 wordpress-mobile/WordPress-Login-Flow-Android#32

Merged

wzieba pushed a commit that referenced this pull request Sep 27, 2024

Merge pull request #10833 from wordpress-mobile/issue/10832-fix-npe-i…

b65502b

…n-login-fragment Fixed NPE in LoginUsernamePasswordFragment

wzieba pushed a commit that referenced this pull request Oct 21, 2024

Merge pull request #10833 from wordpress-mobile/issue/10832-fix-npe-i…

e6a2491

…n-login-fragment Fixed NPE in LoginUsernamePasswordFragment

wzieba pushed a commit that referenced this pull request Oct 21, 2024

Merge pull request #10833 from wordpress-mobile/issue/10832-fix-npe-i…

0fb73bd

…n-login-fragment Fixed NPE in LoginUsernamePasswordFragment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fixed NPE in LoginUsernamePasswordFragment #10833

Fixed NPE in LoginUsernamePasswordFragment #10833

Uh oh!

khaykov commented Nov 22, 2019 •

edited

Loading

Uh oh!

peril-wordpress-mobile bot commented Nov 22, 2019

Uh oh!

peril-wordpress-mobile bot commented Nov 22, 2019 •

edited

Loading

Uh oh!

malinajirka left a comment

Uh oh!

planarvoid Nov 25, 2019

Uh oh!

malinajirka Nov 25, 2019 •

edited

Loading

Uh oh!

jkmassel commented Dec 3, 2019

Uh oh!

planarvoid left a comment

Uh oh!

khaykov commented Dec 11, 2019

Uh oh!

planarvoid left a comment

Uh oh!

sentry bot commented Dec 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Fixed NPE in LoginUsernamePasswordFragment #10833

Fixed NPE in LoginUsernamePasswordFragment #10833

Uh oh!

Conversation

khaykov commented Nov 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peril-wordpress-mobile bot commented Nov 22, 2019

Uh oh!

peril-wordpress-mobile bot commented Nov 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

malinajirka left a comment

Choose a reason for hiding this comment

Uh oh!

planarvoid Nov 25, 2019

Choose a reason for hiding this comment

Uh oh!

malinajirka Nov 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkmassel commented Dec 3, 2019

Uh oh!

planarvoid left a comment

Choose a reason for hiding this comment

Uh oh!

khaykov commented Dec 11, 2019

Uh oh!

planarvoid left a comment

Choose a reason for hiding this comment

Uh oh!

sentry bot commented Dec 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

khaykov commented Nov 22, 2019 •

edited

Loading

peril-wordpress-mobile bot commented Nov 22, 2019 •

edited

Loading

malinajirka Nov 25, 2019 •

edited

Loading