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

[bug] Improper XML and DOCTYPE Declarations #97

Closed
kvid opened this issue Jul 17, 2020 · 5 comments
Closed

[bug] Improper XML and DOCTYPE Declarations #97

kvid opened this issue Jul 17, 2020 · 5 comments
Labels
bug Something isn't working

Comments

@kvid
Copy link
Collaborator

kvid commented Jul 17, 2020

The HTML output violates these rules:

The root cause is that Harness.output() start HTML output with file.write('<html><body style="font-family:Arial">') (i.e. no leading XML or DOCTYPE declarations) and then later embed the SVG diagram into the HTML output without hiding (remove or comment) the leading XML and DOCTYPE declarations from the SVG file.

If we are including XML and/or DOCTYPE declarations, then we should comply to their rules. Related issues I have not investigated:

The answer is no to both questions above, as show in my comment below with errors from the W3C Markup Validation Service.

@aakatz3
Copy link
Contributor

aakatz3 commented Jul 17, 2020

The svg doctype is generated by graphviz itself. The best way to deal with this would be a complete refactor of HTML generation, using a library. I'm not amazing at python, but I found a couple that can be tried.

@kvid
Copy link
Collaborator Author

kvid commented Jul 17, 2020

The svg doctype is generated by graphviz itself.

I know that, but it's not hard to filter out the leading declarations before embedding the rest of the SVG file.

The best way to deal with this would be a complete refactor of HTML generation, using a library.

I cannot see how a library will solve this issue. It might be a good idea for other reasons, but that belongs in a separate issue. If we want to add proper leading declarations, then it's easy to include them in the first file.write(). IMHO, "a complete refactor of HTML generation" is overkill in this case.

The question is: Do we want any of these declarations at all?

@aakatz3
Copy link
Contributor

aakatz3 commented Jul 17, 2020

My thought was that if we use the xml.dom.minidom library, we can easily strip out or modify tags as necessary from the Graphviz SVG. Similarly, if we use that library, or a similar one for HTML, we can then just pass an object instead of repeatedly reading and writing files.

The doctype and meta tags are probably pretty important for W3C compliance 1 2, if we care about that.

A proper XML declaration does help, but a meta encoding tag in HTML is also needed (I implemented in #96 )

@kvid
Copy link
Collaborator Author

kvid commented Jul 18, 2020

The W3C Markup Validation Service complained with these errors:

  • When using XML declaration: Saw <?. Probable cause: Attempt to use an XML processing instruction in HTML. (XML processing instructions are not supported in HTML.)
  • When not using DOCTYPE declaration: Start tag seen without seeing a doctype first. Expected <!DOCTYPE html>.
  • When SVG DOCTYPE declaration was not removed: Stray doctype.
  • Missing and obsolete elements in the HTML, but that belongs in a separate issue.

@formatc1702
Copy link
Collaborator

Thanks for the PR, this fixes the primary issue.

The other minor issues should be addressed as part of #32 and the feature/technical-drw branch.

kvid added a commit to kvid/WireViz that referenced this issue Oct 15, 2020
The https://validator.w3.org/ reported Errors:
The align attribute on the th/td element is obsolete. Use CSS instead.

By replacing align="X" attributes with text-align:X; CSS equivalent,
the validator now completes without any errors or warnings.

This solves the remaining issues from wireviz#97.
kvid added a commit to kvid/WireViz that referenced this issue Oct 15, 2020
The https://validator.w3.org/ reported Errors:
The align attribute on the th/td element is obsolete. Use CSS instead.

By replacing align="X" attributes with text-align:X; CSS equivalent,
the validator now completes without any errors or warnings.

This solves the remaining issues from wireviz#97.
kvid added a commit to kvid/WireViz that referenced this issue Oct 16, 2020
The https://validator.w3.org/ reported Errors:
The align attribute on the th/td element is obsolete. Use CSS instead.

By replacing align="X" attributes with text-align:X; CSS equivalent,
the validator now completes without any errors or warnings.

This solves the remaining issues from wireviz#97.
formatc1702 pushed a commit that referenced this issue Oct 16, 2020
The https://validator.w3.org/ reported Errors:
The align attribute on the th/td element is obsolete. Use CSS instead.

By replacing align="X" attributes with text-align:X; CSS equivalent,
the validator now completes without any errors or warnings.

This solves the remaining issues from #97.
@kvid kvid added the bug Something isn't working label Aug 18, 2023
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

No branches or pull requests

3 participants