Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix detection of anchor click events inside svg #23272

Merged
merged 7 commits into from Feb 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/next/client/link.tsx
Expand Up @@ -86,7 +86,10 @@ function linkClicked(
): void {
const { nodeName } = e.currentTarget

if (nodeName === 'A' && (isModifiedEvent(e) || !isLocalURL(href))) {
// anchors inside an svg have a lowercase nodeName
const isAnchorNodeName = nodeName.toUpperCase() === 'A'

if (isAnchorNodeName && (isModifiedEvent(e) || !isLocalURL(href))) {
// ignore click for browser’s default behavior
return
}
Expand Down
23 changes: 21 additions & 2 deletions test/integration/client-navigation/pages/nav/index.js
Expand Up @@ -78,14 +78,33 @@ export default class extends Component {
</Link>
<Link href="/nav/on-click?count=1">
<a id="on-click-link" style={linkStyle}>
A element with onClick
An element with onClick
</a>
</Link>
<Link href="/nav/about">
<a id="target-link" target="_blank">
A element with target
An element with target
</a>
</Link>

<svg
width={24}
height={24}
fill="none"
xmlns="http://www.w3.org/2000/svg"
>
<Link href="/nav/about">
<a id="in-svg-link" style={{ display: 'block' }}>
<path
fillRule="evenodd"
clipRule="evenodd"
d="M12 0C5.373 0 0 5.373 0 12s5.373 12 12 12 12-5.373 12-12S18.627 0 12 0Zm4.5 18a4 4 0 1 0 0-8 4 4 0 0 0 0 8Z"
fill="#335BF1"
/>
</a>
</Link>
</svg>

<button
onClick={() => this.visitQueryStringPage()}
style={linkStyle}
Expand Down
29 changes: 25 additions & 4 deletions test/integration/client-navigation/test/index.test.js
Expand Up @@ -180,13 +180,35 @@ describe('Client Navigation', () => {
it('should not navigate if the <a/> tag has a target', async () => {
const browser = await webdriver(context.appPort, '/nav')

const counterText = await browser
await browser
.elementByCss('#increase')
.click()
.elementByCss('#target-link')
.click()
.elementByCss('#counter')
.text()

await waitFor(1000)

const counterText = await browser.elementByCss('#counter').text()

expect(counterText).toBe('Counter: 1')
await browser.close()
})

it('should not navigate if the click-event is modified', async () => {
const browser = await webdriver(context.appPort, '/nav')

await browser.elementByCss('#increase').click()

const key = process.platform === 'darwin' ? 'Meta' : 'Control'

await browser.keydown(key)

await browser.elementByCss('#in-svg-link').click()

await browser.keyup(key)
await waitFor(1000)

const counterText = await browser.elementByCss('#counter').text()

expect(counterText).toBe('Counter: 1')
await browser.close()
Expand Down Expand Up @@ -413,7 +435,6 @@ describe('Client Navigation', () => {
}
})
})

describe('resets scroll at the correct time', () => {
it('should reset scroll before the new page runs its lifecycles (<Link />)', async () => {
let browser
Expand Down
11 changes: 10 additions & 1 deletion test/lib/browsers/base.ts
Expand Up @@ -29,7 +29,16 @@ export class BrowserInterface {
elementById(selector: string): BrowserInterface {
return this
}
click(): BrowserInterface {
click(opts?: { modifierKey?: boolean }): BrowserInterface {
return this
}
keydown(key: string): BrowserInterface {
return this
}
keyup(key: string): BrowserInterface {
return this
}
focusPage(): BrowserInterface {
return this
}
type(text: string): BrowserInterface {
Expand Down
16 changes: 16 additions & 0 deletions test/lib/browsers/playwright.ts
Expand Up @@ -165,6 +165,10 @@ class Playwright extends BrowserInterface {
return this.chain(async () => context.clearCookies())
}

focusPage() {
return this.chain(() => page.bringToFront())
}

private wrapElement(el: ElementHandle, selector: string) {
;(el as any).selector = selector
;(el as any).text = () => el.innerText()
Expand Down Expand Up @@ -237,6 +241,18 @@ class Playwright extends BrowserInterface {
return this.eval(`!!document.querySelector('${selector}')`) as any
}

keydown(key: string): BrowserInterface {
return this.chain((el) => {
return page.keyboard.down(key).then(() => el)
})
}

keyup(key: string): BrowserInterface {
return this.chain((el) => {
return page.keyboard.up(key).then(() => el)
})
}

click() {
return this.chain((el) => {
return el.click().then(() => el)
Expand Down