TeamAmaze / AmazeFileManager

Material design file manager for Android
https://teamamaze.xyz
GNU General Public License v3.0
5.31k stars 1.57k forks source link

Dialog disappears after rotation #1794

Closed anlalalu closed 2 years ago

anlalalu commented 4 years ago

Describe the bug Dialog disappears after rotation.

To Reproduce Steps to reproduce the behavior:

  1. tap 'properties'
  2. tap 'Rename' and a dialog appears
  3. rotate the screen

Expected behavior The dialog still exists with no error log.

Screenshots If applicable, add screenshots to help explain your problem. Screen Shot

Smartphone (please complete the following information):

Additional context Add any other context about the problem here.

Stack Trace E/WindowManager: android.view.WindowLeaked: Activity com.amaze.filemanager.activities.MainActivity has leaked window com.android.internal.policy.impl.PhoneWindow$DecorView{52ad9290 V.E..... R....... 0,0-656,367} that was originally added here at android.view.ViewRootImpl.(ViewRootImpl.java:346) at android.view.WindowManagerGlobal.addView(WindowManagerGlobal.java:248) at android.view.WindowManagerImpl.addView(WindowManagerImpl.java:69) at android.app.Dialog.show(Dialog.java:286) at com.afollestad.materialdialogs.MaterialDialog.show(MaterialDialog.java:464) at com.afollestad.materialdialogs.MaterialDialog$Builder.show(MaterialDialog.java:2189) at com.amaze.filemanager.ui.dialogs.GeneralDialogCreation.showNameDialog(GeneralDialogCreation.java:172) at com.amaze.filemanager.fragments.MainFragment.rename(MainFragment.java:1202) at com.amaze.filemanager.ui.ItemPopupMenu.onMenuItemClick(ItemPopupMenu.java:100) at android.widget.PopupMenu.onMenuItemSelected(PopupMenu.java:201) at com.android.internal.view.menu.MenuBuilder.dispatchMenuItemSelected(MenuBuilder.java:741) at com.android.internal.view.menu.MenuItemImpl.invoke(MenuItemImpl.java:152) at com.android.internal.view.menu.MenuBuilder.performItemAction(MenuBuilder.java:884) at com.android.internal.view.menu.MenuBuilder.performItemAction(MenuBuilder.java:874) at com.android.internal.view.menu.MenuPopupHelper.onItemClick(MenuPopupHelper.java:177) at android.widget.AdapterView.performItemClick(AdapterView.java:299) at android.widget.AbsListView.performItemClick(AbsListView.java:1113) at android.widget.AbsListView$PerformClick.run(AbsListView.java:2911) at android.widget.AbsListView$3.run(AbsListView.java:3645) at android.os.Handler.handleCallback(Handler.java:733) at android.os.Handler.dispatchMessage(Handler.java:95) at android.os.Looper.loop(Looper.java:136) at android.app.ActivityThread.main(ActivityThread.java:5001) 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:785) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:601) at dalvik.system.NativeStart.main(Native Method)

anlalalu commented 4 years ago

Do you judge this as a defect? Maybe can be solved with reference to this: https://stackoverflow.com/questions/2850573/activity-has-leaked-window-that-was-originally-added.

TranceLove commented 4 years ago

Yes, symptom can be reproduced on Android emulators running 4.4.4 and 5.1.1 too.

TranceLove commented 4 years ago

The problem seems to be harder to fix than expected.

A better way as people suggest will be putting the dialogs inside DialogFragment like this, and this will cause the code to stink further, though I doubt if it's really necessary to port the 43 (found by searching MaterialDialog.Builder) dialogs into DialogFragments.

Thoughts? @EmmanuelMess @VishalNehra @bowiechen

EmmanuelMess commented 4 years ago

@TranceLove what is a leaked window and why is the window leaked?

Edit: it seems we are showing a dialog from a destroyed Activity.

Edit 2: Please remember the MainFragment is set to not be destroyed when the phone rotates, but the Activity does get destroyed.

VishalNehra commented 4 years ago

But some of the dialogs do have fragments, like SftpConnectDialog. We can try to fix this leak for them and verify. BTW I don't see a problem with migrating to fragments. Actually it'll be better code.

TranceLove commented 4 years ago

And looking over the code again, as many dialogs shares the same layouts, the conversion process may not be so horrible as it seems, depends on how much abstraction we can make.

EmmanuelMess commented 4 years ago

Having as class per dialog is a good idea IMO, as long as we don't have all in the same package.

VishalNehra commented 2 years ago

Dupe #1657