fixed namespaced attributes not updating

[AndyOGo]

fixes #124

Using SVG sprites with <use xlink:href="" /> will not update the icon.

The reason is that getAttributeNS and setAttributeNS 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 the localName, i.e. getAttributeNS('https://www.w3.org/1999/foo', 'bar');

  getAttributeNS(namespace, localName)
  https://www.w3.org/TR/DOM-Level-2-Core/glossary.html#dt-localname
  A local name is the local part of a qualified name. This is called the local part in Namespaces in XML [Namespaces].

• BUT setAttributeNS expects it's fully qualified name, i.e. setAttributeNS('https://www.w3.org/1999/foo', 'foo:bar');

  setAttributeNS(namespace, qualifiedName, value)
  https://www.w3.org/TR/DOM-Level-2-Core/glossary.html#dt-qualifiedname
  A qualified name is the name of an element or attribute defined as the concatenation of a local name (as defined in this specification), optionally preceded by a namespace prefix and colon character. See Qualified Names in Namespaces in XML [Namespaces].

[AndyOGo]

@patrick-steele-idem
May I asked you to look into this as soon as you have some free time?
I'm happy to get your feedback:)

[AutoSponge]

@AndyOGo is it possible to demonstrate this with a test (one that fails under current version but passes under proposed)?

I know @patrick-steele-idem changed jobs recently, so he may not have the time to look into this.

[AndyOGo]

@AutoSponge
Thank you for your answer.

Well, the thing is that, I faced this issue in Firefox at a customer of mine for axa-ch-webhub-cloud/pattern-library#411.
Though we are using nanomorph at above project, I figured since they borrowed a few bits from morphdom, may the same issue exists here. And indeed morphdom's code is identical.

Here are the copies from my screencasts (but baked by nanomorph):

Latest Chrome:

Latest Firefox:

May I ask, aren't the links I provided form the official specs making the point clear enough?
For me it's obvious that the official API specification is inconsistent and there is the real bug, but I can't change that. We could only change the used API calls 😊

And I supplied the same Bugfix to nanomorph

[AndyOGo]

PS. The reason this works in Chrome, is that SVG 2 drop <use>'s xlink:href in favour of href...

[AndyOGo]

PPS: I also updated the related MDN article here:
https://developer.mozilla.org/en-US/docs/Web/API/Element/setAttributeNS

[AutoSponge]

This PR needs a test which can be run cross-browser to ensure it will not fail existing functionality. Fixing firefox but breaking IE is not an option.

This also serves to explain to other developers what's expected for a given mutation.

[AutoSponge]

We use || attrName because IE doesn't seem to support .localName see

[AndyOGo]

Thanks for your feedback, very appreciated.
About which version of IE are we speaking? (IE9 supports localName, see).
Microsoft officially stopped to support old IE versions until version 11

My point is that the setAttributeNS API is called with the wrong argument and I provided the official W3C specifications. May I ask you to study them yourself, look up all the technical definitions in the official DOM API glossary and then decide for yourself if I'm right or wrong.

[AndyOGo]

@AutoSponge
I just inlined the attrName fallback just for getAttributeNS.

[AutoSponge]

@AndyOGo are you saying these two blocks are different?

current

attrName = attr.localName || attrName;
fromValue = fromNode.getAttributeNS(attrNamespaceURI, attrName);

this PR

var attrLocalName = attr.localName;
fromValue = fromNode.getAttributeNS(attrNamespaceURI, attrLocalName || attrName);

Because I'm not seeing it. I'm still confused as to exactly what you think this fixes.

[AndyOGo]

No with my last commit I restored the original getAttributeNS function call with a fallback to attrName if attr.localName is not defined.

But the line below using setAttributeNS has changed to never use attr.localName.

[AndyOGo]

@AutoSponge
Since I don't overwrite attrName with attr.localName || attrName anymore, setAttributeNS will always only use the full qualified attribute name, including it's namespace.

[AndyOGo]

I also moved my complementary comments to the related API calls

[AndyOGo]

@AutoSponge
I really want to suggest you read and understand the execution flow of morphAttrs function at runtime. My contribution is definitely NOT a noop.

Before the local variable attrName was conditionally overwritten.
By my contribution this variable is never overwritten, causing proper argument passing to setAttributeNS below.

And please stop trolling and giving me bad reputation, I develop frontend projects since decades, have studied all ECMA script specs starting with version 1 fully by myself.

[AutoSponge]

xlink:href is deprecated: https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/xlink:href

Do not use. Without a test case to demonstrate how this affects morphdom, I'm keeping it closed. I've already blocked you but I will report if you continue this harassment and waste of my time.

[AndyOGo]

Agreed xlink:href is deprecated and I understand this point.

I'm sorry if you interpret my comments as harassment, they don't meant to be.

My main idea is to help and to suggest an spec-conformant implementation regarding the usage of setAttributeNS. From my viewpoint it's really a general and very detailed bug in morphdom. So I made also sure to provide all official documentation regarding this issue.

Unfortunately cooperation with you is not a perfect match and therefore I can't help morphdom getting better.