bug: Ionic5.01, Android 6~8, modalController works strange when two modals open togather.

[netsesame2]

I test it in emulator, android 6,7,8, all of them reproduce the following issues. These issues doesn't exist in ionic 4.4.

Steps:

1. create modal A, present it.

   let modal = await this.modalCtl.create({
   component: modalAClass
   ....
   });
   modal.present();

2. in modal A, create modal B.

   let modal = await this.modalCtl.create({
   component: modalBClass
   ....
   });
   modal.present();

3. in modal B, dismiss itself.

   this.modalCtl.dismiss();

4. then you will see, both modal A and B are dismissed in the UI.
5. and the page below can't be interactive anymore.
6. inspect DOM in devTools, you will see modal A is still there. And more strange is, you call interact with the buttons in modal A, although you can't see it.

To find more, I try another test. Steps:

1. create modal A, present it.

   let modal = await this.modalCtl.create({
   component: modalAClass
   ....
   });
   modal.present();

2. in modal A, close A, and then create modal B.

   this.modalCtl.getTop().dismiss();
   let modal = await this.modalCtl.create({
   component: modalBClass
   ....
   });
   modal.present();

3. then you will see, both modal A and B are dismissed in the UI
4. and the page below can't be interactive anymore.
5. inspect DOM in devTools, you will see modal B is still there. And as above, you call interact with the buttons in modal B, although you can't see it.
6. Workaround: add await before modalA dismiss, modal B will works. But, you will wait until modal A close before modalB popup, which is not a good UI expirence.

Envirorment: Ionic: Ionic CLI : 6.1.0 Ionic Framework : @ionic/angular 5.0.1 @angular-devkit/build-angular : 0.900.3 @angular-devkit/schematics : 9.0.3 @angular/cli : 9.0.3 @ionic/angular-toolkit : 2.1.2

Capacitor: Capacitor CLI : 1.5.0 @capacitor/core : 1.5.0

[NikolaPeevski]

@netsesame2 When dealing with multiple modal windows at once it is a good idea to assign an ID to each modal. The modalController is a bit funky and my experience it works well with only one modal at a time but with multiples you have to pass on a modal ID through the modal options as I have done here https://github.com/NikolaPeevski/ionic-modal-service/blob/master/src/app/shared/services/modal.service.ts#L21 Can you try to assign IDs to each modal and respectively when interacting with them passing on the ID for your desired modal ? Also comment what is the outcome and if the issue still persists.

[netsesame2]

@NikolaPeevski Thanks for your workaround. I tried it by adding an unique id to both modals, there is no lucky happened. In fact, I check the source code, modalCtroller will add a default id to each modal if you don't offer.

By digging the source code, I don't think the id is the reason. When dismiss, it find the modal layer in a global layers, and then call its dismiss method. Really don't know what happened that it close all the layer unrelated.

[NikolaPeevski]

Did you try target dismissal by id @netsesame2 ?

[netsesame2]

@NikolaPeevski Yes, I did so, still no lucky.

It seems that, the animation on dismiss affect the other modals, for that, modal A. I can see the DOM of modal A alive there.

[NikolaPeevski]

@netsesame2 Can you provide a repo for reproduction ? I don't have much time to setup a repo with the steps you explained above right now.

[netsesame2]

@NikolaPeevski I create a repo: https://github.com/netsesame2/demo.git

You can test it in Emulator, android 8.1.

[liamdebeasi]

Thanks for the issue. Can you try this on a physical device? We do not recommend testing on the emulator because they ship with very outdated versions of the webview. (Note: You can update the webview on emulators with Android 7+ but you need to do it manually I believe).

[netsesame2]

@liamdebeasi Sorry for delay responsing.

I did try to test on a physical device, android 7.1.1, chrome 55.0.2883.91. The above issue is reproduced.

[ASDAlexey]

I've got same issue to. On Pixel 2 ad Pixel XL with android 10. It's issue (@ionic/angular 5.0.4)

[kdfemi]

I am also facing the same issue, it works fine on my Pc browser but when i test it on a mobile device i get the same weird behaviour

[TheLazyHatGuy]

I have the same problem with two devices running android 8.1 and 6, although it is not a problem on my OnePlus 7 running android 10 Using this line for dismissing makes no difference

  async dismissModal() {
    await this.modalController.dismiss(null, 'backdrop', 'reset-password');
  }

[TheLazyHatGuy]

I have a good idea of what causes this bug. It is an outdated Webview. At the moment I can confirm that WebView versions older than 71 have the bug and versions newer than 80 do not have the modal dismiss bug.

I have made an app to help test for the modal dismiss bug - https://play.google.com/store/apps/details?id=com.sanitynotfound.modaltest It will give you a link to update your webview if your webview is older than 80

[netsesame2]

@TheLazyHatGuy

Thanks for your idea. To update to a newer webview version is always the best choice. However, the google update is not avaliable to devices with some OEM android installed.

I found the issues only in ionic 5.0x, but not when run in ionic 4.11.x. So it seems that there is no more relations with the webview itself.

[milkmusket]

Just commenting that I also have this issue, implemented the workaround by @NikolaPeevski and it didn't solve my issue either. Testing on Nexus 5X with android 10.

[wiscont-test]

Just confirm that I also found the issue on my android 7 device. Everything works well on PC, but fails when it comes to physical devices. Dismissing the child modal page makes its parent become invisible. @netsesame2 have you got any workaround?

PS: There is no problem on IOS devices.

Update: I haven't fully tested it, but if there are only two modals got chained (say ModalA -> ModalB), things seem to work if ModalA is in 'ios' mode while ModalB is not.

For example in the root page create modalA in ios mode:

    const modalA = await this.ModalController.create({
      component: ModalAPage,
      mode: 'ios'
      ......
    });
    await modalA.present();

and then within modalA create a non-ios-mode modalB:

    const modalB = await this.ModalController.create({
      component: ModalBPage,
      ......
    });
    await modalB.present();

Now dismissing modalB on Android devices doesn't hide modalA anymore, at least in my case.

However for nested layers that have three modals or more, this trick doesn't work anymore.

[WintonLi]

Finally I was able to identify the root cause of the issue. This is actually a bug that may have a wider impact than on modal pages on Android devices.

The Problem

Modal elements (and some other elements) share animation stylesheets that are created dynamically by createKeyframeStylesheet() in core/src/utils/animation/animation-utils.ts. These stylesheets are necessary for modals to display.

When a modal is destroyed, all associating animation stylesheets will be removed from DOM by the cleanUpStyleSheets() method in ionic/core/src/utils/animation/animation.ts, and the current method simply performs removal without checking whether the stylesheets are still in use by other elements.

It happens that when multiple modals are created, one on top of another, they have identically animating behaviour, and therefore they share the same set of animation stylesheets. When one is dismissed (probably the last opened one), all these stylesheets are gone, leaving the rest modals displayed improperly - in most cases, they become invisible.

Solution

I have tried to solve the problem by creating a fork: https://github.com/WintonLi/ionic/tree/fix-chained-modals where a counter has been used to record the number of modal elements that are using a stylesheet. The stylesheet will never be removed unless its counter reaches zero.

These two functions has been modified:

1. createKeyframeStylesheet() in core/src/utils/animation/animation-utils.ts - a counter will be initialized to 1 whenever a stylesheet is created, and the counter will be increased by one whenever a new use is detected.
2. cleanUpStyleSheets() in ionic/core/src/utils/animation/animation.ts - it decreases the counter when called, and removes the stylesheet if the counter becomes zero.

Tests

I am now testing it manually in the following environment and everything works fine:

Ionic CLI : 5.4.6 @ionic/angular 5.0.7 @angular-devkit/build-angular : 0.901.0-rc.0 @angular-devkit/schematics : 9.1.0-rc.0 @angular/cli : 9.1.0-rc.0 @ionic/angular-toolkit : 2.1.2

@capacitor/android: 1.5.1 @capacitor/core : 1.5.1 @capacitor/cli : 1.5.1

Pull request

@liamdebeasi If needed, I am happy to create a pull request for the fix. Please note that this bug may affect not only modals. Without the fix, I observed some shared animation stylesheets for non-modal elements got randomly removed, which may cause other problems. I would suggest fixing it as soon as possible.

For those who suffer much from the bug (like myself...) and cannot wait until the fix comes out, you can try the fork I created to build your local version of ionic. Building instructions can be found in the official repo, but I am happy to help. Please note that I cannot guarantee anything and I would recommend against using it for production.

[netsesame2]

@WintonLi You dig out an important issue to us.

[WintonLi]

I was doing this because I have more than 20 nested modal pages in my app, and changing all of their routing and data sharing to avoid this bug is really a pain. I am curious to see how many people will complain about this issue - it's quite natural to use a modal on top of another isn't it?

[liamdebeasi]

@WintonLi Thanks for the detailed response! My concern with this approach is that it does not account for elements that have been removed from the DOM. For example, when you close the ion-modal, the element itself is removed from the DOM. As a result, the "close" animation styles are never removed from the DOM since we do not explicitly destroy the animation object.

Also to clarify, this issue only affects devices that do not support Web Animations. As of now, most browsers support Web Animations so the relative impact should be fairly low.

I will look into a fix for this and will provide an update here when I have more to share. Thanks!

[WintonLi]

@liamdebeasi Thank you so much for your comments. It's always better to have the Ionic team to get bug fixed.

The stylesheets will be removed, as can be seen from my debugger. The logic is whenever a modal is removed from the DOM, cleanUpStyleSheets() will be called, the counter associated with the stylesheet will be decreased, and we will check whether the modal being destroyed is the last one that's using the stylesheet. If it is, the stylesheet will be removed. If an animation stylesheet is in the DOM, it must has a counter of no less than 1, meaning at least one modal (or something) is still using the stylesheet. It's sort of like the link count pattern used in Linux like file systems.

With such approach, I did observe that there are 3 or 4 animation stylesheets that persist in the DOM with ever-increasing counters, which means they have been attempted to create regularly. It's reasonable to keep them in the DOM because they are used frequently. Of course this is based on my limited understanding of Ionic core. Perhaps the remaining stylesheet can lead to other conflicts that I don't realise.

About the impact... it depends. There are still quite a lot of people using Android 7 or even earlier version mobiles (more than 60% in our case because our app focus on unskilled/ elder people) and we have to consider their experience.

Finally thanks again for your attention. Let's together make Ionic better.

[liamdebeasi]

@WintonLi Yeah there is definitely a trade off between keeping stylesheets in the DOM and constantly cleaning them up. In this case, I would prefer to clean them up even if they are used frequently because the framework has no way of knowing ahead of time what the usage will be like.

One solution could just be to prevent animations from sharing stylesheets. It was an optimization we made, but if it is causing too many issues it may not be worth it.

[liamdebeasi]

Can everyone try the following dev build and let me know if it resolves the issue?

Ionic Angular npm i @ionic/angular@5.1.0-dev.202004011618.3123a31

Ionic React npm i @ionic/react@5.1.0-dev.202004011618.3123a31

[WintonLi]

@liamdebeasi Thanks for this. I will give it a try tomorrow morning (I would have done it already if I hadn't forgotten my mobile in the office...)

[dragos-atanasoae]

@liamdebeasi You save my day, I tried your solution and the multiple modals problem is solved 🥳

Can everyone try the following dev build and let me know if it resolves the issue?

Ionic Angular
`npm i @ionic/angular@5.1.0-dev.202004011618.3123a31`

Ionic React
`npm i @ionic/react@5.1.0-dev.202004011618.3123a31`

[WintonLi]

@liamdebeasi Thanks for the dev build. It works very well on my Android.

PS: I agree that the animation performance can be improved with shared stylesheets, and really hope this feature can be added again in the future.

[liamdebeasi]

@WintonLi I was able to preserve the shared stylesheet functionality. I just changed it so the stylesheets are not cleaned up by default.

[liamdebeasi]

Thanks for the issue. This has been resolved via https://github.com/ionic-team/ionic/pull/20940 and will be available in an upcoming release of Ionic Framework.

[ionitron-bot[bot]]

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.