Adding support for <link> css inclusion #721

FabioFerrettiMoodys · 2020-04-29T06:31:53Z

adding support for <Link ref="stylesheet" url="">

Adding support for css inclusion

Read the content of a <link rel="stylesheet" href="..> and add it like a <style> node.

in order to do that, the base uri must be provided as the SvgDocument.BaseUri is set after the Open function.

Read the content of a <link rel="stylesheet" href="..> and add it like a <style> node. in order to do that, the base uri must be provided.

FabioFerrettiMoodys · 2020-04-29T09:03:46Z

here is why the test are important ;)

i got a error in Open of svgDocument : i forgot the else ;)

   public static T Open<T>(string path, Dictionary<string, string> entities, Uri baseUri = null) where T : SvgDocument, new()
    {
        if (string.IsNullOrEmpty(path))
        {
            throw new ArgumentNullException("path");
        }

        if (!File.Exists(path))
        {
            throw new FileNotFoundException("The specified document cannot be found.", path);
        }

        using (var stream = File.OpenRead(path))
        {
            var doc = Open<T>(stream, entities, baseUri == null ? new Uri(System.IO.Path.GetFullPath(path)) : baseUri);
            if (baseUri == null)
                doc.BaseUri = new Uri(System.IO.Path.GetFullPath(path));
            else
                doc.BaseUri = baseUri;
            return doc;
        }
    }

error in open sdocument

H1Gdev

Please add target SVG files to your test.

Source/SvgDocument.cs

test

mrbean-bremen · 2020-06-07T17:34:24Z

@H1Gdev - are there open review issues here?

H1Gdev · 2020-06-08T10:41:12Z

There is no <link> element in the SVG specifications.

SVG
SVG2

@FabioFerrettiMoodys

Please add target SVG files to your test.
Please tell me supported software (major browser etc.).

FabioFerrettiMoodys · 2020-06-08T11:45:40Z

Hello

https://www.w3.org/TR/SVG2/styling.html

6.3. External style sheets: the effect of the HTML ‘link’ element
An HTML ‘link’ element in an SVG document (that is, an element in the HTML namespace with local name "link") with its ‘rel’ attribute set to 'stylesheet' must be processed as defined in the HTML specification and cause external style sheets to be loaded and applied to the document. Such elements in HTML documents outside of an inline SVG fragment must also apply to the SVG content.

all major browser seems to support it
i've test this sample with chrome, ie11 and firefox.
http://www.schepers.cc/svg/style/external-link-xhtml.svg

H1Gdev · 2020-06-09T04:38:44Z

This PR has the following issues.

File output by Write is not supported.
Do not support the case if href is specified in URL.

First, make the following changes.

Create an element for <link>.
BaseUri is a user-settable property, so please resolve link href later.

H1Gdev · 2020-06-09T04:46:11Z

FYI

Output SVG with this PR is as follows.

<?xml version="1.0" encoding="utf-8"?>
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
<svg width="100%" height="100%" viewBox="0, 0, 50, 50" xhtml="http://www.w3.org/1999/xhtml" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns:xml="http://www.w3.org/XML/1998/namespace" version="1.1">
  <title>Styling Test</title>
  <desc>
    Tests linking to external stylesheet with "link" element in XHML namespace.  
    Seems to work in Firefox, Opera, and Safari, as of 18-01-2009
  </desc>
  <metadata>author: schepers, created: 18-01-2009</metadata>
  <link href="svgstyle.css" rel="stylesheet" type="text/css">circle
{
   fill: lime;
   stroke: green;
   stroke-width: 10px;
}
</link>
  <circle cx="25" cy="25" r="20" stroke-width="10px" style="fill:lime;stroke:green;" />
</svg>

FabioFerrettiMoodys · 2020-06-10T09:13:16Z

BaseUri is a user-settable property, so please resolve link href later.

i don't know how to do it, you collect all the styles in the function Open(XmlReader reader...
You create a "resulting css" named "cssTotal" and assign the styles to te elements at the end of these function : before we can set the BaseUri.
line 480 of svgDocument.cs
var cssTotal = styles.Select((s) => s.Content).Aggregate((p, c) => p + Environment.NewLine + c);

H1Gdev · 2020-06-10T11:37:38Z

@mrbean-bremen
@FabioFerrettiMoodys

By #90, BaseUri property can be set...
We plan to ensure backward compatibility as much as possible.

This is modification to review style application sequence by supporting the SVG2 specification.

FabioFerrettiMoodys · 2020-06-12T08:18:43Z

@H1Gdev

By #90, BaseUri property can be set...
We plan to ensure backward compatibility as much as possible.

This is modification to review style application sequence by supporting the SVG2 specification.

I'm not sure to follow you : it can be set but only after the Open. but the styling is done in the Open. (SvgDocument.cs line 507 : svgDocument?.FlushStyles(true); ).
i can make a link element and resolve the href later, but if a user set it, it will not work as all the styling is done at the open of the file. (if it's a new element, i will also need to redo the aggregate of styles at the end of the Open function. As the aggregate of the ISvGNode.Content will not work anymore.)

ps: sorry but i'm struggling to understand your comments and what and how you want me to do. I don't know your library as deep as you. Can you be more specific ?

for exemple what do you mean by "File output by Draw is not supported."

var svg = SvgDocument.Open(Source);
var image = svg.Draw();

this code work, do you mean something else ?

I also don't understand your "Output SVG with this PR is as follows."
how do you generate it like this ? i didn't alter the svgdocument structure, i've only added the css content to the list of styles used for the styling at the end of the Open.

H1Gdev · 2020-06-12T10:23:31Z

for exemple what do you mean by "File output by Draw is not supported."

It's a typo.

File output by Write is not supported.

H1Gdev · 2020-06-13T00:25:20Z

it can be set but only after the Open. but the styling is done in the Open. (SvgDocument.cs line 507 : svgDocument?.FlushStyles(true); ).

This is because it doesn't support <link>.
This is because we was able to solve style here.

Now, the changes that are considered are the following.

Make setting of BaseUri property private so that it can be set only in Open method.
Apply style before Draw.

The change of 1 cannot maintain backward compatibility.

Maybe, I think change of 2 will be like SvgDeferredPaintServer.

adding svgLink element for link correction of dowument.Write correction of non local uri for href param. adding a "NonSvgElementAttribute" and SvgElementFactory modification for theses elements.

FabioFerrettiMoodys · 2020-06-15T08:40:00Z

As the link element wasn't in the svg namespace i've added a NonSvgElementAttribute so the SvgElementFactory can create NonSvgElement by refraction like the SvgElement.

i've assigned the BaseUri just after the creation of the svgdocument in the open function : the svglink use the OwnerDocument.BaseUri when the content of the link is requested. (svgLink.GetLinkContentAsText)

i've corrected the svgDocument.Write : it write the element, but i don't know if it is what you want as the styles are already added on each elements (by the flushstyles function).

H1Gdev

Please apply EditorConfig first.

Source/Document Structure/SvgLink.cs

Source/SvgDocument.cs

Source/SvgElementFactory.cs

H1Gdev · 2020-06-15T12:02:52Z

Source/Document Structure/SvgLink.cs

+
+        public Stream GetLinkContentAsStream(RelativeValue rel = RelativeValue.Unknown)
+        {
+            if (Rel != rel && rel != RelativeValue.Unknown)


If you're doing a comparison with Rel property, why not use the value of Rel property value ??

it is to avoid as much as possible the use of the content with a incorrect rel type.

if you call GetLinkContentAsStream(stylesheet) on a link with rel="icon" for exemple, you will not get the content and cannot use it as a stylesheet by mistake (and make the code crash).

assuming you are absolutely sure of what you do and what you have, you can just call GetLinkContentAsStream(), you will get the content independently of the rel

these technique avoid to check everywhere if the Rel is correct every where you use the link content, here it is done in only one place.

I can remove if if you prefer.

Source/Document Structure/SvgLink.cs

mrbean-bremen · 2020-12-27T14:15:42Z

@H1Gdev - can you please check if the current state can be merged? You did some review that has been addressed (admittetly, this was quite some time ago, but I hadn't been active in this project since then).

kimsey0 · 2021-05-14T12:10:08Z

(If #872 is merged and this pull request is resumed, it should check that SvgDocument.ResolveExternalResources == true before creating a WebRequest in GetLinkContentAsStream.)

paulushub · 2024-01-14T14:35:05Z

@FabioFerrettiMoodys Sorry for the delay. Please update your project for review.

kimsey0 · 2024-01-14T15:14:52Z

Source/Document Structure/SvgLink.cs

+                    uri = new Uri(OwnerDocument.BaseUri, uri);
+
+                // should work with http: and file: protocol urls
+                var httpRequest = WebRequest.Create(uri);


This should check that SvgDocument.ResolveExternalResources is true before creating a web request to avoid XSS vulnerabilities. See #873.

@kimsey0 Can you fix it by editing the PR source?

It's been too long since I worked on this project, so I can't really remember the details, nor how tests etc. are structured. I can see that #873 split SvgDocument.ResolveExternalResources into three properties: ResolveExternalXmlEntites, ResolveExternalImages, and ResolveExternalElements, probably to give more fine-grained control over which external resources are allowed to be resolved. You may want to introduce a new ExternalType ResolveExternalStylesheets property, somehow pass it in or make it available to this GetLinkContentAsStream method, and then trace a warning and return null or an empty string if !ResolveExternalStylesheets.AllowsResolving(uri).

@kimsey0 Thanks for the update. Just do not know the best way to handle these staled PR.

Take over (clone PR reposition and fix issues), the original owner looses his/her contribution

Close the PR and see how to reimplement the issue, a waste but may be worth it

adapt/finish the original PR, so it will just have 2 authors

adapt/finish the original PR, so it will just have 2 authors

Never done, can you outline the procedure and give me some hints?

If you are asking about the git workflow, something like:

git remote add FabioFerrettiMoodys https://github.com/FabioFerrettiMoodys/SVG
git checkout FabioFerrettiMoodys/master (probably under some other name)
git rebase origin/main -> resolve conflicts as usual
git push --force-with-lease
(or the equivalents if you use a git GUI client(

make your changes, commit, push...

when done and reviewed, squash the commit, edit the commit message, but leave the authors in the message

Is this what you are asking?

Is this what you are asking?

I think so, thanks - will look into it this weekend.

Depending on the policies of this project (the contribution guide doesn't say), you can also just avoid squashing the commits, leaving the original author for each one and making it clear who's done what.

the contribution guide doesn't say

Something to be added...

The current (undocumented) convention is that commits are usually squashed and only rebased if the individual commits are independent changes to be preserved, and have sensible commit comments (which is not the case here). Though it is possible to rebase the PR respectively before the merge, if a rebase is wanted.

Adding support for <link> css inclusion

48db17f

Read the content of a <link rel="stylesheet" href="..> and add it like a <style> node. in order to do that, the base uri must be provided.

adding support for <Link ref="stylesheet" url=""> (correction)

b09100c

error in open sdocument

H1Gdev reviewed Apr 29, 2020

View reviewed changes

Source/SvgDocument.cs Show resolved Hide resolved

Source/SvgDocument.cs Outdated Show resolved Hide resolved

Fabio Ferretti and others added 2 commits May 15, 2020 07:35

adding var

3bb451b

update source Merge pull request #1 from vvvv/master

62d3cc1

test

switching to SvgLink element

d313e10

adding svgLink element for link correction of dowument.Write correction of non local uri for href param. adding a "NonSvgElementAttribute" and SvgElementFactory modification for theses elements.