BigBadaboom / androidsvg

SVG rendering library for Android
http://bigbadaboom.github.io/androidsvg/
Apache License 2.0
1.21k stars 231 forks source link

@import of external CSS file not working #200

Closed ewaldc closed 4 years ago

ewaldc commented 4 years ago

The attached SVG file does not call resolveCSSStyleSheet in CSSParser.java (line 1169) The issue seems to be here: List<MediaType> mediaList = parseMediaList(scan); if (!scan.empty() && !scan.consume(';')) throw new CSSParseException("Invalid @media rule: expected '}' at end of rule set"); if (SVG.getFileResolver() != null && mediaMatches(mediaList, deviceMediaType)) { String css = SVG.getFileResolver().resolveCSSStyleSheet(file);

mediaList seems to result in an empty list, and thus mediaMatches(mediaList, deviceMediaType) is false. html.zip

The SVG renders fine in several tested browsers and as I understand, @import should be supported for CSS files.

ewaldc commented 4 years ago

I found the problem. Medialists are optional, so a mediaList() with size 0 is an acceptable condition to invoke resolveCSSStyleSheet() Happy to post a PR is this is easier for you. I have forked androidsvg but I had to make more changes to publish it in jitpack.io and allow it to compile in a broader project (Gradle) so I can't just provide a straight PR from that fork. androidsvg/src/main/java/com/caverock/androidsvg/CSSParser.java @@ -1166,7 +1166,7 @@ else if (!inMediaRule && atKeyword.equals("import"))

         if (!scan.empty() && !scan.consume(';'))
            throw new CSSParseException("Invalid @media rule: expected '}' at end of rule set");

  -       if (SVG.getFileResolver() != null && mediaMatches(mediaList, deviceMediaType)) {
 +       if (SVG.getFileResolver() != null && (mediaList.size() == 0 || mediaMatches(mediaList, deviceMediaType))) {
            String  css = SVG.getFileResolver().resolveCSSStyleSheet(file);
            if (css == null)
               return;
BigBadaboom commented 4 years ago

Hi. Thanks for the report. The diff you have already posted is fine. I'll merge it when I get a chance.

Thanks very much.

ewaldc commented 4 years ago

Wonderful! The best problems are the ones you help to solve :-)

BigBadaboom commented 4 years ago

I have just had a proper look at this, and I am not able to reproduce the issue as reported. In my tyest program itgets read and parsed as expected. I see the correct text colours in the rendered SVG.

The exception, related to @media, that you are seeing should not be affecting the @import, as it happens later in the file. Support for media queries is extremely basic at present. We basically just support simple @media screen {} etc. The failure to parse your media query will mean that the CSS rules inside the media query, and the one after it (.grid {}) won't be parsed properly. For now, I would recommend avoiding media query blocks.

Can you retest please, and let me know if you are still seeing the issue. Perhaps something else causing your problem?

ewaldc commented 4 years ago

Sorry, I don't understand ... In order for String css = SVG.getFileResolver().resolveCSSStyleSheet(file); to be called, two conditions need to be fulfilled. Since I provided a callback, SVG.getFileResolver() != null will of course be true, but the second condition mediaMatches(mediaList, deviceMediaType) will be false unless there is a mediaList defined. In the example, I posted, there is no mediaList, hence the style sheet does not resolve. When I make the posted change in the code, it renders properly. Have you tried the example I included in the first post? I have not been able to test directly with the master branch of your library as I did not find a quick way to test in standalone mode. I replicated it using OpenHab for Android, which includes v1.4. However, before opening the issue, I compared v1.4. and the master branch and did not see anything that would cause it to behave differently.

BigBadaboom commented 4 years ago

Sorry. I just realised I may not been testing it correctly. I'll have another look tomorrow.

BigBadaboom commented 4 years ago

Fixed.

@ewaldc Sorry for the confusion. It's a long story, but there was an issue with my test app that made me think it was working when it wasn't. Thanks for your help.