Fix detection of anchor click events inside svg #23272

klaasman · 2021-03-22T11:22:41Z

Bug

The nodeName of an anchor inside an SVG equals the lowercase a instead of the html anchor's uppercase A.
This behavior can be seen in this plain js demo: https://jsfiddle.net/L2p8f4ve/28/

fixes #23252

Bug

Related issues linked using fixes #number
Integration tests added

Feature

Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
Related issues linked using fixes #number
Integration tests added
Documentation added
Telemetry added. In case of a feature if it's used or not.

Documentation / Examples

Make sure the linting passes

timneutkens · 2021-03-23T15:43:17Z

Thanks!

I've added back the PR template as there's a missing step in this PR:

Integration tests added

klaasman · 2021-03-23T19:51:29Z

Any pointers on where to add/write the integration test?

timneutkens · 2021-03-24T09:08:23Z

In test/integration you can find examples of other integration tests.

# Conflicts: # package.json # test/lib/next-webdriver.d.ts # test/lib/wd-chain.js # yarn.lock

ijjk · 2022-02-06T20:38:14Z

Stats from current PR

Default Build (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	klaasman/next.js fix-link-in-svg	Change
buildDuration	19.6s	19.4s	-173ms
buildDurationCached	7.6s	7.6s	-35ms
nodeModulesSize	359 MB	359 MB	⚠️ +221 B

Page Load Tests Overall increase ✓

	vercel/next.js canary	klaasman/next.js fix-link-in-svg	Change
/ failed reqs	0	0	✓
/ total time (seconds)	4.041	3.972	-0.07
/ avg req/sec	618.62	629.46	+10.84
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.713	1.682	-0.03
/error-in-render avg req/sec	1459.81	1486.03	+26.22

Client Bundles (main, webpack, commons)

	vercel/next.js canary	klaasman/next.js fix-link-in-svg	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.2 kB	42.2 kB	✓
main-HASH.js gzip	27.4 kB	27.4 kB	✓
webpack-HASH.js gzip	1.44 kB	1.44 kB	✓
Overall change	71.2 kB	71.2 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	klaasman/next.js fix-link-in-svg	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages Overall increase ⚠️

	vercel/next.js canary	klaasman/next.js fix-link-in-svg	Change
_app-HASH.js gzip	1.36 kB	1.36 kB	✓
_error-HASH.js gzip	194 B	194 B	✓
amp-HASH.js gzip	312 B	312 B	✓
css-HASH.js gzip	326 B	326 B	✓
dynamic-HASH.js gzip	2.37 kB	2.37 kB	✓
head-HASH.js gzip	350 B	350 B	✓
hooks-HASH.js gzip	919 B	919 B	✓
image-HASH.js gzip	4.94 kB	4.94 kB	✓
index-HASH.js gzip	263 B	263 B	✓
link-HASH.js gzip	2.19 kB	2.2 kB	⚠️ +10 B
routerDirect..HASH.js gzip	321 B	321 B	✓
script-HASH.js gzip	383 B	383 B	✓
withRouter-HASH.js gzip	318 B	318 B	✓
85e02e95b279..7e3.css gzip	107 B	107 B	✓
Overall change	14.3 kB	14.4 kB	⚠️ +10 B

Client Build Manifests Overall increase ⚠️

	vercel/next.js canary	klaasman/next.js fix-link-in-svg	Change
_buildManifest.js gzip	459 B	460 B	⚠️ +1 B
Overall change	459 B	460 B	⚠️ +1 B

Rendered Page Sizes

	vercel/next.js canary	klaasman/next.js fix-link-in-svg	Change
index.html gzip	532 B	532 B	✓
link.html gzip	546 B	546 B	✓
withRouter.html gzip	526 B	526 B	✓
Overall change	1.6 kB	1.6 kB	✓

Diffs

Diff for _buildManifest.js

@@ -13,7 +13,7 @@ self.__BUILD_MANIFEST = {
   "/head": ["static\u002Fchunks\u002Fpages\u002Fhead-7100d3b2a548f0e4.js"],
   "/hooks": ["static\u002Fchunks\u002Fpages\u002Fhooks-538d621a0e670391.js"],
   "/image": ["static\u002Fchunks\u002Fpages\u002Fimage-f97943edf7ae3dd3.js"],
-  "/link": ["static\u002Fchunks\u002Fpages\u002Flink-f0a2c3bb0706d8b2.js"],
+  "/link": ["static\u002Fchunks\u002Fpages\u002Flink-27386a147a7100fa.js"],
   "/routerDirect": [
     "static\u002Fchunks\u002Fpages\u002FrouterDirect-76232dd6bc335a24.js"
   ],

Diff for link-HASH.js

@@ -151,8 +151,10 @@
         locale
       ) {
         var nodeName = e.currentTarget.nodeName;
+        // anchors inside an svg have a lowercase nodeName
+        var isAnchorNodeName = nodeName.toUpperCase() === "A";
         if (
-          nodeName === "A" &&
+          isAnchorNodeName &&
           (isModifiedEvent(e) || !(0, _router).isLocalURL(href))
         ) {
           // ignore click for browser’s default behavior

Diff for link.html

@@ -27,7 +27,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/pages/link-f0a2c3bb0706d8b2.js"
+      src="/_next/static/chunks/pages/link-27386a147a7100fa.js"
       defer=""
     ></script>
     <script src="/_next/static/BUILD_ID/_buildManifest.js" defer=""></script>

Default Build with SWC (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	klaasman/next.js fix-link-in-svg	Change
buildDuration	23.4s	23.6s	⚠️ +192ms
buildDurationCached	7.7s	7.6s	-36ms
nodeModulesSize	359 MB	359 MB	⚠️ +221 B

Page Load Tests Overall increase ✓

	vercel/next.js canary	klaasman/next.js fix-link-in-svg	Change
/ failed reqs	0	0	✓
/ total time (seconds)	3.96	4.007	⚠️ +0.05
/ avg req/sec	631.35	623.89	⚠️ -7.46
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.798	1.668	-0.13
/error-in-render avg req/sec	1390.05	1498.64	+108.59

Client Bundles (main, webpack, commons)

	vercel/next.js canary	klaasman/next.js fix-link-in-svg	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.3 kB	42.3 kB	✓
main-HASH.js gzip	27.4 kB	27.4 kB	✓
webpack-HASH.js gzip	1.44 kB	1.44 kB	✓
Overall change	71.3 kB	71.3 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	klaasman/next.js fix-link-in-svg	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages Overall increase ⚠️

	vercel/next.js canary	klaasman/next.js fix-link-in-svg	Change
_app-HASH.js gzip	1.35 kB	1.35 kB	✓
_error-HASH.js gzip	180 B	180 B	✓
amp-HASH.js gzip	305 B	305 B	✓
css-HASH.js gzip	321 B	321 B	✓
dynamic-HASH.js gzip	2.36 kB	2.36 kB	✓
head-HASH.js gzip	342 B	342 B	✓
hooks-HASH.js gzip	911 B	911 B	✓
image-HASH.js gzip	4.97 kB	4.97 kB	✓
index-HASH.js gzip	256 B	256 B	✓
link-HASH.js gzip	2.21 kB	2.22 kB	⚠️ +11 B
routerDirect..HASH.js gzip	314 B	314 B	✓
script-HASH.js gzip	375 B	375 B	✓
withRouter-HASH.js gzip	309 B	309 B	✓
85e02e95b279..7e3.css gzip	107 B	107 B	✓
Overall change	14.3 kB	14.3 kB	⚠️ +11 B

Client Build Manifests

	vercel/next.js canary	klaasman/next.js fix-link-in-svg	Change
_buildManifest.js gzip	459 B	459 B	✓
Overall change	459 B	459 B	✓

Rendered Page Sizes

	vercel/next.js canary	klaasman/next.js fix-link-in-svg	Change
index.html gzip	533 B	533 B	✓
link.html gzip	546 B	546 B	✓
withRouter.html gzip	528 B	528 B	✓
Overall change	1.61 kB	1.61 kB	✓

Diffs

Diff for _buildManifest.js

@@ -13,7 +13,7 @@ self.__BUILD_MANIFEST = {
   "/head": ["static\u002Fchunks\u002Fpages\u002Fhead-7100d3b2a548f0e4.js"],
   "/hooks": ["static\u002Fchunks\u002Fpages\u002Fhooks-538d621a0e670391.js"],
   "/image": ["static\u002Fchunks\u002Fpages\u002Fimage-f97943edf7ae3dd3.js"],
-  "/link": ["static\u002Fchunks\u002Fpages\u002Flink-f0a2c3bb0706d8b2.js"],
+  "/link": ["static\u002Fchunks\u002Fpages\u002Flink-27386a147a7100fa.js"],
   "/routerDirect": [
     "static\u002Fchunks\u002Fpages\u002FrouterDirect-76232dd6bc335a24.js"
   ],

Diff for link-HASH.js

@@ -151,8 +151,10 @@
         locale
       ) {
         var nodeName = e.currentTarget.nodeName;
+        // anchors inside an svg have a lowercase nodeName
+        var isAnchorNodeName = nodeName.toUpperCase() === "A";
         if (
-          nodeName === "A" &&
+          isAnchorNodeName &&
           (isModifiedEvent(e) || !(0, _router).isLocalURL(href))
         ) {
           // ignore click for browser’s default behavior

Diff for link.html

@@ -27,7 +27,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/pages/link-f0a2c3bb0706d8b2.js"
+      src="/_next/static/chunks/pages/link-27386a147a7100fa.js"
       defer=""
     ></script>
     <script src="/_next/static/BUILD_ID/_buildManifest.js" defer=""></script>

Commit: fb67e68

ijjk

Thanks for the PR!

## Bug The `nodeName` of an anchor inside an SVG equals the lowercase `a` instead of the html anchor's uppercase `A`. This behavior can be seen in this plain js demo: https://jsfiddle.net/L2p8f4ve/28/ fixes vercel#23252 ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. ## Documentation / Examples - [ ] Make sure the linting passes Co-authored-by: JJ Kasper <22380829+ijjk@users.noreply.github.com>

Fix detection of anchor click events inside svg

60071f2

klaasman requested review from divmain, ijjk, lfades, shuding and timneutkens as code owners March 22, 2021 11:22

ijjk added the type: next label Mar 22, 2021

This comment has been minimized.

Sign in to view

Merge branch 'canary' into fix-link-in-svg

6f70825

This comment has been minimized.

Sign in to view

klaasman added 2 commits March 24, 2021 12:22

Write integration test

3117964

Merge branch 'canary' into fix-link-in-svg

56fdc36

This comment has been minimized.

Sign in to view

This comment was marked as outdated.

Sign in to view

klaasman requested review from huozhi, padmaia and styfle as code owners October 26, 2021 15:29

ijjk added 3 commits February 6, 2022 13:37

Merge branch 'canary' into fix-link-in-svg

97e7aef

# Conflicts: # package.json # test/lib/next-webdriver.d.ts # test/lib/wd-chain.js # yarn.lock

Update test

b747b66

Merge branch 'canary' into fix-link-in-svg

fb67e68

ijjk approved these changes Feb 6, 2022

View reviewed changes

kodiakhq bot merged commit b39e49e into vercel:canary Feb 6, 2022

klaasman deleted the fix-link-in-svg branch February 11, 2022 14:21

klaasman restored the fix-link-in-svg branch February 11, 2022 14:21

github-actions bot locked as resolved and limited conversation to collaborators Mar 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix detection of anchor click events inside svg #23272

Fix detection of anchor click events inside svg #23272

klaasman commented Mar 22, 2021 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

timneutkens commented Mar 23, 2021 •

edited

Loading

klaasman commented Mar 23, 2021

timneutkens commented Mar 24, 2021

This comment has been minimized.

This comment was marked as outdated.

ijjk commented Feb 6, 2022

Diffs

Diffs

ijjk left a comment

Fix detection of anchor click events inside svg #23272

Fix detection of anchor click events inside svg #23272

Conversation

klaasman commented Mar 22, 2021 • edited Loading

Bug

Bug

Feature

Documentation / Examples

This comment has been minimized.

This comment has been minimized.

timneutkens commented Mar 23, 2021 • edited Loading

klaasman commented Mar 23, 2021

timneutkens commented Mar 24, 2021

This comment has been minimized.

This comment was marked as outdated.

ijjk commented Feb 6, 2022

Stats from current PR

Diffs

Diffs

ijjk left a comment

Choose a reason for hiding this comment

klaasman commented Mar 22, 2021 •

edited

Loading

timneutkens commented Mar 23, 2021 •

edited

Loading