-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Fix some files are recognized as SVG incorrectly, make the diff UI unable to work #23310
Conversation
It should work (I manually constructed those files, one side is valid while other side is invalid)
What's the root problem of the breaking? eg: how the detection goes wrong? Maybe a case is like this: <!-- BEGIN: filter namings -->
<div>
<!-- prettier-ignore -->
<svg xmlns="http://www.w3.org/2000/svg" width="10" height="10" viewBox="0 0 10 10">
<path fill="#8E8E93" fill-rule="evenodd" d="M9.816 8.981L7.08 6.243c.49-.67.753-1.479.75-2.309C7.819 1.767 6.067.013 3.9 0 2.864-.005 1.868.406 1.135 1.14.403 1.872-.006 2.868 0 3.905.01 6.072 1.762 7.828 3.93 7.84c.833.004 1.645-.262 2.315-.758l.003-.002 2.735 2.736c.148.156.368.218.576.165.207-.054.37-.216.423-.424.054-.207-.01-.428-.165-.576zm-5.89-1.925C2.193 7.046.79 5.642.783 3.909c-.005-.83.322-1.626.908-2.213.586-.587 1.383-.915 2.212-.912 1.733.01 3.135 1.414 3.143 3.147.004.83-.323 1.626-.909 2.213-.586.587-1.382.915-2.211.912z"/>
</svg>
</div> |
The initial problem is php files containing svg are not displaying as gitea returns svg mime type for them. The svg regex cause this issue Example of the current regex used https://regex101.com/r/m1bCCP/1 And also on latest firefox (110.0.1) this is what I see |
Yup, so I guess the detection should be improved, instead of removed? |
There's so many cases when browser won't display an svg (e.g. https://try.gitea.io/stuzer05/test-preview/raw/commit/72f4584ea11cf8579f57d1ab909a6d7d349bc4e7/test%20%28copy%201%29.svg), I think it's better to remove this smart parsing. It's just unreliable |
If remove the smart detection, will it make the SVG files won't be shown as image in the diff view? |
Yeah. I think |
Have you tried that? According to Golang's issue tracker:
I will take a look at this problem , and fix some more SVG related problems. |
No, I didn't check that. Maybe we can detect svg by file extension alone. That should actually cover most cases. Embedded svgs shouldn't really be detected (e.g. in html) as we don't want to display html content in diff preview |
Thank you! Maybe if you could make svg regex not match anything between html comments, except spaces and |
The problem is that the detector fails on this test: assert.False(t, DetectContentType([]byte(`
<!-- comment1 -->
<div>
<!-- comment2 -->
<svg></svg>
</div>
`)).IsSvgImage()) the regexp was quite tricky before. |
I proposed a new PR for SVG/ImageDiff fix: Fix various ImageDiff/SVG bugs #23312 |
Replace #23310, Close #19733 And fix various UI problems, including regressions from #22959 #22950 and more. ## SVG Detection The old regexp may mismatch non-SVG files. This PR adds new tests for those cases. ## UI Changes ### Before ![image](https://user-images.githubusercontent.com/2114189/222967716-f6ad8721-f46a-4a3f-9eb0-a89e488d3436.png) ![image](https://user-images.githubusercontent.com/2114189/222967780-8af8981a-e69d-4304-9dc4-0235582fa4f4.png) ### After ![image](https://user-images.githubusercontent.com/2114189/222967575-c21c23d4-0200-4e09-aac3-57895e853000.png) ![image](https://user-images.githubusercontent.com/2114189/222967585-8b8da262-bc96-441a-9851-8d3845f2659d.png) ![image](https://user-images.githubusercontent.com/2114189/222967595-58d9bea5-6df4-41fa-bf8a-86704117959d.png) ![image](https://user-images.githubusercontent.com/2114189/222967608-38757c1a-b8bd-4ebf-b7a8-3b30edb7f303.png) ![image](https://user-images.githubusercontent.com/2114189/222967623-9849a339-6fae-4484-8fa5-939e2fdacbf5.png) ![image](https://user-images.githubusercontent.com/2114189/222967633-4383d7dd-62ba-47a3-8c10-86f7ca7757ae.png) --------- Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Replace go-gitea#23310, Close go-gitea#19733 And fix various UI problems, including regressions from go-gitea#22959 go-gitea#22950 and more. ## SVG Detection The old regexp may mismatch non-SVG files. This PR adds new tests for those cases. ## UI Changes ### Before ![image](https://user-images.githubusercontent.com/2114189/222967716-f6ad8721-f46a-4a3f-9eb0-a89e488d3436.png) ![image](https://user-images.githubusercontent.com/2114189/222967780-8af8981a-e69d-4304-9dc4-0235582fa4f4.png) ### After ![image](https://user-images.githubusercontent.com/2114189/222967575-c21c23d4-0200-4e09-aac3-57895e853000.png) ![image](https://user-images.githubusercontent.com/2114189/222967585-8b8da262-bc96-441a-9851-8d3845f2659d.png) ![image](https://user-images.githubusercontent.com/2114189/222967595-58d9bea5-6df4-41fa-bf8a-86704117959d.png) ![image](https://user-images.githubusercontent.com/2114189/222967608-38757c1a-b8bd-4ebf-b7a8-3b30edb7f303.png) ![image](https://user-images.githubusercontent.com/2114189/222967623-9849a339-6fae-4484-8fa5-939e2fdacbf5.png) ![image](https://user-images.githubusercontent.com/2114189/222967633-4383d7dd-62ba-47a3-8c10-86f7ca7757ae.png) --------- Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Backport #23312 Replace #23310, Close #19733 And fix various UI problems, including regressions from #22959 #22950 and more. ## SVG Detection The old regexp may mismatch non-SVG files. This PR adds new tests for those cases. ## UI Changes ### Before ![image](https://user-images.githubusercontent.com/2114189/222967716-f6ad8721-f46a-4a3f-9eb0-a89e488d3436.png) ![image](https://user-images.githubusercontent.com/2114189/222967780-8af8981a-e69d-4304-9dc4-0235582fa4f4.png) ### After ![image](https://user-images.githubusercontent.com/2114189/222967575-c21c23d4-0200-4e09-aac3-57895e853000.png) ![image](https://user-images.githubusercontent.com/2114189/222967585-8b8da262-bc96-441a-9851-8d3845f2659d.png) ![image](https://user-images.githubusercontent.com/2114189/222967595-58d9bea5-6df4-41fa-bf8a-86704117959d.png) ![image](https://user-images.githubusercontent.com/2114189/222967608-38757c1a-b8bd-4ebf-b7a8-3b30edb7f303.png) ![image](https://user-images.githubusercontent.com/2114189/222967623-9849a339-6fae-4484-8fa5-939e2fdacbf5.png) ![image](https://user-images.githubusercontent.com/2114189/222967633-4383d7dd-62ba-47a3-8c10-86f7ca7757ae.png) Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
SVG file displaying is broken, meanwhile also breaks other diff files (such as php html templates) from displaying in the diff ui
Every commit here does not display SVGs, and also showcaases broken php html template display
https://try.gitea.io/stuzer05/test-preview/commits/branch/master
So I removed "smart" svg detection at all
Closes #19733