Modify

Opened 3 weeks ago

Closed 3 weeks ago

Last modified 3 weeks ago

#18102 closed defect (fixed)

support svg defining currentColor without color

Reported by: Klumbumbus Owned by: Don-vip
Priority: normal Milestone:
Component: External imagery source Version:
Keywords: icon imagery svg argentina Cc:

Description

The first was added in wiki:Maps/Argentina?action=diff&version=86

data:image/svg+xml;base64,PHN2ZyBjbGFzcz0ic3ZnLWlubGluZS0tZmEgZmEtZG93bmxvYWQgZmEtdy0xNiIgYXJpYS1oaWRkZW49InRydWUiIGRhdGEtZmEtcHJvY2Vzc2VkPSIiIGRhdGEtcHJlZml4PSJmYXMiIGRhdGEtaWNvbj0iZG93bmxvYWQiIHJvbGU9ImltZyIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIiB2aWV3Qm94PSIwIDAgNTEyIDUxMiI+PHBhdGggZmlsbD0iY3VycmVudENvbG9yIiBkPSJNMjE2IDBoODBjMTMuMyAwIDI0IDEwLjcgMjQgMjR2MTY4aDg3LjdjMTcuOCAwIDI2LjcgMjEuNSAxNC4xIDM0LjFMMjY5LjcgMzc4LjNjLTcuNSA3LjUtMTkuOCA3LjUtMjcuMyAwTDkwLjEgMjI2LjFjLTEyLjYtMTIuNi0zLjctMzQuMSAxNC4xLTM0LjFIMTkyVjI0YzAtMTMuMyAxMC43LTI0IDI0LTI0em0yOTYgMzc2djExMmMwIDEzLjMtMTAuNyAyNC0yNCAyNEgyNGMtMTMuMyAwLTI0LTEwLjctMjQtMjRWMzc2YzAtMTMuMyAxMC43LTI0IDI0LTI0aDE0Ni43bDQ5IDQ5YzIwLjEgMjAuMSA1Mi41IDIwLjEgNzIuNiAwbDQ5LTQ5SDQ4OGMxMy4zIDAgMjQgMTAuNyAyNCAyNHptLTEyNCA4OGMwLTExLTktMjAtMjAtMjBzLTIwIDktMjAgMjAgOSAyMCAyMCAyMCAyMC05IDIwLTIwem02NCAwYzAtMTEtOS0yMC0yMC0yMHMtMjAgOS0yMCAyMCA5IDIwIDIwIDIwIDIwLTkgMjAtMjB6Ij48L3BhdGg+PC9zdmc+

and is displayed fine at least in Firefox. It decodes to:

<svg class="svg-inline--fa fa-download fa-w-16" aria-hidden="true" data-fa-processed="" data-prefix="fas" data-icon="download" role="img" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 512 512"><path fill="currentColor" d="M216 0h80c13.3 0 24 10.7 24 24v168h87.7c17.8 0 26.7 21.5 14.1 34.1L269.7 378.3c-7.5 7.5-19.8 7.5-27.3 0L90.1 226.1c-12.6-12.6-3.7-34.1 14.1-34.1H192V24c0-13.3 10.7-24 24-24zm296 376v112c0 13.3-10.7 24-24 24H24c-13.3 0-24-10.7-24-24V376c0-13.3 10.7-24 24-24h146.7l49 49c20.1 20.1 52.5 20.1 72.6 0l49-49H488c13.3 0 24 10.7 24 24zm-124 88c0-11-9-20-20-20s-20 9-20 20 9 20 20 20 20-9 20-20zm64 0c0-11-9-20-20-20s-20 9-20 20 9 20 20 20 20-9 20-20z"></path></svg>

Attachments (0)

Change History (15)

comment:1 Changed 3 weeks ago by Klumbumbus

Keywords: icon imagery added

comment:2 Changed 3 weeks ago by Don-vip

Milestone: 19.09
Owner: changed from team to Don-vip
Status: newassigned

comment:3 Changed 3 weeks ago by Don-vip

Error in logs:

2019-08-31 13:19:36.589 WARNING: Could not parse path svgSalamander:/currentColor
java.net.MalformedURLException: unknown protocol: svgsalamander
	at java.net.URL.<init>(URL.java:600)
	at java.net.URL.<init>(URL.java:490)
	at java.net.URL.<init>(URL.java:439)
	at java.net.URI.toURL(URI.java:1089)
	at com.kitfox.svg.SVGUniverse.getElement(SVGUniverse.java:345)
	at com.kitfox.svg.SVGUniverse.getElement(SVGUniverse.java:310)
	at com.kitfox.svg.ShapeElement.renderShape(ShapeElement.java:163)
	at com.kitfox.svg.Path.render(Path.java:94)
	at com.kitfox.svg.Group.render(Group.java:205)
	at com.kitfox.svg.SVGRoot.render(SVGRoot.java:335)
	at com.kitfox.svg.SVGRoot.renderToViewport(SVGRoot.java:266)
	at com.kitfox.svg.SVGDiagram.render(SVGDiagram.java:111)
	at org.openstreetmap.josm.tools.ImageProvider.createImageFromSvg(ImageProvider.java:1671)
	at org.openstreetmap.josm.tools.ImageResource.getImageIcon(ImageResource.java:163)
	at org.openstreetmap.josm.tools.ImageResource.getImageIconBounded(ImageResource.java:276)
	at org.openstreetmap.josm.tools.ImageResource.getImageIconBounded(ImageResource.java:229)
	at org.openstreetmap.josm.tools.ImageResource.attachImageIcon(ImageResource.java:104)
	at org.openstreetmap.josm.actions.AddImageryLayerAction.lambda$1(AddImageryLayerAction.java:83)
...
	at org.openstreetmap.josm.tools.ImageProvider.getResourceAsync(ImageProvider.java:749)
	at org.openstreetmap.josm.actions.AddImageryLayerAction.<init>(AddImageryLayerAction.java:81)
	at org.openstreetmap.josm.gui.ImageryMenu.refreshImageryMenu(ImageryMenu.java:173)

comment:4 Changed 3 weeks ago by stoecker

In 15326/josm:

see #18102 - better support for data: url images

comment:5 Changed 3 weeks ago by stoecker

To me the fill="currentColor" looks very strange. Is this really correct? I'd say JOSM is right and the image needs to be fixed.

comment:6 Changed 3 weeks ago by stoecker

It is an HTML+SVG interlinking method. As we have no HTML here it makes little sense. currentColor should simply be replaced by a proper color definition for the JOSM use case.

comment:7 Changed 3 weeks ago by Klumbumbus

It could fall back to e.g. black.

comment:8 Changed 3 weeks ago by stoecker

Yeah, but why support it in the first place? Changing the image is the better solution.

comment:9 Changed 3 weeks ago by Don-vip

Keywords: base64 removed
Milestone: 19.09
Resolution: invalid
Status: assignedclosed
Summary: support base64 coded svgsupport svg defining currentColor without color

This has nothing to do with base64.

currentColor is valid, as per https://www.w3.org/TR/SVG11/painting.html#SpecifyingPaint

Indicates that painting is done using the current animated value of the color specified by the ‘color’ property. This mechanism is provided to facilitate sharing of color attributes between parent grammars such as other (non-SVG) XML. This mechanism allows you to define a style in your HTML which sets the ‘color’ property and then pass that style to the SVG user agent so that your SVG text will draw in the same color.

svgSalamander supports it:

        if (styleAttrib.getStringValue().equals("currentColor"))
        {
            StyleAttribute currentColorAttrib = new StyleAttribute();
            if (getStyle(currentColorAttrib.setName("color")))
            {
                if (!currentColorAttrib.getStringValue().equals("none"))
                {
                    return currentColorAttrib.getColorValue();
                }
            }
            return null;
        }

But the SVG is invalid as it doesn't define any color. It must be fixed.

comment:10 in reply to:  9 ; Changed 3 weeks ago by stoecker

Replying to Don-vip:

This has nothing to do with base64.

currentColor is valid, as per https://www.w3.org/TR/SVG11/painting.html#SpecifyingPaint

Indicates that painting is done using the current animated value of the color specified by the ‘color’ property. This mechanism is provided to facilitate sharing of color attributes between parent grammars such as other (non-SVG) XML. This mechanism allows you to define a style in your HTML which sets the ‘color’ property and then pass that style to the SVG user agent so that your SVG text will draw in the same color.

svgSalamander supports it:

        if (styleAttrib.getStringValue().equals("currentColor"))
        {
            StyleAttribute currentColorAttrib = new StyleAttribute();
            if (getStyle(currentColorAttrib.setName("color")))
            {
                if (!currentColorAttrib.getStringValue().equals("none"))
                {
                    return currentColorAttrib.getColorValue();
                }
            }
            return null;
        }

But the SVG is invalid as it doesn't define any color. It must be fixed.

Hmm. I don't understand this comment. "currentColor" is defined outside the SVG, which we don't have, as there is no "outside" in our case.

comment:12 in reply to:  10 ; Changed 3 weeks ago by Don-vip

Replying to stoecker:

Hmm. I don't understand this comment. "currentColor" is defined outside the SVG, which we don't have, as there is no "outside" in our case.

I understand all the "parent grammar" stuff as an additional possibility of the simple case where a parent SVG tag defines the color property (which is what svgSalamander expects).

comment:13 Changed 3 weeks ago by Don-vip

Component: CoreExternal imagery source
Keywords: argentina added
Resolution: invalidfixed
Type: enhancementdefect

comment:14 in reply to:  12 Changed 3 weeks ago by stoecker

Replying to Don-vip:

Replying to stoecker:

Hmm. I don't understand this comment. "currentColor" is defined outside the SVG, which we don't have, as there is no "outside" in our case.

I understand all the "parent grammar" stuff as an additional possibility of the simple case where a parent SVG tag defines the color property (which is what svgSalamander expects).

So essentially you also suppose to replace currentColor with a fixed definition for this image?

comment:15 Changed 3 weeks ago by Don-vip

I suppose nothing, I just see this image is not compatible with what svgSalamander expects, thus needed to be changed, and Stefan changed it :)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Don-vip.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.