canjs / can-stache

Live binding handlebars templates
https://canjs.com/doc/can-stache.html
MIT License
10 stars 13 forks source link

SVG namespace fix when svg-attribute contains stache-expression #712

Closed cleong-tc closed 5 years ago

cleong-tc commented 5 years ago

How often can you reproduce it?

Description: if the svg-namespace-attribute is an expression, the svg-image does not render. hard-coded svg-namespace-attribute does render (e.g. <use xlink:href="icons.svg#logo"></use>) which was resolved by https://github.com/canjs/can-stache/pull/604 .

for example, the stache...

<svg role="img"
    width="9"
    height="9"
>
    <use xlink:href="icons.svg#{{./hash}}"></use>
</svg>

does not render svg-image because can-attribute-observable/behaviors.js calls setAttribute() instead of setAttributeNS().

related to https://github.com/canjs/canjs/pull/2438

i am migrating from donejs@1 (with canjs@3) to donejs@3 (with canjs@5), so bug probably re-introducted when can-stache switched from can-util/dom/attr.

fix PRs:

  1. https://github.com/canjs/can-attribute-observable/pull/38
  2. https://github.com/canjs/can-dom-mutate/pull/68 (1.x-legacy-branch)
  3. https://github.com/canjs/can-dom-mutate/pull/69 (master-branch)

call-trace...

1) can-stache/can-stache.js loads can-stache/src/html_section.js

2) can-view-target/can-view-target.js goes to...

                    if(typeof value === "function"){
                        getCallback().callbacks.push({
                            callback:  value
                        });

instead of...

                    } else if (value !== null && typeof value === "object" && value.namespaceURI) {
                        el.setAttributeNS(value.namespaceURI,attrName,value.value);

3) can-stache/src/text_section.js goes to...

                else if(state.attr) {
                    live.attr(this, state.attr, observation);

4) can-attribute-observable/behaviors.js goes to...

                domMutateNode.setAttribute.call(this, attrName, val);

5) can-dom-mutate/node/node.js goes to...

var nodeMethods = ['appendChild', 'insertBefore', 'removeChild', 'replaceChild', 'setAttribute', 'removeAttribute'];
nodeMethods.forEach(function (methodName) {
    normal[methodName] = function () {
        return this[methodName].apply(this, arguments);
    };
});

Steps to reproduce:

  1. Include a JS Bin (or equivalent) link if possible
  2. Detail the exact steps taken to produce the problem
  3. Include a gif if possible; you can use LICEcap to make a gif: http://www.cockos.com/licecap/

Expected results: the svg-image should render.

Actual results: the svg-image appears in the server-side-render (via donejs-ssr), but on the client-side-render the svg-image disappears.

Environment:

Software Version
can-stache version 4.17.19
Browser Chrome 75.0.3770.142 (Official Build) (64-bit)
Operating system Ubuntu 16.04 LTS
nlundquist commented 5 years ago

hey @cleong-tc thanks for submitting these! The PRs look good, but would you consider also including a test (in can-attribute-observable) and opening an additional PR for the master branch of can-dom-mutate? To accept these for the master branches of the other two projects we'll need to add compatibility to the master branch of can-dom-mutate also.

cleong-tc commented 5 years ago

@nlundquist : unit-tests pushed for can-attribute-observable & can-dom-mutate (1.x-legacy & master branches).

nlundquist commented 5 years ago

Awesome, thanks! Just released can-attribute-observable, can you bump the version of can-dom-mutate to 2.0.5 in your PR canjs/can-attribute-observable#38 ?

cleong-tc commented 5 years ago

@nlundquist : thanks

nlundquist commented 5 years ago

Closing this issue due to all the fix PRs having been merged. Thanks again Colin.