Namespace issue? - loss of shape attribute on import

[sjernigan]

Describe the bug I think the following is tied to the unresolved namespace discussion but would appreciate confirmation. Basically, the shape attributes are lost when saving the SVG source/opening a file.

To Reproduce Steps to reproduce the behavior:

1. Draw a star using the ext-star extension, notice the extension panels for the shape (e.g., "points")
2. Click on 'U' to show the SVG source, notice the shape="star" attribute
3. Click OK
4. Select the star, you will not see the extension panels
5. Click on 'U' to show the SVG source, the shape="star" attribute is now missing

You can get similar behavior from saving and reopening a shape. I assume that saving the SVG causes it to reimport all of the shapes into a blank canvas in a similar fashion as opening an image.

Expected behavior I would expect for the shapes to have the same behavior regardless of showing the source.

If this is the namespace issue, I don't know that I am sufficiently versed in the issue to suggest a fix. If it is not, I'll go looking for a fix (likely a poor thought out one).

SVG-Edit environment (IMPORTANT)

• Version: 4.2.0

Desktop (please complete the following information):

• OS: OSX
• Browser chrome
• Version 72

[sjernigan]

The "fix" for me was rather simple...adding 'shape' to the white list for various SVG elements in svgedit-config-iife.js. I don't know if you would consider this to be a customization or if it should it go into the sanitize.js or xdomain version. Without it the example extensions distributed with the editor don't import correctly. That said, they are extensions.

Also, as someone who's spent a limited about of time with the code. It would be helpful for new people for the readme to say a few words about when to use which versions and where the suggested locations to make customizations. I have found various pages on this topic but I feel like I'm missing the big picture. For instance, I don't understand why the iife completely duplicate the content in the other files as oppose to augment them. If they are always loaded, what's the point of the other files. Would you prefer this as a separate issue?

[cuixiping]

You can add some property names to the whitelist as your need.

Edit editor/sanitize.js, line 43.
Append 'shape' and some other necessary names.

polygon: [.........., 'shape', 'cx', 'cy', 'r', 'r2', 'point', 'orient', 'radialshift', 'size', 'edge', 'sides'],

[brettz9]

If the shape attribute is not a standard part of SVG, I think this should actually be fixed by implementing attribute behavior via standard HTML/SVG data-* attributes. This would work in HTML or XHTML/SVG-as-XML. Sound reasonable?

[cuixiping]

Agree with @brettz9 . data-* attributes is much better.

And using json style data as the attribute value of extension data is more flexible. json can save data type but string value can not.

For Example:

    data-smart-shape="{shape:'star',point:5,cx:57,cy:336,r:42,r2:17}"

It's not standard json format. Hjson or JSON5 are good choices.

[brettz9]

So I guess the priority here is on reduced file size rather than the convenience of multiple data-* attributes:

<polygon
data-smart-shape="star"
data-smart-point="5"
data-smart-cx="57"
data-smart-cy="336"
data-smart-r="42"
data-smart-r2="17"
>

...allowing for:

polygon.dataset.smartCx = 58;
polygon.dataset.smartR2--;

?

[brettz9]

The one advantage of namespaces is that they are designed to be usable cross-application, whereas data-* attributes are not; still, given the trend toward HTML over XHTML (and as HTML has at least been getting custom elements), data-* is still probably the way to go...

[sjernigan]

The change made should resolve the current bug. The suggested data-* approach sounds like it would be more standard. As such, I would suggest the label of this issue be changed from "bug" to "enhancement".

[brettz9]

We'd need a PR to actually change to data-*, but then yes, there shouldn't be any "loss" at that time once the change has been made. But I'd still kind of consider it a bug as something is not being preserved which should, regardless of the fact that our intent is to eliminate the cause of the breakage in this instance.

[sjernigan]

Sorry, I thought cuixiping's 52613a3 was to this repo.

[brettz9]

Yeah, no, that is just whitelisting the non-data attributes rather than changing the code to use the appropriate data-* attributes. Shouldn't be too hard for a PR if someone can offer...

[cuixiping]

Inkscape use extra xml namespace to save these attributes.

[brettz9]

That is compelling for adding compatibility...