From 4c59c8c7682da31410decba3bd868fde5116e073 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 7 Mar 2023 20:11:24 +0800 Subject: [PATCH] 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 --- modules/typesniffer/typesniffer.go | 21 +++++++++++++++------ modules/typesniffer/typesniffer_test.go | 25 ++++++++++++++++++++++++- web_src/js/features/imagediff.js | 15 +++++++-------- web_src/less/features/imagediff.less | 5 +++-- 4 files changed, 49 insertions(+), 17 deletions(-) diff --git a/modules/typesniffer/typesniffer.go b/modules/typesniffer/typesniffer.go index c9fef953c..5b215496b 100644 --- a/modules/typesniffer/typesniffer.go +++ b/modules/typesniffer/typesniffer.go @@ -4,6 +4,7 @@ package typesniffer import ( + "bytes" "fmt" "io" "net/http" @@ -24,8 +25,9 @@ const ( ) var ( - svgTagRegex = regexp.MustCompile(`(?si)\A\s*(?:(||>))\s*)*\/]`) - svgTagInXMLRegex = regexp.MustCompile(`(?si)\A<\?xml\b.*?\?>\s*(?:(||>))\s*)*\/]`) + svgComment = regexp.MustCompile(`(?s)`) + svgTagRegex = regexp.MustCompile(`(?si)\A\s*(?:(|>))\s*)*\s*(?:(|>))\s*)*")).IsSvgImage()) assert.True(t, DetectContentType([]byte(" ")).IsSvgImage()) assert.True(t, DetectContentType([]byte(``)).IsSvgImage()) - assert.True(t, DetectContentType([]byte("")).IsSvgImage()) assert.True(t, DetectContentType([]byte(``)).IsSvgImage()) assert.True(t, DetectContentType([]byte(` `)).IsSvgImage()) @@ -57,6 +56,10 @@ func TestIsSvgImage(t *testing.T) { `)).IsSvgImage()) + + // the DetectContentType should work for incomplete data, because only beginning bytes are used for detection + assert.True(t, DetectContentType([]byte(`....`)).IsSvgImage()) + assert.False(t, DetectContentType([]byte{}).IsSvgImage()) assert.False(t, DetectContentType([]byte("svg")).IsSvgImage()) assert.False(t, DetectContentType([]byte("")).IsSvgImage()) @@ -68,6 +71,26 @@ func TestIsSvgImage(t *testing.T) { assert.False(t, DetectContentType([]byte(` `)).IsSvgImage()) + + assert.False(t, DetectContentType([]byte(` + +
+ + +
+`)).IsSvgImage()) + + assert.False(t, DetectContentType([]byte(` + +
+ + +
+`)).IsSvgImage()) + assert.False(t, DetectContentType([]byte(``)).IsSvgImage()) + assert.False(t, DetectContentType([]byte(``)).IsSvgImage()) } func TestIsPDF(t *testing.T) { diff --git a/web_src/js/features/imagediff.js b/web_src/js/features/imagediff.js index 7a285f1f8..a26b927c3 100644 --- a/web_src/js/features/imagediff.js +++ b/web_src/js/features/imagediff.js @@ -129,8 +129,8 @@ export function initImageDiff() { initOverlay(createContext($imageAfter[2], $imageBefore[2])); } - hideElem($container.find('> .loader')); $container.find('> .gt-hidden').removeClass('gt-hidden'); + hideElem($container.find('.ui.loader')); } function initSideBySide(sizes) { @@ -155,7 +155,7 @@ export function initImageDiff() { height: sizes.size1.height * factor }); sizes.image1.parent().css({ - margin: `${sizes.ratio[1] * factor + 15}px ${sizes.ratio[0] * factor}px ${sizes.ratio[1] * factor}px`, + margin: `10px auto`, width: sizes.size1.width * factor + 2, height: sizes.size1.height * factor + 2 }); @@ -164,7 +164,7 @@ export function initImageDiff() { height: sizes.size2.height * factor }); sizes.image2.parent().css({ - margin: `${sizes.ratio[3] * factor}px ${sizes.ratio[2] * factor}px`, + margin: `10px auto`, width: sizes.size2.width * factor + 2, height: sizes.size2.height * factor + 2 }); @@ -255,13 +255,12 @@ export function initImageDiff() { width: sizes.size2.width * factor + 2, height: sizes.size2.height * factor + 2 }); + + // some inner elements are `position: absolute`, so the container's height must be large enough + // the "css(width, height)" is somewhat hacky and not easy to understand, it could be improved in the future sizes.image2.parent().parent().css({ width: sizes.max.width * factor + 2, - height: sizes.max.height * factor + 2 - }); - $container.find('.onion-skin').css({ - width: sizes.max.width * factor + 2, - height: sizes.max.height * factor + 4 + height: sizes.max.height * factor + 2 + 20 /* extra height for inner "position: absolute" elements */, }); const $range = $container.find("input[type='range']"); diff --git a/web_src/less/features/imagediff.less b/web_src/less/features/imagediff.less index a9ba7f8c8..07763c15e 100644 --- a/web_src/less/features/imagediff.less +++ b/web_src/less/features/imagediff.less @@ -1,6 +1,6 @@ .image-diff-container { text-align: center; - padding: 30px 0; + padding: 1em 0; img { border: 1px solid var(--color-primary-light-7); @@ -22,6 +22,7 @@ display: inline-block; line-height: 0; vertical-align: top; + margin: 0 1em; .side-header { font-weight: bold; @@ -98,7 +99,7 @@ } input { - width: 300px; + max-width: 300px; } } }