Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

no svg validation if jock.svg is also installed #805

Closed
BuZZ-dEE opened this issue Nov 15, 2022 · 18 comments · Fixed by #806
Closed

no svg validation if jock.svg is also installed #805

BuZZ-dEE opened this issue Nov 15, 2022 · 18 comments · Fixed by #806
Labels
bug Something isn't working
Milestone

Comments

@BuZZ-dEE
Copy link

BuZZ-dEE commented Nov 15, 2022

No svg validation if jock.svg is also installed. If for example the svg has a group and you remove the end tag </g>, then you get no error, so the validation does not work.

angelozerr pushed a commit to angelozerr/vscode-xml that referenced this issue Nov 15, 2022
Fixes redhat-developer#805

Signed-off-by: azerr <azerr@redhat.com>
@datho7561 datho7561 added this to the 0.23.0 milestone Nov 15, 2022
@datho7561 datho7561 added the bug Something isn't working label Nov 15, 2022
datho7561 pushed a commit that referenced this issue Nov 15, 2022
Fixes #805

Signed-off-by: azerr <azerr@redhat.com>
@BuZZ-dEE
Copy link
Author

BuZZ-dEE commented Nov 15, 2022

Thx for your fast fix. Release it 🚀

@angelozerr
Copy link
Contributor

@BuZZ-dEE once build we be done you will able to install vscode-xml prerelease to enjoy with this fix

@angelozerr
Copy link
Contributor

@BuZZ-dEE you can install pre-release to enjoy with this fix. Please give me feedback.

You should set xml.validation.resolveExternalEntities to true, see #810

@BuZZ-dEE
Copy link
Author

BuZZ-dEE commented Nov 17, 2022

Hi @angelozerr I will try it, but what do I need from: releases/tag/latest ?

@angelozerr
Copy link
Contributor

Try to install vscode-xml as you did, and click on Switch To Pre Release version

image

@BuZZ-dEE
Copy link
Author

BuZZ-dEE commented Nov 17, 2022

g: Group Element ..................................

Source: [svg11.dtd](vscode-file://vscode-app/usr/lib/code/out/vs/code/electron-sandbox/workbench/workbench.html)

The SVG element is a container used to group other SVG elements.

[MDN Reference](vscode-file://vscode-app/usr/lib/code/out/vs/code/electron-sandbox/workbench/workbench.html)

The element type "g" must be terminated by the matching end-tag "</g>".xml(ETagRequired)
test.svg(54, 9): Closing tag expected here

It works now. (Content from hovering over the g element above)

@angelozerr
Copy link
Contributor

Thanks for your feedback.

The problem now is with completion. You will see 2 duplicate completion item for the same element which comes from jock.svg and vscode-xml.

I think vscode-xml could support the same feature than jock.svg like better hover documentation and color for rgb attribute.

I will see if jock.svg could be interested to delegate thise features to vscode-xml

@BuZZ-dEE
Copy link
Author

BuZZ-dEE commented Nov 17, 2022

@angelozerr I have created lishu/vscode-svg2#129 just to make vscode-svg2 / jock.svg aware if the issue.

@lishu
Copy link

lishu commented Dec 17, 2022

@BuZZ-dEE
I installed the vscode-xml plugin and found no duplicate completions, whether some specific configuration or version has changed?

@angelozerr
Copy link
Contributor

@lishu have you svg validation which is working?

Could you share your svg file please. It requires the svg namespace if I remember

@lishu
Copy link

lishu commented Dec 17, 2022

Maybe it's not working properly.

image

@angelozerr
Copy link
Contributor

Could you share please your svg file

@angelozerr
Copy link
Contributor

I tested it and you can see duplicate completion items:

image

Here my svg file:

<?xml version="1.0" standalone="no"?>
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" 
  "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
<svg width="5cm" height="4cm" version="1.1"
     xmlns="http://www.w3.org/2000/svg">
     <a xlink:href=""></a>
  <desc>Four separate rectangles
  </desc>
    <a
    <rect x="0.5cm" y="0.5cm" width="2cm" height="1cm"/>
    <rect x="0.5cm" y="2cm" width="1cm" height="1.5cm"/>
    <rect x="3cm" y="0.5cm" width="1.5cm" height="2cm"/>
    <rect x="3.5cm" y="3cm" width="1cm" height="0.5cm"/>

  <!-- Show outline of canvas using 'rect' element -->
  <rect x=".01cm" y=".01cm" width="4.98cm" height="3.98cm"
        fill="none" stroke="blue" stroke-width=".02cm" />

</svg>

You can notice you need to declare DOCTYPE.

Is the validation working for you?

@angelozerr
Copy link
Contributor

I noticed in the log the same error than you, but is is working for me. We need to invesitgate this error log.

@angelozerr
Copy link
Contributor

I noticec that the first completion item a (coming from vscode-xml) generates:

<a xlink:href=""></a>

which is valid according to the DTD, although your extension generates:

<a href="#url"></a>

which seems not be valid. Is it allowed?

@lishu
Copy link

lishu commented Dec 17, 2022

Yes, my svg doesn't use DOCTYPE

DOCTYPE - MDN does not recommend using DOCTYPE

xlink:href - Deprecated XLink URL reference attributes

@angelozerr
Copy link
Contributor

Yes, my svg doesn't use DOCTYPE

LemMinx doesn't provide a specific SVG support, so you need to declare the DOCTYPE with the proper DTD. But it is possible to write a little Java LemMinx extension which does that. But I think you will not be interested to provide that.

My another idea is to gives the capability to do that with package.json in your extension point (no dependency to vscode-xml), but before developping that, we need some user requirements.

@lishu
Copy link

lishu commented Dec 17, 2022

There are many conflicts between the standards under HTML and the XML standard, so it is indeed a problem that is not easy to handle 🤣

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants