From e286f32ebe3f051fbc41c4903ae2e7615363a644 Mon Sep 17 00:00:00 2001 From: ShaneK Date: Wed, 17 Jun 2026 12:34:55 -0700 Subject: [PATCH 1/2] fix(angular): honor modifier-click on routerLink --- .../navigation/router-link-delegate.ts | 39 +++++++++- .../router-link-modifier-click.spec.ts | 73 +++++++++++++++++++ .../router-link/router-link.component.html | 6 ++ .../router-link/router-link.component.ts | 4 +- 4 files changed, 118 insertions(+), 4 deletions(-) create mode 100644 packages/angular/test/base/e2e/src/standalone/router-link-modifier-click.spec.ts diff --git a/packages/angular/common/src/directives/navigation/router-link-delegate.ts b/packages/angular/common/src/directives/navigation/router-link-delegate.ts index 4391d6ea0d1..ddc6e451681 100644 --- a/packages/angular/common/src/directives/navigation/router-link-delegate.ts +++ b/packages/angular/common/src/directives/navigation/router-link-delegate.ts @@ -1,5 +1,5 @@ import { LocationStrategy } from '@angular/common'; -import { ElementRef, OnChanges, OnInit, Directive, HostListener, Input, Optional } from '@angular/core'; +import { ElementRef, OnChanges, OnDestroy, OnInit, Directive, HostListener, Input, Optional } from '@angular/core'; import { Router, RouterLink } from '@angular/router'; import type { AnimationBuilder, RouterDirection } from '@ionic/core/components'; @@ -14,7 +14,7 @@ import { NavController } from '../../providers/nav-controller'; @Directive({ selector: ':not(a):not(area)[routerLink]', }) -export class RouterLinkDelegateDirective implements OnInit, OnChanges { +export class RouterLinkDelegateDirective implements OnInit, OnChanges, OnDestroy { @Input() routerDirection: RouterDirection = 'forward'; @@ -32,12 +32,47 @@ export class RouterLinkDelegateDirective implements OnInit, OnChanges { ngOnInit(): void { this.updateTargetUrlAndHref(); this.updateTabindex(); + + /** + * Ionic components like `ion-item` render a native anchor in their shadow DOM, + * so a modifier click (ctrl/meta/shift) or a non-`_self` target should open a + * new tab via the browser default instead of navigating in-app. + * + * We listen in the capture phase so this runs before Angular's `RouterLink` + * handler and our own bubble-phase `onClick`. On a new-tab intent it stops + * propagation to cancel the in-app navigation, but leaves `preventDefault` + * alone so the native anchor can still open the tab. + */ + this.elementRef.nativeElement.addEventListener('click', this.onCaptureClick, { capture: true }); } ngOnChanges(): void { this.updateTargetUrlAndHref(); } + ngOnDestroy(): void { + this.elementRef.nativeElement.removeEventListener('click', this.onCaptureClick, { capture: true }); + } + + private onCaptureClick = (ev: Event): void => { + if (this.opensInNewTab(ev)) { + ev.stopImmediatePropagation(); + } + }; + + /** + * True when the click should open a new tab: a modifier was held + * (ctrl/meta/shift), or the host targets something other than `_self`. + */ + private opensInNewTab(ev: Event): boolean { + if (ev instanceof MouseEvent && (ev.ctrlKey || ev.metaKey || ev.shiftKey)) { + return true; + } + + const target = this.elementRef.nativeElement.target; + return target != null && target !== '' && target !== '_self'; + } + /** * The `tabindex` is set to `0` by default on the host element when * the `routerLink` directive is used. This causes issues with Ionic diff --git a/packages/angular/test/base/e2e/src/standalone/router-link-modifier-click.spec.ts b/packages/angular/test/base/e2e/src/standalone/router-link-modifier-click.spec.ts new file mode 100644 index 00000000000..ae31a21de20 --- /dev/null +++ b/packages/angular/test/base/e2e/src/standalone/router-link-modifier-click.spec.ts @@ -0,0 +1,73 @@ +import { test, expect, type Page } from '@playwright/test'; + +/** + * Issue #26394: a modifier click (ctrl/meta/shift) or a non-`_self` target on a + * `routerLink` over a non-anchor Ionic component (ion-item, ion-button) must + * open a new tab instead of navigating the current page in-app. + * + * Playwright headless can't dispatch a real modifier-click or observe the + * browser's native new-tab default, so we dispatch a synthetic click on the + * host (not the shadow anchor, which would fire its own default navigation) and + * assert the JS handler chain leaves the page unchanged with `preventDefault` + * uncalled. + */ +test.describe('RouterLink: modifier click', () => { + const dispatchClick = (page: Page, selector: string, modifiers: Record) => + page.evaluate( + ({ selector, mods }: { selector: string; mods: Record }) => { + const item = document.querySelector(selector) as HTMLElement; + const ev = new MouseEvent('click', { bubbles: true, composed: true, cancelable: true, button: 0, ...mods }); + item.dispatchEvent(ev); + return { defaultPrevented: ev.defaultPrevented }; + }, + { selector, mods: modifiers } + ); + + test.beforeEach(async ({ page }) => { + await page.goto('/standalone/router-link'); + }); + + test('normal click navigates the current page in-app', async ({ page }) => { + const { defaultPrevented } = await dispatchClick(page, '#modifier-item', {}); + + await expect(page).toHaveURL(/.*\/standalone\/popover/); + // The delegate prevents the default to stop a hard page reload. + expect(defaultPrevented).toBe(true); + }); + + for (const modifier of ['ctrlKey', 'metaKey', 'shiftKey']) { + test(`${modifier}+click does not navigate the current page and allows a native new tab`, async ({ + page, + }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/26394', + }); + + const { defaultPrevented } = await dispatchClick(page, '#modifier-item', { [modifier]: true }); + + // Give any in-app navigation a chance to run before asserting it did not. + await page.waitForTimeout(300); + await expect(page).toHaveURL(/.*\/standalone\/router-link/); + // Default is left intact so the browser can open the link in a new tab. + expect(defaultPrevented).toBe(false); + }); + } + + test('click on a non-_self target does not navigate the current page and allows a native new tab', async ({ + page, + }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/26394', + }); + + const { defaultPrevented } = await dispatchClick(page, '#target-item', {}); + + // Give any in-app navigation a chance to run before asserting it did not. + await page.waitForTimeout(300); + await expect(page).toHaveURL(/.*\/standalone\/router-link/); + // Default is left intact so the browser can open the link in a new tab. + expect(defaultPrevented).toBe(false); + }); +}); diff --git a/packages/angular/test/base/src/app/standalone/router-link/router-link.component.html b/packages/angular/test/base/src/app/standalone/router-link/router-link.component.html index 8887626d1cb..4de92de77c2 100644 --- a/packages/angular/test/base/src/app/standalone/router-link/router-link.component.html +++ b/packages/angular/test/base/src/app/standalone/router-link/router-link.component.html @@ -10,3 +10,9 @@ I'm an ion-button + + I'm an ion-item + + + I'm an ion-item with a target + diff --git a/packages/angular/test/base/src/app/standalone/router-link/router-link.component.ts b/packages/angular/test/base/src/app/standalone/router-link/router-link.component.ts index 31653f3bb0d..73f470da7ca 100644 --- a/packages/angular/test/base/src/app/standalone/router-link/router-link.component.ts +++ b/packages/angular/test/base/src/app/standalone/router-link/router-link.component.ts @@ -1,11 +1,11 @@ import { Component } from '@angular/core'; import { RouterLink } from '@angular/router'; -import { IonRouterLink, IonRouterLinkWithHref } from '@ionic/angular/standalone'; +import { IonItem, IonRouterLink, IonRouterLinkWithHref } from '@ionic/angular/standalone'; @Component({ selector: 'app-router-link', templateUrl: './router-link.component.html', standalone: true, - imports: [RouterLink, IonRouterLink, IonRouterLinkWithHref], + imports: [RouterLink, IonItem, IonRouterLink, IonRouterLinkWithHref], }) export class RouterLinkComponent {} From d86e78cd8b1529e54f1933248ed32f61847eed8b Mon Sep 17 00:00:00 2001 From: ShaneK Date: Thu, 18 Jun 2026 08:31:12 -0700 Subject: [PATCH 2/2] fix(angular): include alt in routerLink modifier-click handling --- .../navigation/router-link-delegate.ts | 24 +++++++++++-------- .../router-link-modifier-click.spec.ts | 14 ++++++----- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/packages/angular/common/src/directives/navigation/router-link-delegate.ts b/packages/angular/common/src/directives/navigation/router-link-delegate.ts index ddc6e451681..3da7d105c9b 100644 --- a/packages/angular/common/src/directives/navigation/router-link-delegate.ts +++ b/packages/angular/common/src/directives/navigation/router-link-delegate.ts @@ -35,13 +35,14 @@ export class RouterLinkDelegateDirective implements OnInit, OnChanges, OnDestroy /** * Ionic components like `ion-item` render a native anchor in their shadow DOM, - * so a modifier click (ctrl/meta/shift) or a non-`_self` target should open a - * new tab via the browser default instead of navigating in-app. + * so a modifier click (ctrl/meta/shift/alt) or a non-`_self` target should let + * the browser handle the navigation natively (new tab, new window, download) + * instead of navigating in-app. * * We listen in the capture phase so this runs before Angular's `RouterLink` - * handler and our own bubble-phase `onClick`. On a new-tab intent it stops - * propagation to cancel the in-app navigation, but leaves `preventDefault` - * alone so the native anchor can still open the tab. + * handler and our own bubble-phase `onClick`. On a native-navigation intent it + * stops propagation to cancel the in-app navigation, but leaves `preventDefault` + * alone so the native anchor can still act. */ this.elementRef.nativeElement.addEventListener('click', this.onCaptureClick, { capture: true }); } @@ -55,17 +56,20 @@ export class RouterLinkDelegateDirective implements OnInit, OnChanges, OnDestroy } private onCaptureClick = (ev: Event): void => { - if (this.opensInNewTab(ev)) { + if (this.opensNatively(ev)) { ev.stopImmediatePropagation(); } }; /** - * True when the click should open a new tab: a modifier was held - * (ctrl/meta/shift), or the host targets something other than `_self`. + * True when the browser should handle the click natively instead of routing + * in-app: a modifier was held (ctrl/meta/shift/alt), or the host targets + * something other than `_self`. This mirrors the modifier set Angular's own + * `RouterLink` guards on, so an Ionic `routerLink` behaves like a plain anchor + * for new-tab, new-window, and download intents. */ - private opensInNewTab(ev: Event): boolean { - if (ev instanceof MouseEvent && (ev.ctrlKey || ev.metaKey || ev.shiftKey)) { + private opensNatively(ev: Event): boolean { + if (ev instanceof MouseEvent && (ev.ctrlKey || ev.metaKey || ev.shiftKey || ev.altKey)) { return true; } diff --git a/packages/angular/test/base/e2e/src/standalone/router-link-modifier-click.spec.ts b/packages/angular/test/base/e2e/src/standalone/router-link-modifier-click.spec.ts index ae31a21de20..a4926fd46b7 100644 --- a/packages/angular/test/base/e2e/src/standalone/router-link-modifier-click.spec.ts +++ b/packages/angular/test/base/e2e/src/standalone/router-link-modifier-click.spec.ts @@ -1,9 +1,10 @@ import { test, expect, type Page } from '@playwright/test'; /** - * Issue #26394: a modifier click (ctrl/meta/shift) or a non-`_self` target on a - * `routerLink` over a non-anchor Ionic component (ion-item, ion-button) must - * open a new tab instead of navigating the current page in-app. + * Issue #26394: a modifier click (ctrl/meta/shift/alt) or a non-`_self` target on + * a `routerLink` over a non-anchor Ionic component (ion-item, ion-button) must let + * the browser handle the navigation natively (new tab, window, download) instead + * of navigating the current page in-app. * * Playwright headless can't dispatch a real modifier-click or observe the * browser's native new-tab default, so we dispatch a synthetic click on the @@ -35,8 +36,8 @@ test.describe('RouterLink: modifier click', () => { expect(defaultPrevented).toBe(true); }); - for (const modifier of ['ctrlKey', 'metaKey', 'shiftKey']) { - test(`${modifier}+click does not navigate the current page and allows a native new tab`, async ({ + for (const modifier of ['ctrlKey', 'metaKey', 'shiftKey', 'altKey']) { + test(`${modifier}+click does not navigate the current page and allows native handling`, async ({ page, }, testInfo) => { testInfo.annotations.push({ @@ -49,7 +50,8 @@ test.describe('RouterLink: modifier click', () => { // Give any in-app navigation a chance to run before asserting it did not. await page.waitForTimeout(300); await expect(page).toHaveURL(/.*\/standalone\/router-link/); - // Default is left intact so the browser can open the link in a new tab. + // Default is left intact so the browser can handle the link natively + // (new tab, new window, or download, depending on the browser and OS). expect(defaultPrevented).toBe(false); }); }