Crash in block editor on double rotation when inserter is open #10227
Comments
hypest
changed the title
|
Reproduced the crash on a very old version of the app which had GB mobile in it (alpha-150). |
|
The first rotation does already show a problem: and then the crash happens on the 2nd rotation |
|
Another flow that reproduces the problem:
|
|
Testing on a Xiaomi Mi A2 Lite: It's interesting that rotating the device shows the keyboard even when there are no blocks selected. Might it be a conflict between however the modal works on Android and the Keyboard? |
|
I have discovered the source of this crash, as well as the leaked context (great observation regarding the leak @daniloercoli 👍 ) There is a fix for the crash here: #10383. @etoledom you are quite right about the strange keyboard behavior, and I suspect is has something to do with focus behavior during the initialization of the editor on the second go-around. This may also be related to the strange behavior described here: #10383 (comment) after the fix is applied. |
|
I just verified that this issue seems isolated to newest Android version, Pie/ API 28 /v9.0. See devices/builds tested (from related gutenberg-mobile issue 1333:
Video of it not crashing using Galaxy Tab 3 running API 26: @mkevins The fact that the crash seems isolated to API 28 may, or may not be helpful for #10383. Android Pie Behavior changes do mention some rotation changes, but a first glance didn't look promising for that being the root cause. Here's the link just in case - https://developer.android.com/about/versions/pie/android-9.0-changes-all#screen-rotation-changes |
|
I'm working on this issue now. I'm working off of @mkevins insight and some new details listed in #10383 and gb-mobile issue 1333 |
Nice catch, @cameronvoell ! If nothing stands out in the Android code, I wonder whether Do you know whether the leaked context was occurring on the older API versions? That may give a clue to the underlying cause as well, particularly if life-cycle events are somehow handled differently in the newer API. |
|
If the bug is on React Native side, it might be worth it to check: https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/views/modal/ReactModalHostView.java |
@mkevins So testing on PRE API 28, there is no leaked context error message in the Android Logcat log. On API 28 I am seeing the same messages @daniloercoli reported in the previous comment. There is a message however in the Javascript logs referencing memory leaks. However this message shows before and after your fix (#10383) is implemented on API PRE 28 and on API 28. So it does not cause the crash alone. But it could be related
@etoledom So debugging the two apps, one running on API 28 and one pre 28, the clearest divergence I see happening does seem to be within the ReactModalHostView here. That line runs fine on pre API 28, but it is crashing on API 28. Unfortunately, I think this is caused by some race condition between the WindowManager, the restarting Activity, and the ReactModalHostView and its Dialog object. There are hints that ReactModalHostView has had similar issues in the past in React Native update notes, one of which points to this fix, which has the same error message that we are seeing (java.lang.IllegalArgumentException): facebook/react-native@e360b0b. I think there is still a good chance that we could fix the behavior of API 28 so that it behaves like the non crashing, modal preserving Pre-API 28 behavior shown in my GIF above. However it is hard to estimate the time for a fix since it could be an issue with the React Native modal, or somehow related to how we are updating context, and references to activities within our "headless" retained fragment setup, containing our React Native Root view. #10383 could be a good short term fix for at least not crashing. This brings me to my last point:
There seems to be some existing issues in the editor that show the wrong blocks having focus on rotate, that is actually unrelated to Modals. I've uploaded a GIF in a separate issue here. Even with no modal open, rotating the phone can sometimes select an incorrect text block and then open a keyboard over the screen unexpectedly. This in my opinion is just as bad or worse of an issue than this one, since it does not require a modal to be open, and it would be quite jarring for a user. Fixing scroll position and caret position was documented and fixed earlier according to this #8739, so I think incorrect focus of blocks on rotate could probably be debugged as a regression, and I'd welcome any advice on when the team thinks the bad behavior could have been introduced. Suggested Steps:
|
|
Hi @cameronvoell 👋
Thanks for checking that. I think this suggests a difference (pre/post API 28) in how the context is handled (perhaps either by RN itself, or the
IIRC, this isn't always Android specific (but it can be), and in this case seems related to the block toolbar, so I wouldn't think it's related to the modal.
Some more info on that: When the inserter menu is closed, it usually hides the "insertion point" here: https://github.com/WordPress/gutenberg/blob/master/packages/block-editor/src/components/inserter/menu.native.js#L60 . So, this is either not being called, or somehow the saved state of the app is causing it to be "un-hidden" somehow when the activity is recreated. I haven't investigated this deeply, so I'm only noting this here for now in case it's useful. |
|
Came down here tracking another issue - this crash is being tracked in Sentry, adding that information here: Comes from #9494 (comment) 90-day impact: ~16 per day |
|
90-day impact: ~19 per day |
|
Users affected in the last 90 days: 768 |
|
Tested and confirmed using WPAndroid 13.8-rc-1 on Pixel 3 Android 10. Steps to reproduce:
Result: the editor closes automatically if you rotate the device while the add image BottomSheet is open. (15s) Click to expand app logs.
|
|
Users affected in the last 90 days: 734 |
|
Users affected in the last 90 days: 782 |
Steps to reproduce the behavior
Tested on [device], Android [version], WPAndroid [version]
Google Pixel 2XL, Android 9, WPAndroid v12.9-rc-1 (though is probably happening in older versions too)
The text was updated successfully, but these errors were encountered: