-
Notifications
You must be signed in to change notification settings - Fork 58
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
fixed updating namespaced attributes is broken #102
base: master
Are you sure you want to change the base?
fixed updating namespaced attributes is broken #102
Conversation
All tests pass:
|
@yoshuawuyts |
This same code was submitted to morphdom but is a noop. All it does is rename a variable. I suggest close. |
That previous comment is incorrect. @AutoSponge does not understand the flow of program execution during runtime and he is trolling my reputation. |
@AutoSponge how can this only be a variable name change? The magic is on line 24 and 29 as attrName contains the original value instead of the conditional one removed at line 19. |
I'm not having any objections to this patch. Interested in hearing more from @AutoSponge though, I value his experience from prior work on @AndyOGo I wouldn't stress about reputation here; this patch is pretty great. If we can keep this on-topic (without any vigorous up/downvoting) I'm sure we can figure out the right approach for this patch. I'm convinced everyone is here with the best intentions. Also would it perhaps be possible to add a test to catch any future regressions for this? Only if it's not too much effort. Thanks! edit: also thanks for your first contribution; it's very thorough - that's real nice 😁 |
@yoshuawuyts Btw the main contributor of |
Ah, I see what its doing now. It looked like a noop to me first too but the difference is that |
fixes #101
Using SVG sprites with
<use xlink:href="" />
will not update the icon.The reason is that
getAttributeNS
andsetAttributeNS
DOM APIs are inconsistent in it's arguments.E.g.: lets say 'foo:bar' ('foo' being the namespace and 'bar' the local attribute name):
getAttributeNS
only needs thelocalName
, i.e.getAttributeNS('https://www.w3.org/1999/foo', 'bar');
setAttributeNS
expects it's fully qualified name, i.e.setAttributeNS('https://www.w3.org/1999/foo', 'foo:bar');