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.

Sign up for GitHub

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

Jump to bottom

[Proposal] Keep/add xmlns attribute for generated SVG images #23409

Closed
wxiaoguang opened this issue Mar 10, 2023 · 4 comments · Fixed by #23410
Closed

[Proposal] Keep/add xmlns attribute for generated SVG images #23409

wxiaoguang opened this issue Mar 10, 2023 · 4 comments · Fixed by #23410
Labels
type/enhancement An improvement of existing functionality type/proposal The new feature has not been accepted yet but needs to be discussed first.
Milestone
1.20.0

Comments

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 10, 2023 •
edited
Loading

Feature Description

Gitea's SVG files do not have the xmlns attribute. Without it, these SVG images couldn't be display correctly.

  • Browsers (Safari/Chrome/Firefox) only display the XML content, but no the image
  • VSCode shows 0 x 0 broken images
  • macOS Finder show blank page

Then developers couldn't browse these images, they couldn't know what the images look like before rendering them as inline SVG in a page.

So it's better to keep/add the xmlns attribute for the generated SVG files.

https://stackoverflow.com/questions/18467982/are-svg-parameters-such-as-xmlns-and-version-needed

cc @silverwind

Screenshots

image

image
@wxiaoguang wxiaoguang added type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first. labels Mar 10, 2023
@silverwind
Copy link
Member

Yes it would be welcome to add xmlns to SVGs before rendering. GitHub also recently did this change.

@silverwind
Copy link
Member

silverwind commented Mar 10, 2023 •
edited
Loading

Actually I misread. You are talking about SVG assets, I'm talking about user content SVG image rendering.

xmlns is optional for SVG embedded inside HTML. It's only necessary when a SVG is rendered as it's own document, which we don't do. Adding it into HTML rendering would blow up our HTML unnecessarily by a few kilobytes, GitHub also does not do this.

@wxiaoguang
Copy link
Contributor Author

Yup, for rendering, they do not need the xmlns.

We can remove the attribute before rendering, how do you think?

@wxiaoguang wxiaoguang mentioned this issue Mar 10, 2023
Merged
@wxiaoguang
Copy link
Contributor Author

The PR: Keep/add xmlns attribute for generated SVG images #23410

The output is still clear, and developers could understand/choose SVG images quickly.

@lunny lunny added type/enhancement An improvement of existing functionality and removed type/feature Completely new functionality. Can only be merged if feature freeze is not active. labels Mar 17, 2023
@lunny lunny added this to the 1.20.0 milestone Mar 17, 2023
lunny pushed a commit that referenced this issue Mar 21, 2023
@wxiaoguang @silverwind @delvh
Keep (add if not existing) xmlns attribute for generated SVG images (#…
a797b84 
…23410)

Fix #23409 

Developers could browse & preview the local SVG images files directly.

It still has clear output.

![image](https://user-images.githubusercontent.com/2114189/224317107-f4b26c76-e36a-4e80-9eee-d8dc2e16421f.png)


![image](https://user-images.githubusercontent.com/2114189/224317527-2d4ca131-978c-4933-b071-4bae483f06e1.png)

---------

Co-authored-by: silverwind <[email protected]>
Co-authored-by: delvh <[email protected]>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
type/enhancement An improvement of existing functionality type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Milestone
1.20.0
Development

Successfully merging a pull request may close this issue.

Keep (add if not existing) xmlns attribute for generated SVG images wxiaoguang/gitea
3 participants
@lunny @silverwind @wxiaoguang