Replace ajax with fetch, improve image diff (#27267) (#27583)

Backport #27267 by @silverwind

1. Dropzone attachment removal, pretty simple replacement
2. Image diff: The previous code fetched every image twice, once via
`img[src]` and once via `$.ajax`. Now it's only fetched once and a
second time only when necessary. The image diff code was partially
rewritten.

Co-authored-by: silverwind <me@silverwind.io>

routers/web/repo/compare.go

@@ -32,6 +32,7 @@ import (
 	"code.gitea.io/gitea/modules/markup"
 	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/typesniffer"
 	"code.gitea.io/gitea/modules/upload"
 	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/services/gitdiff"
@@ -60,6 +61,21 @@ func setCompareContext(ctx *context.Context, before, head *git.Commit, headOwner
 		return blob
 	}
 
+	ctx.Data["GetSniffedTypeForBlob"] = func(blob *git.Blob) typesniffer.SniffedType {
+		st := typesniffer.SniffedType{}
+
+		if blob == nil {
+			return st
+		}
+
+		st, err := blob.GuessContentType()
+		if err != nil {
+			log.Error("GuessContentType failed: %v", err)
+			return st
+		}
+		return st
+	}
+
 	setPathsCompareContext(ctx, before, head, headOwner, headName)
 	setImageCompareContext(ctx)
 	setCsvCompareContext(ctx)
@@ -87,16 +103,7 @@ func setPathsCompareContext(ctx *context.Context, base, head *git.Commit, headOw
 
 // setImageCompareContext sets context data that is required by image compare template
 func setImageCompareContext(ctx *context.Context) {
-	ctx.Data["IsBlobAnImage"] = func(blob *git.Blob) bool {
-		if blob == nil {
-			return false
-		}
-
-		st, err := blob.GuessContentType()
-		if err != nil {
-			log.Error("GuessContentType failed: %v", err)
-			return false
-		}
+	ctx.Data["IsSniffedTypeAnImage"] = func(st typesniffer.SniffedType) bool {
 		return st.IsImage() && (setting.UI.SVG.Enabled || !st.IsSvgImage())
 	}
 }

templates/repo/diff/box.tmpl

@@ -97,7 +97,9 @@
 					{{/*notice: the index of Diff.Files should not be used for element ID, because the index will be restarted from 0 when doing load-more for PRs with a lot of files*/}}
 					{{$blobBase := call $.GetBlobByPathForCommit $.BeforeCommit $file.OldName}}
 					{{$blobHead := call $.GetBlobByPathForCommit $.HeadCommit $file.Name}}
-					{{$isImage := or (call $.IsBlobAnImage $blobBase) (call $.IsBlobAnImage $blobHead)}}
+					{{$sniffedTypeBase := call $.GetSniffedTypeForBlob $blobBase}}
+					{{$sniffedTypeHead := call $.GetSniffedTypeForBlob $blobHead}}
+					{{$isImage:= or (call $.IsSniffedTypeAnImage $sniffedTypeBase) (call $.IsSniffedTypeAnImage $sniffedTypeHead)}}
 					{{$isCsv := (call $.IsCsvFile $file)}}
 					{{$showFileViewToggle := or $isImage (and (not $file.IsIncomplete) $isCsv)}}
 					{{$isExpandable := or (gt $file.Addition 0) (gt $file.Deletion 0) $file.IsBin}}
@@ -198,9 +200,9 @@
 								<div id="diff-rendered-{{$file.NameHash}}" class="file-body file-code {{if $.IsSplitStyle}}code-diff-split{{else}}code-diff-unified{{end}} gt-overflow-x-scroll">
 									<table class="chroma gt-w-100">
 										{{if $isImage}}
-											{{template "repo/diff/image_diff" dict "file" . "root" $ "blobBase" $blobBase "blobHead" $blobHead}}
+											{{template "repo/diff/image_diff" dict "file" . "root" $ "blobBase" $blobBase "blobHead" $blobHead "sniffedTypeBase" $sniffedTypeBase "sniffedTypeHead" $sniffedTypeHead}}
 										{{else}}
-											{{template "repo/diff/csv_diff" dict "file" . "root" $ "blobBase" $blobBase "blobHead" $blobHead}}
+											{{template "repo/diff/csv_diff" dict "file" . "root" $ "blobBase" $blobBase "blobHead" $blobHead "sniffedTypeBase" $sniffedTypeBase "sniffedTypeHead" $sniffedTypeHead}}
 										{{end}}
 									</table>
 								</div>

templates/repo/diff/image_diff.tmpl

@@ -1,7 +1,12 @@
 {{if or .blobBase .blobHead}}
 <tr>
 	<td colspan="2">
-		<div class="image-diff" data-path-before="{{.root.BeforeRawPath}}/{{PathEscapeSegments .file.OldName}}" data-path-after="{{.root.RawPath}}/{{PathEscapeSegments .file.Name}}">
+		<div class="image-diff"
+			data-path-before="{{.root.BeforeRawPath}}/{{PathEscapeSegments .file.OldName}}"
+			data-path-after="{{.root.RawPath}}/{{PathEscapeSegments .file.Name}}"
+			data-mime-before="{{.sniffedTypeBase.GetMimeType}}"
+			data-mime-after="{{.sniffedTypeHead.GetMimeType}}"
+		>
 			<div class="ui secondary pointing tabular top attached borderless menu new-menu">
 				<div class="new-menu-inner">
 					<a class="item active" data-tab="diff-side-by-side-{{.file.Index}}">{{ctx.Locale.Tr "repo.diff.image.side_by_side"}}</a>

web_src/js/features/common-global.js

@@ -11,7 +11,7 @@ import {htmlEscape} from 'escape-goat';
 import {showTemporaryTooltip} from '../modules/tippy.js';
 import {confirmModal} from './comp/ConfirmModal.js';
 import {showErrorToast} from '../modules/toast.js';
-import {request} from '../modules/fetch.js';
+import {request, POST} from '../modules/fetch.js';
 
 const {appUrl, appSubUrl, csrfToken, i18n} = window.config;
 
@@ -243,9 +243,8 @@ export function initGlobalDropzone() {
         this.on('removedfile', (file) => {
           $(`#${file.uuid}`).remove();
           if ($dropzone.data('remove-url')) {
-            $.post($dropzone.data('remove-url'), {
-              file: file.uuid,
-              _csrf: csrfToken,
+            POST($dropzone.data('remove-url'), {
+              data: new URLSearchParams({file: file.uuid}),
             });
           }
         });

web_src/js/features/imagediff.js

@@ -1,11 +1,14 @@
 import $ from 'jquery';
-import {hideElem} from '../utils/dom.js';
+import {GET} from '../modules/fetch.js';
+import {hideElem, loadElem} from '../utils/dom.js';
+import {parseDom} from '../utils.js';
 
-function getDefaultSvgBoundsIfUndefined(svgXml, src) {
+function getDefaultSvgBoundsIfUndefined(text, src) {
   const DefaultSize = 300;
   const MaxSize = 99999;
 
-  const svg = svgXml.documentElement;
+  const svgDoc = parseDom(text, 'image/svg+xml');
+  const svg = svgDoc.documentElement;
   const width = svg?.width?.baseVal;
   const height = svg?.height?.baseVal;
   if (width === undefined || height === undefined) {
@@ -65,73 +68,53 @@ export function initImageDiff() {
     };
   }
 
-  $('.image-diff:not([data-image-diff-loaded])').each(function() {
+  $('.image-diff:not([data-image-diff-loaded])').each(async function() {
     const $container = $(this);
     $container.attr('data-image-diff-loaded', 'true');
 
     // the container may be hidden by "viewed" checkbox, so use the parent's width for reference
     const diffContainerWidth = Math.max($container.closest('.diff-file-box').width() - 300, 100);
-    const pathAfter = $container.data('path-after');
-    const pathBefore = $container.data('path-before');
 
     const imageInfos = [{
-      loaded: false,
-      path: pathAfter,
-      $image: $container.find('img.image-after'),
+      path: this.getAttribute('data-path-after'),
+      mime: this.getAttribute('data-mime-after'),
+      $images: $container.find('img.image-after'), // matches 3 <img>
       $boundsInfo: $container.find('.bounds-info-after')
     }, {
-      loaded: false,
-      path: pathBefore,
-      $image: $container.find('img.image-before'),
+      path: this.getAttribute('data-path-before'),
+      mime: this.getAttribute('data-mime-before'),
+      $images: $container.find('img.image-before'), // matches 3 <img>
       $boundsInfo: $container.find('.bounds-info-before')
     }];
 
-    for (const info of imageInfos) {
-      if (info.$image.length > 0) {
-        $.ajax({
-          url: info.path,
-          success: (data, _, jqXHR) => {
-            info.$image.on('load', () => {
-              info.loaded = true;
-              setReadyIfLoaded();
-            }).on('error', () => {
-              info.loaded = true;
-              setReadyIfLoaded();
-              info.$boundsInfo.text('(image error)');
-            });
-            info.$image.attr('src', info.path);
-
-            if (jqXHR.getResponseHeader('Content-Type') === 'image/svg+xml') {
-              const bounds = getDefaultSvgBoundsIfUndefined(data, info.path);
-              if (bounds) {
-                info.$image.attr('width', bounds.width);
-                info.$image.attr('height', bounds.height);
-                hideElem(info.$boundsInfo);
-              }
-            }
-          }
-        });
-      } else {
-        info.loaded = true;
-        setReadyIfLoaded();
+    await Promise.all(imageInfos.map(async (info) => {
+      const [success] = await Promise.all(Array.from(info.$images, (img) => {
+        return loadElem(img, info.path);
+      }));
+      // only the first images is associated with $boundsInfo
+      if (!success) info.$boundsInfo.text('(image error)');
+      if (info.mime === 'image/svg+xml') {
+        const resp = await GET(info.path);
+        const text = await resp.text();
+        const bounds = getDefaultSvgBoundsIfUndefined(text, info.path);
+        if (bounds) {
+          info.$images.attr('width', bounds.width);
+          info.$images.attr('height', bounds.height);
+          hideElem(info.$boundsInfo);
+        }
       }
+    }));
+
+    const $imagesAfter = imageInfos[0].$images;
+    const $imagesBefore = imageInfos[1].$images;
+
+    initSideBySide(createContext($imagesAfter[0], $imagesBefore[0]));
+    if ($imagesAfter.length > 0 && $imagesBefore.length > 0) {
+      initSwipe(createContext($imagesAfter[1], $imagesBefore[1]));
+      initOverlay(createContext($imagesAfter[2], $imagesBefore[2]));
     }
 
-    function setReadyIfLoaded() {
-      if (imageInfos[0].loaded && imageInfos[1].loaded) {
-        initViews(imageInfos[0].$image, imageInfos[1].$image);
-      }
-    }
-
-    function initViews($imageAfter, $imageBefore) {
-      initSideBySide(createContext($imageAfter[0], $imageBefore[0]));
-      if ($imageAfter.length > 0 && $imageBefore.length > 0) {
-        initSwipe(createContext($imageAfter[1], $imageBefore[1]));
-        initOverlay(createContext($imageAfter[2], $imageBefore[2]));
-      }
-
-      $container.find('> .image-diff-tabs').removeClass('is-loading');
-    }
+    $container.find('> .image-diff-tabs').removeClass('is-loading');
 
     function initSideBySide(sizes) {
       let factor = 1;

web_src/js/modules/fetch.js

@@ -11,9 +11,7 @@ const safeMethods = new Set(['GET', 'HEAD', 'OPTIONS', 'TRACE']);
 export function request(url, {method = 'GET', headers = {}, data, body, ...other} = {}) {
   let contentType;
   if (!body) {
-    if (data instanceof FormData) {
-      body = data;
-    } else if (data instanceof URLSearchParams) {
+    if (data instanceof FormData || data instanceof URLSearchParams) {
       body = data;
     } else if (isObject(data) || Array.isArray(data)) {
       contentType = 'application/json';

web_src/js/svg.js

@@ -1,4 +1,5 @@
 import {h} from 'vue';
+import {parseDom, serializeXml} from './utils.js';
 import giteaDoubleChevronLeft from '../../public/assets/img/svg/gitea-double-chevron-left.svg';
 import giteaDoubleChevronRight from '../../public/assets/img/svg/gitea-double-chevron-right.svg';
 import giteaEmptyCheckbox from '../../public/assets/img/svg/gitea-empty-checkbox.svg';
@@ -145,22 +146,19 @@ const svgs = {
 //  At the moment, developers must check, pick and fill the names manually,
 //  most of the SVG icons in assets couldn't be used directly.
 
-const parser = new DOMParser();
-const serializer = new XMLSerializer();
-
 // retrieve an HTML string for given SVG icon name, size and additional classes
 export function svg(name, size = 16, className = '') {
   if (!(name in svgs)) throw new Error(`Unknown SVG icon: ${name}`);
   if (size === 16 && !className) return svgs[name];
 
-  const document = parser.parseFromString(svgs[name], 'image/svg+xml');
+  const document = parseDom(svgs[name], 'image/svg+xml');
   const svgNode = document.firstChild;
   if (size !== 16) {
     svgNode.setAttribute('width', String(size));
     svgNode.setAttribute('height', String(size));
   }
   if (className) svgNode.classList.add(...className.split(/\s+/).filter(Boolean));
-  return serializer.serializeToString(svgNode);
+  return serializeXml(svgNode);
 }
 
 export function svgParseOuterInner(name) {
@@ -176,7 +174,7 @@ export function svgParseOuterInner(name) {
   if (p1 === -1 || p2 === -1) throw new Error(`Invalid SVG icon: ${name}`);
   const svgInnerHtml = svgStr.slice(p1 + 1, p2);
   const svgOuterHtml = svgStr.slice(0, p1 + 1) + svgStr.slice(p2);
-  const svgDoc = parser.parseFromString(svgOuterHtml, 'image/svg+xml');
+  const svgDoc = parseDom(svgOuterHtml, 'image/svg+xml');
   const svgOuter = svgDoc.firstChild;
   return {svgOuter, svgInnerHtml};
 }

web_src/js/utils.js

@@ -128,3 +128,14 @@ export function decodeURLEncodedBase64(base64url) {
     .replace(/_/g, '/')
     .replace(/-/g, '+'));
 }
+
+const domParser = new DOMParser();
+const xmlSerializer = new XMLSerializer();
+
+export function parseDom(text, contentType) {
+  return domParser.parseFromString(text, contentType);
+}
+
+export function serializeXml(node) {
+  return xmlSerializer.serializeToString(node);
+}

web_src/js/utils/dom.js

@@ -183,3 +183,14 @@ export function autosize(textarea, {viewportMarginBottom = 0} = {}) {
 export function onInputDebounce(fn) {
   return debounce(300, fn);
 }
+
+// Set the `src` attribute on an element and returns a promise that resolves once the element
+// has loaded or errored. Suitable for all elements mention in:
+// https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/load_event
+export function loadElem(el, src) {
+  return new Promise((resolve) => {
+    el.addEventListener('load', () => resolve(true), {once: true});
+    el.addEventListener('error', () => resolve(false), {once: true});
+    el.src = src;
+  });
+}