Skip to content

Commit 703a2aa

Browse files
silverwindGiteaBot
silverwind
and
GiteaBot
committed
Replace ajax with fetch, improve image diff (go-gitea#27267)
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: Giteabot <[email protected]>
1 parent 4986dc8 commit 703a2aa

File tree

9 files changed

+95
-81
lines changed

9 files changed

+95
-81
lines changed

‎routers/web/repo/compare.go

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
"code.gitea.io/gitea/modules/markup"
3333
"code.gitea.io/gitea/modules/setting"
3434
api "code.gitea.io/gitea/modules/structs"
35+
"code.gitea.io/gitea/modules/typesniffer"
3536
"code.gitea.io/gitea/modules/upload"
3637
"code.gitea.io/gitea/modules/util"
3738
"code.gitea.io/gitea/services/gitdiff"
@@ -60,6 +61,21 @@ func setCompareContext(ctx *context.Context, before, head *git.Commit, headOwner
6061
return blob
6162
}
6263

64+
ctx.Data["GetSniffedTypeForBlob"] = func(blob *git.Blob) typesniffer.SniffedType {
65+
st := typesniffer.SniffedType{}
66+
67+
if blob == nil {
68+
return st
69+
}
70+
71+
st, err := blob.GuessContentType()
72+
if err != nil {
73+
log.Error("GuessContentType failed: %v", err)
74+
return st
75+
}
76+
return st
77+
}
78+
6379
setPathsCompareContext(ctx, before, head, headOwner, headName)
6480
setImageCompareContext(ctx)
6581
setCsvCompareContext(ctx)
@@ -87,16 +103,7 @@ func setPathsCompareContext(ctx *context.Context, base, head *git.Commit, headOw
87103

88104
// setImageCompareContext sets context data that is required by image compare template
89105
func setImageCompareContext(ctx *context.Context) {
90-
ctx.Data["IsBlobAnImage"] = func(blob *git.Blob) bool {
91-
if blob == nil {
92-
return false
93-
}
94-
95-
st, err := blob.GuessContentType()
96-
if err != nil {
97-
log.Error("GuessContentType failed: %v", err)
98-
return false
99-
}
106+
ctx.Data["IsSniffedTypeAnImage"] = func(st typesniffer.SniffedType) bool {
100107
return st.IsImage() && (setting.UI.SVG.Enabled || !st.IsSvgImage())
101108
}
102109
}

‎templates/repo/diff/box.tmpl

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,9 @@
9797
{{/*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*/}}
9898
{{$blobBase := call $.GetBlobByPathForCommit $.BeforeCommit $file.OldName}}
9999
{{$blobHead := call $.GetBlobByPathForCommit $.HeadCommit $file.Name}}
100-
{{$isImage := or (call $.IsBlobAnImage $blobBase) (call $.IsBlobAnImage $blobHead)}}
100+
{{$sniffedTypeBase := call $.GetSniffedTypeForBlob $blobBase}}
101+
{{$sniffedTypeHead := call $.GetSniffedTypeForBlob $blobHead}}
102+
{{$isImage:= or (call $.IsSniffedTypeAnImage $sniffedTypeBase) (call $.IsSniffedTypeAnImage $sniffedTypeHead)}}
101103
{{$isCsv := (call $.IsCsvFile $file)}}
102104
{{$showFileViewToggle := or $isImage (and (not $file.IsIncomplete) $isCsv)}}
103105
{{$isExpandable := or (gt $file.Addition 0) (gt $file.Deletion 0) $file.IsBin}}
@@ -198,9 +200,9 @@
198200
<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">
199201
<table class="chroma gt-w-100">
200202
{{if $isImage}}
201-
{{template "repo/diff/image_diff" dict "file" . "root" $ "blobBase" $blobBase "blobHead" $blobHead}}
203+
{{template "repo/diff/image_diff" dict "file" . "root" $ "blobBase" $blobBase "blobHead" $blobHead "sniffedTypeBase" $sniffedTypeBase "sniffedTypeHead" $sniffedTypeHead}}
202204
{{else}}
203-
{{template "repo/diff/csv_diff" dict "file" . "root" $ "blobBase" $blobBase "blobHead" $blobHead}}
205+
{{template "repo/diff/csv_diff" dict "file" . "root" $ "blobBase" $blobBase "blobHead" $blobHead "sniffedTypeBase" $sniffedTypeBase "sniffedTypeHead" $sniffedTypeHead}}
204206
{{end}}
205207
</table>
206208
</div>

‎templates/repo/diff/image_diff.tmpl

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
{{if or .blobBase .blobHead}}
22
<tr>
33
<td colspan="2">
4-
<div class="image-diff" data-path-before="{{.root.BeforeRawPath}}/{{PathEscapeSegments .file.OldName}}" data-path-after="{{.root.RawPath}}/{{PathEscapeSegments .file.Name}}">
4+
<div class="image-diff"
5+
data-path-before="{{.root.BeforeRawPath}}/{{PathEscapeSegments .file.OldName}}"
6+
data-path-after="{{.root.RawPath}}/{{PathEscapeSegments .file.Name}}"
7+
data-mime-before="{{.sniffedTypeBase.GetMimeType}}"
8+
data-mime-after="{{.sniffedTypeHead.GetMimeType}}"
9+
>
510
<div class="ui secondary pointing tabular top attached borderless menu new-menu">
611
<div class="new-menu-inner">
712
<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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {htmlEscape} from 'escape-goat';
1111
import {showTemporaryTooltip} from '../modules/tippy.js';
1212
import {confirmModal} from './comp/ConfirmModal.js';
1313
import {showErrorToast} from '../modules/toast.js';
14-
import {request} from '../modules/fetch.js';
14+
import {request, POST} from '../modules/fetch.js';
1515

1616
const {appUrl, appSubUrl, csrfToken, i18n} = window.config;
1717

@@ -243,9 +243,8 @@ export function initGlobalDropzone() {
243243
this.on('removedfile', (file) => {
244244
$(`#${file.uuid}`).remove();
245245
if ($dropzone.data('remove-url')) {
246-
$.post($dropzone.data('remove-url'), {
247-
file: file.uuid,
248-
_csrf: csrfToken,
246+
POST($dropzone.data('remove-url'), {
247+
data: new URLSearchParams({file: file.uuid}),
249248
});
250249
}
251250
});

‎web_src/js/features/imagediff.js

Lines changed: 37 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
import $ from 'jquery';
2-
import {hideElem} from '../utils/dom.js';
2+
import {GET} from '../modules/fetch.js';
3+
import {hideElem, loadElem} from '../utils/dom.js';
4+
import {parseDom} from '../utils.js';
35

4-
function getDefaultSvgBoundsIfUndefined(svgXml, src) {
6+
function getDefaultSvgBoundsIfUndefined(text, src) {
57
const DefaultSize = 300;
68
const MaxSize = 99999;
79

8-
const svg = svgXml.documentElement;
10+
const svgDoc = parseDom(text, 'image/svg+xml');
11+
const svg = svgDoc.documentElement;
912
const width = svg?.width?.baseVal;
1013
const height = svg?.height?.baseVal;
1114
if (width === undefined || height === undefined) {
@@ -65,74 +68,54 @@ export function initImageDiff() {
6568
};
6669
}
6770

68-
$('.image-diff:not([data-image-diff-loaded])').each(function() {
71+
$('.image-diff:not([data-image-diff-loaded])').each(async function() {
6972
const $container = $(this);
7073
$container.attr('data-image-diff-loaded', 'true');
7174

7275
// the container may be hidden by "viewed" checkbox, so use the parent's width for reference
7376
const diffContainerWidth = Math.max($container.closest('.diff-file-box').width() - 300, 100);
74-
const pathAfter = $container.data('path-after');
75-
const pathBefore = $container.data('path-before');
7677

7778
const imageInfos = [{
78-
loaded: false,
79-
path: pathAfter,
80-
$image: $container.find('img.image-after'),
79+
path: this.getAttribute('data-path-after'),
80+
mime: this.getAttribute('data-mime-after'),
81+
$images: $container.find('img.image-after'), // matches 3 <img>
8182
$boundsInfo: $container.find('.bounds-info-after')
8283
}, {
83-
loaded: false,
84-
path: pathBefore,
85-
$image: $container.find('img.image-before'),
84+
path: this.getAttribute('data-path-before'),
85+
mime: this.getAttribute('data-mime-before'),
86+
$images: $container.find('img.image-before'), // matches 3 <img>
8687
$boundsInfo: $container.find('.bounds-info-before')
8788
}];
8889

89-
for (const info of imageInfos) {
90-
if (info.$image.length > 0) {
91-
$.ajax({
92-
url: info.path,
93-
success: (data, _, jqXHR) => {
94-
info.$image.on('load', () => {
95-
info.loaded = true;
96-
setReadyIfLoaded();
97-
}).on('error', () => {
98-
info.loaded = true;
99-
setReadyIfLoaded();
100-
info.$boundsInfo.text('(image error)');
101-
});
102-
info.$image.attr('src', info.path);
103-
104-
if (jqXHR.getResponseHeader('Content-Type') === 'image/svg+xml') {
105-
const bounds = getDefaultSvgBoundsIfUndefined(data, info.path);
106-
if (bounds) {
107-
info.$image.attr('width', bounds.width);
108-
info.$image.attr('height', bounds.height);
109-
hideElem(info.$boundsInfo);
110-
}
111-
}
112-
}
113-
});
114-
} else {
115-
info.loaded = true;
116-
setReadyIfLoaded();
117-
}
118-
}
119-
120-
function setReadyIfLoaded() {
121-
if (imageInfos[0].loaded && imageInfos[1].loaded) {
122-
initViews(imageInfos[0].$image, imageInfos[1].$image);
90+
await Promise.all(imageInfos.map(async (info) => {
91+
const [success] = await Promise.all(Array.from(info.$images, (img) => {
92+
return loadElem(img, info.path);
93+
}));
94+
// only the first images is associated with $boundsInfo
95+
if (!success) info.$boundsInfo.text('(image error)');
96+
if (info.mime === 'image/svg+xml') {
97+
const resp = await GET(info.path);
98+
const text = await resp.text();
99+
const bounds = getDefaultSvgBoundsIfUndefined(text, info.path);
100+
if (bounds) {
101+
info.$images.attr('width', bounds.width);
102+
info.$images.attr('height', bounds.height);
103+
hideElem(info.$boundsInfo);
104+
}
123105
}
124-
}
106+
}));
125107

126-
function initViews($imageAfter, $imageBefore) {
127-
initSideBySide(createContext($imageAfter[0], $imageBefore[0]));
128-
if ($imageAfter.length > 0 && $imageBefore.length > 0) {
129-
initSwipe(createContext($imageAfter[1], $imageBefore[1]));
130-
initOverlay(createContext($imageAfter[2], $imageBefore[2]));
131-
}
108+
const $imagesAfter = imageInfos[0].$images;
109+
const $imagesBefore = imageInfos[1].$images;
132110

133-
$container.find('> .image-diff-tabs').removeClass('is-loading');
111+
initSideBySide(createContext($imagesAfter[0], $imagesBefore[0]));
112+
if ($imagesAfter.length > 0 && $imagesBefore.length > 0) {
113+
initSwipe(createContext($imagesAfter[1], $imagesBefore[1]));
114+
initOverlay(createContext($imagesAfter[2], $imagesBefore[2]));
134115
}
135116

117+
$container.find('> .image-diff-tabs').removeClass('is-loading');
118+
136119
function initSideBySide(sizes) {
137120
let factor = 1;
138121
if (sizes.max.width > (diffContainerWidth - 24) / 2) {

‎web_src/js/modules/fetch.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,7 @@ const safeMethods = new Set(['GET', 'HEAD', 'OPTIONS', 'TRACE']);
1111
export function request(url, {method = 'GET', headers = {}, data, body, ...other} = {}) {
1212
let contentType;
1313
if (!body) {
14-
if (data instanceof FormData) {
15-
body = data;
16-
} else if (data instanceof URLSearchParams) {
14+
if (data instanceof FormData || data instanceof URLSearchParams) {
1715
body = data;
1816
} else if (isObject(data) || Array.isArray(data)) {
1917
contentType = 'application/json';

‎web_src/js/svg.js

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {h} from 'vue';
2+
import {parseDom, serializeXml} from './utils.js';
23
import giteaDoubleChevronLeft from '../../public/assets/img/svg/gitea-double-chevron-left.svg';
34
import giteaDoubleChevronRight from '../../public/assets/img/svg/gitea-double-chevron-right.svg';
45
import giteaEmptyCheckbox from '../../public/assets/img/svg/gitea-empty-checkbox.svg';
@@ -145,22 +146,19 @@ const svgs = {
145146
// At the moment, developers must check, pick and fill the names manually,
146147
// most of the SVG icons in assets couldn't be used directly.
147148

148-
const parser = new DOMParser();
149-
const serializer = new XMLSerializer();
150-
151149
// retrieve an HTML string for given SVG icon name, size and additional classes
152150
export function svg(name, size = 16, className = '') {
153151
if (!(name in svgs)) throw new Error(`Unknown SVG icon: ${name}`);
154152
if (size === 16 && !className) return svgs[name];
155153

156-
const document = parser.parseFromString(svgs[name], 'image/svg+xml');
154+
const document = parseDom(svgs[name], 'image/svg+xml');
157155
const svgNode = document.firstChild;
158156
if (size !== 16) {
159157
svgNode.setAttribute('width', String(size));
160158
svgNode.setAttribute('height', String(size));
161159
}
162160
if (className) svgNode.classList.add(...className.split(/\s+/).filter(Boolean));
163-
return serializer.serializeToString(svgNode);
161+
return serializeXml(svgNode);
164162
}
165163

166164
export function svgParseOuterInner(name) {
@@ -176,7 +174,7 @@ export function svgParseOuterInner(name) {
176174
if (p1 === -1 || p2 === -1) throw new Error(`Invalid SVG icon: ${name}`);
177175
const svgInnerHtml = svgStr.slice(p1 + 1, p2);
178176
const svgOuterHtml = svgStr.slice(0, p1 + 1) + svgStr.slice(p2);
179-
const svgDoc = parser.parseFromString(svgOuterHtml, 'image/svg+xml');
177+
const svgDoc = parseDom(svgOuterHtml, 'image/svg+xml');
180178
const svgOuter = svgDoc.firstChild;
181179
return {svgOuter, svgInnerHtml};
182180
}

‎web_src/js/utils.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,3 +128,14 @@ export function decodeURLEncodedBase64(base64url) {
128128
.replace(/_/g, '/')
129129
.replace(/-/g, '+'));
130130
}
131+
132+
const domParser = new DOMParser();
133+
const xmlSerializer = new XMLSerializer();
134+
135+
export function parseDom(text, contentType) {
136+
return domParser.parseFromString(text, contentType);
137+
}
138+
139+
export function serializeXml(node) {
140+
return xmlSerializer.serializeToString(node);
141+
}

‎web_src/js/utils/dom.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,3 +183,14 @@ export function autosize(textarea, {viewportMarginBottom = 0} = {}) {
183183
export function onInputDebounce(fn) {
184184
return debounce(300, fn);
185185
}
186+
187+
// Set the `src` attribute on an element and returns a promise that resolves once the element
188+
// has loaded or errored. Suitable for all elements mention in:
189+
// https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/load_event
190+
export function loadElem(el, src) {
191+
return new Promise((resolve) => {
192+
el.addEventListener('load', () => resolve(true), {once: true});
193+
el.addEventListener('error', () => resolve(false), {once: true});
194+
el.src = src;
195+
});
196+
}

0 commit comments

Comments
 (0)