Skip to content

Conversation

@zagi25
Copy link
Contributor

@zagi25 zagi25 commented Nov 20, 2025

Issue: If window.location.hash is changed manually, modal stays open resulting in possibility to open another modal on top of already open modal or scrolling the page to the heading while modal is open

  • Close modal when hash changes
  • Prevent focusing modal button if new hash is heading hash

Resolves: MWPW-182328

Test URLs:

@zagi25 zagi25 requested a review from a team as a code owner November 20, 2025 11:36
@NadiiaSokolova NadiiaSokolova self-assigned this Nov 24, 2025
@NadiiaSokolova NadiiaSokolova requested a review from a team December 1, 2025 11:58
@github-actions
Copy link
Contributor

github-actions bot commented Dec 9, 2025

This PR has not been updated recently and will be closed in 7 days if no action is taken. Please ensure all checks are passing, https://github.com/orgs/adobecom/discussions/997 provides instructions. If the PR is ready to be merged, please mark it with the "Ready for Stage" label.

@github-actions
Copy link
Contributor

This PR has not been updated recently and will be closed in 7 days if no action is taken. Please ensure all checks are passing, https://github.com/orgs/adobecom/discussions/997 provides instructions. If the PR is ready to be merged, please mark it with the "Ready for Stage" label.

@github-actions
Copy link
Contributor

This PR has not been updated recently and will be closed in 7 days if no action is taken. Please ensure all checks are passing, https://github.com/orgs/adobecom/discussions/997 provides instructions. If the PR is ready to be merged, please mark it with the "Ready for Stage" label.

@github-actions github-actions bot added the Stale label Dec 26, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jan 2, 2026

Closing this PR due to inactivity.

@github-actions github-actions bot closed this Jan 2, 2026
@zagi25 zagi25 reopened this Jan 6, 2026
@zagi25 zagi25 removed the Stale label Jan 6, 2026
expect(document.getElementById('milo')).not.to.exist;
});

it('Closes a modal on manual hash chagne', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it('Closes a modal on manual hash chagne', async () => {
it('Closes a modal on manual hash change', async () => {

@yeibercano
Copy link
Contributor

@zagi25 , I am seeing issues with hashes containing from_ims=true:
https://mwpw-182328-modal-hash--milo--adobecom.aem.page/drafts/ratko/multiple-modals/multiple-modals#old_hash=checkout&from_ims=true

Modal1 and Modal2 break: no modals open after clicking CTAs

Here is a patch to add the fixes:
modal_fixes.patch

# 1. Check patch applies cleanly
git apply --check modal_fixes.patch

# 2. Apply the patch
git apply modal_fixes.patch

# 3. Commit the changes
git add libs/blocks/modal/modal.js
git commit -m "fix: modal IMS handling and querySelector crashes

- Add safeQuerySelector helper to prevent DOMException crashes
- Fix IMS detection logic (|| to &&)
- Fix isDeepLink reset position and logic
- Update all querySelector calls to use safe wrapper"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants