mirror of
https://github.com/ionic-team/ionic-framework.git
synced 2026-03-13 10:22:08 +08:00
fix(modal): prevent browser hang when using ModalController in Angular (#30845)
Issue number: resolves internal --------- ## What is the current behavior? When using ModalController to present a modal in Angular applications, the browser becomes non-responsive and hangs in some circumstances. This regression was introduced in #30544 with the addition of a MutationObserver that watches document.body with subtree: true to detect when a modal's parent element is removed from the DOM. For controller-based modals, this observer fires on every DOM mutation in the document, causing severe performance issues during Angular's change detection cycles. ## What is the new behavior? The MutationObserver for parent removal detection is now skipped for controller-based modals and when the cached parent is the app root (document.body or ion-app). These parents are never removed from the DOM, so observing them is unnecessary. This prevents the performance issues while still maintaining the parent removal detection behavior for inline modals with meaningful parent elements. ## Does this introduce a breaking change? - [ ] Yes - [X] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> Current dev build: ``` 8.7.12-dev.11765231260.1def96ab ``` --------- Co-authored-by: Maria Hutt <thetaPC@users.noreply.github.com>
This commit is contained in:
@@ -1279,6 +1279,20 @@ export class Modal implements ComponentInterface, OverlayInterface {
|
||||
return;
|
||||
}
|
||||
|
||||
/**
|
||||
* Don't observe for controller-based modals or when the parent is the
|
||||
* app root (document.body or ion-app). These parents won't be removed,
|
||||
* and observing document.body with subtree: true causes performance
|
||||
* issues with frameworks like Angular during change detection.
|
||||
*/
|
||||
if (
|
||||
this.hasController ||
|
||||
this.cachedOriginalParent === document.body ||
|
||||
this.cachedOriginalParent.tagName === 'ION-APP'
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
this.parentRemovalObserver = new MutationObserver((mutations) => {
|
||||
mutations.forEach((mutation) => {
|
||||
if (mutation.type === 'childList' && mutation.removedNodes.length > 0) {
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { expect } from '@playwright/test';
|
||||
import { configs, test, Viewports } from '@utils/test/playwright';
|
||||
import type { E2EPage } from '@utils/test/playwright';
|
||||
import { configs, test, Viewports } from '@utils/test/playwright';
|
||||
|
||||
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
|
||||
test.describe(title('modal: focus trapping'), () => {
|
||||
@@ -104,6 +104,28 @@ configs().forEach(({ title, screenshot, config }) => {
|
||||
});
|
||||
|
||||
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
|
||||
test.describe(title('modal: parent removal observer'), () => {
|
||||
test('should not set up parentRemovalObserver for controller-created modals', async ({ page }, testInfo) => {
|
||||
testInfo.annotations.push({
|
||||
type: 'issue',
|
||||
description: 'FW-6766',
|
||||
});
|
||||
|
||||
await page.goto('/src/components/modal/test/basic', config);
|
||||
const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
|
||||
|
||||
await page.click('#basic-modal');
|
||||
await ionModalDidPresent.next();
|
||||
|
||||
const modal = page.locator('ion-modal');
|
||||
const hasObserver = await modal.evaluate((el: any) => {
|
||||
return el.parentRemovalObserver !== undefined;
|
||||
});
|
||||
|
||||
expect(hasObserver).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
test.describe(title('modal: backdrop'), () => {
|
||||
test.beforeEach(async ({ page }) => {
|
||||
await page.goto('/src/components/modal/test/basic', config);
|
||||
|
||||
Reference in New Issue
Block a user