thednp / bootstrap.native

Bootstrap components build with Typescript
http://thednp.github.io/bootstrap.native/
MIT License
1.7k stars 177 forks source link

exception when `isEmptyAnchor` called with SVG <use> tag #466

Closed zbryikt closed 1 year ago

zbryikt commented 1 year ago

We are using v4 so this happens with the code in bs-v4 branch here: https://github.com/thednp/bootstrap.native/blob/15d98c4cea9be1c6c1fd6ea6df29a0835ee6179d/src/util/isEmptyAnchor.js#L15

in function isEmptyAnchor, following codes are used to check against empty href:

element.href.slice(-1) === "#"

however, element.href is not guaranteed to be a string even if element.hasAttribute("href") is true, such as when element is a SVG tag:

<use href="#"/>

in this case the href member may be a SVGAnimatedString object which doesn't have slice builtin function, thus causing a TypeError exception with this message: "t.href.slice is not a function".

A simple way to reproduce this issue is to

  1. toggle on some dropdown
  2. click any SVG <use> tag to toggle off the dropdown.

While we are using v4 version, in main branch there is also isEmptyAnchor.ts with similar codes so this may also happen in the main branch - but this is untested so I'm not sure:

https://github.com/thednp/bootstrap.native/blob/ba129ab7317b627dae0811a7542dc8b93af30fd7/src/util/isEmptyAnchor.ts#L17

thednp commented 1 year ago

Please provide a test pen/fiddle to look into it.

zbryikt commented 1 year ago

It seems that v4 is now in a separated branch (not sure if it's still maintained since there is no separated release for v4 currently) and it doesn't seem to still available in the latest release files so I'm not sure what's the latest officially released version for v4, so in case that I use a wrong version of code, I copied all code from latest snapshot of dist/bootstrap-native.min.js in bs-4 branch directly and make a pen here:

https://codepen.io/loadingio/pen/YzRpJvz

To reproduce, simply toggle the dropdown on and then click the red circle, which is a SVG image tag with data url in itshref attribute.

Have also tried v5 and it seems to work correctly but we don't use v5 so not sure if we've done it right - since this issue mainly discuss v4, I will skip that part here. :)

(migration to v5 will be a big task and when we are ready there will be probably v6 so we will still use bootstrap v4 for some time. )

zbryikt commented 1 year ago

A possible fix is to rewrite these:

t.href.slice(-1)
t.parentNode.href.slice(-1)

into these:

t.getAttribute('href')).slice(-1)
t.parentNode.getAttribute('href')).slice(-1)

since the return value of getAttribute("href") is guaranteed to be a string, based on DOM Level 2 Spec and SVG is compatible with DOM Level 2.

I made a patch in a forked repo but tend to not send a PR for now, only for your reference: https://github.com/zbryikt/bootstrap.native/commit/737dc2e465408f8bcb0f5e9e34ffefa62fc781f9

I found href.slice in both src/util/isEmptyAnchor.js and src/components/dropdown-native.js so I patched both of them in that commit.

thednp commented 1 year ago

@zbryikt just that you know, I've updated the local repo on my machine with the proposed changes but I'm having problems with some dependencies vulnerabilities, we might have to wait a couple of days to get them sorted out.

zbryikt commented 1 year ago

No worries! It's an open-source project so please don't feel any pressure, especially this bs-4 branch is more like a legacy version. We can use the forked custom built version first. Thank you for this awesome lib! :)

thednp commented 1 year ago

Change implemented and solved issues. New version online now.