Closed ghost closed 1 year ago
The following exceptions are thrown:
* `org.w3c.dom.DOMException`: The "stroke" property does not support HSLCOLOR values.
Fixed, now EchoSVG supports level 4 CSS colors as well as the Level 5 color-mix()
function. The next image shows the before/after the patch, the previous render (left) was identical to Inkscape:
[...] seems like something that the DOM parser should issue a warning about, but not stop parsing. This is Inkscape's behaviour and it would be useful if there was an API option to indicate that parsing should try to continue until an unrecoverable error. [...] Having a way to reduce all similar exceptions in this class to warnings, rather than an exception, with the ability to register a warning listener (no-op listener by default) would be useful.
This is a known issue, and EchoSVG already does much better than Batik or Inkscape (unknown rules are just ignored instead of causing the whole stylesheet parsing to fail, which is what Batik does).
Fixing this in full is non-trivial due to the design of Batik's CSSEngine
(for example one cannot just proceed with processing after any value error, due to that value potentially being part of a shorthand or a function).
That said, I have figured out a possible clean solution which I may implement in the future.
* `java.lang.NullPointerException`: Cannot invoke "io.sf.carte.echosvg.util.ParsedURL.toString()" because "uri" is null
[...] is a bug. When
uri
isnull
, callinguri.toString()
fails. See line 1202:String s = Messages.formatMessage("stylesheet.syntax.error", new Object[] { uri.toString(), rules, m });
The root cause is in
SVGOMStyleElement.java
, lines 133 to 139, where it is assumed that a base URI is set for the document. That's not necessarily true (e.g., an SVG file can be streamed from a generator that has no URI). I traced thenull
toAbstractNode.java
, line 435, which assigns the base URI todoc.getDocumentURI()
(for an element that has no attributes). This lets thenull
pass through.The simplest fix, of course, is not to call
uri.toString()
whenuri
isnull
. A deeper fix would be to correct the assumption of a URI that always exists and define a fake URI that can be used when no URI is otherwise determined.
Documents are supposed to always have a documentURI
set, and I encourage you to do that. It is used for security purposes, for example the same-origin policy which is used by default (although it can be configured to a less strict policy, CORS is not implemented and I doubt that it will ever be).
However, I have committed a fix so this NPE should never happen again.
Not sure if my comment made it into the PR before the change was committed. FWIW, the conditional around checking for null
strings can be simplified using Objects
:
import java.util.Objects;
Object o1 = null;
Object o2 = "aString";
String s;
s = Objects.toString(o1, "isNull"); // returns "isNull"
s = Objects.toString(o2, "isNull"); // returns "aString"
Not sure if my comment made it into the PR before the change was committed. FWIW, the conditional around checking for
null
strings can be simplified usingObjects
:import java.util.Objects; Object o1 = null; Object o2 = "aString"; String s; s = Objects.toString(o1, "isNull"); // returns "isNull" s = Objects.toString(o2, "isNull"); // returns "aString"
Thanks for the suggestion, although it wasn't a PR but a direct commit to the repository.
That said, if using Objects.toString
is important for your peace of mind feel free to send a PR. 🙂 But it's the same thing internally and, unless the JVM inlines it, it may have to handle the stack because of entering a new method, which would be slower. So I wouldn't waste time.
Replicate
Produce a Mermaid diagram having the following input (see Mermaid live or Kroki, or the attached diagram.svg.txt):
sequenceDiagram Alice->>+John: Hello John, how are you? Alice->>+John: John, can you hear me? John-->>-Alice: Hi Alice, I can hear you! John-->>-Alice: I feel great
Rasterize the SVG diagram using EchoSVG.
Expected
Diagram renders (at least partially). Here's how Inkscape renders the diagram:
Actual
The following exceptions are thrown:
org.w3c.dom.DOMException
: The "stroke" property does not support HSLCOLOR values.java.lang.NullPointerException
: Cannot invoke "io.sf.carte.echosvg.util.ParsedURL.toString()" because "uri" is nullThe first error seems like something that the DOM parser should issue a warning about, but not stop parsing. This is Inkscape's behaviour and it would be useful if there was an API option to indicate that parsing should try to continue until an unrecoverable error. Missing colours aren't the end of the world. See
CSSEngine.java
, line 1194:That eventually calls
AbstractValueFactory.java
, lines 59 to 63:Having a way to reduce all similar exceptions in this class to warnings, rather than an exception, with the ability to register a warning listener (no-op listener by default) would be useful.
Forcing calling classes to register an "exception handler" would be a viable alternative, and could look like:
That would leave the choice about whether to throw an exception up to the calling code.
The second error is a bug. When
uri
isnull
, callinguri.toString()
fails. See line 1202:The root cause is in
SVGOMStyleElement.java
, lines 133 to 139, where it is assumed that a base URI is set for the document. That's not necessarily true (e.g., an SVG file can be streamed from a generator that has no URI). I traced thenull
toAbstractNode.java
, line 435, which assigns the base URI todoc.getDocumentURI()
(for an element that has no attributes). This lets thenull
pass through.The simplest fix, of course, is not to call
uri.toString()
whenuri
isnull
. A deeper fix would be to correct the assumption of a URI that always exists and define a fake URI that can be used when no URI is otherwise determined.