Skip to content

Commit f031ef1

Browse files
ShaneKthetaPC
andauthored
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 <[email protected]>
1 parent 99dcf38 commit f031ef1

File tree

2 files changed

+37
-1
lines changed

2 files changed

+37
-1
lines changed

core/src/components/modal/modal.tsx

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1183,6 +1183,20 @@ export class Modal implements ComponentInterface, OverlayInterface {
11831183
return;
11841184
}
11851185

1186+
/**
1187+
* Don't observe for controller-based modals or when the parent is the
1188+
* app root (document.body or ion-app). These parents won't be removed,
1189+
* and observing document.body with subtree: true causes performance
1190+
* issues with frameworks like Angular during change detection.
1191+
*/
1192+
if (
1193+
this.hasController ||
1194+
this.cachedOriginalParent === document.body ||
1195+
this.cachedOriginalParent.tagName === 'ION-APP'
1196+
) {
1197+
return;
1198+
}
1199+
11861200
this.parentRemovalObserver = new MutationObserver((mutations) => {
11871201
mutations.forEach((mutation) => {
11881202
if (mutation.type === 'childList' && mutation.removedNodes.length > 0) {

core/src/components/modal/test/basic/modal.e2e.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { expect } from '@playwright/test';
2-
import { configs, test, Viewports } from '@utils/test/playwright';
32
import type { E2EPage } from '@utils/test/playwright';
3+
import { configs, test, Viewports } from '@utils/test/playwright';
44

55
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
66
test.describe(title('modal: focus trapping'), () => {
@@ -104,6 +104,28 @@ configs().forEach(({ title, screenshot, config }) => {
104104
});
105105

106106
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
107+
test.describe(title('modal: parent removal observer'), () => {
108+
test('should not set up parentRemovalObserver for controller-created modals', async ({ page }, testInfo) => {
109+
testInfo.annotations.push({
110+
type: 'issue',
111+
description: 'FW-6766',
112+
});
113+
114+
await page.goto('/src/components/modal/test/basic', config);
115+
const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
116+
117+
await page.click('#basic-modal');
118+
await ionModalDidPresent.next();
119+
120+
const modal = page.locator('ion-modal');
121+
const hasObserver = await modal.evaluate((el: any) => {
122+
return el.parentRemovalObserver !== undefined;
123+
});
124+
125+
expect(hasObserver).toBe(false);
126+
});
127+
});
128+
107129
test.describe(title('modal: backdrop'), () => {
108130
test.beforeEach(async ({ page }) => {
109131
await page.goto('/src/components/modal/test/basic', config);

0 commit comments

Comments
 (0)