Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions modules/markup/sanitizer_default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ func TestSanitizer(t *testing.T) {
`<a href="javascript:alert('xss')">bad</a>`, `bad`,
`<a href="vbscript:no">bad</a>`, `bad`,
`<a href="data:1234">bad</a>`, `bad`,

// Some classes and attributes are used by the frontend framework and will execute JS code, so make sure they are removed
`<div class="link-action" data-attr-class="foo" data-url="xxx">txt</div>`, `<div data-attr-class="foo">txt</div>`,
`<div class="form-fetch-action" data-markdown-generated-content="bar" data-global-init="a" data-global-click="b">txt</div>`, `<div data-markdown-generated-content="bar">txt</div>`,
}

for i := 0; i < len(testCases); i += 2 {
Expand Down
16 changes: 8 additions & 8 deletions templates/base/alert.tmpl
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
{{if .Flash.ErrorMsg}}
{{- if .Flash.ErrorMsg -}}
<div class="ui negative message flash-message flash-error">
<p>{{.Flash.ErrorMsg | SanitizeHTML}}</p>
</div>
{{end}}
{{if .Flash.SuccessMsg}}
{{- end -}}
{{- if .Flash.SuccessMsg -}}
<div class="ui positive message flash-message flash-success">
<p>{{.Flash.SuccessMsg | SanitizeHTML}}</p>
</div>
{{end}}
{{if .Flash.InfoMsg}}
{{- end -}}
{{- if .Flash.InfoMsg -}}
<div class="ui info message flash-message flash-info">
<p>{{.Flash.InfoMsg | SanitizeHTML}}</p>
</div>
{{end}}
{{if .Flash.WarningMsg}}
{{- end -}}
{{- if .Flash.WarningMsg -}}
<div class="ui warning message flash-message flash-warning">
<p>{{.Flash.WarningMsg | SanitizeHTML}}</p>
</div>
{{end}}
{{- end -}}
7 changes: 4 additions & 3 deletions templates/repo/issue/new_form.tmpl
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
{{if .Flash}}
{{template "base/alert" .}}
{{end}}
<form class="issue-content ui comment form form-fetch-action" id="new-issue" action="{{.Link}}" method="post">
{{.CsrfTokenHtml}}
<div class="issue-content-left">
Expand All @@ -9,7 +7,10 @@
{{ctx.AvatarUtils.Avatar .SignedUser 40}}
<div class="ui segment content tw-my-0">
<div class="field">
<input name="title" class="js-autofocus-end" id="issue_title" placeholder="{{ctx.Locale.Tr "repo.milestones.title"}}" value="{{if .TitleQuery}}{{.TitleQuery}}{{else if .IssueTemplateTitle}}{{.IssueTemplateTitle}}{{else}}{{.title}}{{end}}" required maxlength="255" autocomplete="off">
<input name="title" data-global-init="initInputAutoFocusEnd" id="issue_title" required maxlength="255" autocomplete="off"
placeholder="{{ctx.Locale.Tr "repo.milestones.title"}}"
value="{{if .TitleQuery}}{{.TitleQuery}}{{else if .IssueTemplateTitle}}{{.IssueTemplateTitle}}{{else}}{{.title}}{{end}}"
>
{{if .PageIsComparePull}}
<div class="title_wip_desc" data-wip-prefixes="{{JsonUtils.EncodeToString .PullRequestWorkInProgressPrefixes}}">{{ctx.Locale.Tr "repo.pulls.title_wip_desc" (index .PullRequestWorkInProgressPrefixes 0)}}</div>
{{end}}
Expand Down
6 changes: 0 additions & 6 deletions web_src/js/features/autofocus-end.ts

This file was deleted.

23 changes: 21 additions & 2 deletions web_src/js/features/common-page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {GET} from '../modules/fetch.ts';
import {showGlobalErrorMessage} from '../bootstrap.ts';
import {fomanticQuery} from '../modules/fomantic/base.ts';
import {queryElems} from '../utils/dom.ts';
import {observeAddedElement} from '../modules/observer.ts';
import {registerGlobalInitFunc, registerGlobalSelectorFunc} from '../modules/observer.ts';

const {appUrl} = window.config;

Expand Down Expand Up @@ -30,7 +30,7 @@ export function initFootLanguageMenu() {

export function initGlobalDropdown() {
// do not init "custom" dropdowns, "custom" dropdowns are managed by their own code.
observeAddedElement('.ui.dropdown:not(.custom)', (el) => {
registerGlobalSelectorFunc('.ui.dropdown:not(.custom)', (el) => {
const $dropdown = fomanticQuery(el);
if ($dropdown.data('module-dropdown')) return; // do not re-init if other code has already initialized it.

Expand Down Expand Up @@ -80,6 +80,25 @@ export function initGlobalTabularMenu() {
fomanticQuery('.ui.menu.tabular:not(.custom) .item').tab({autoTabActivation: false});
}

// for performance considerations, it only uses performant syntax
function attachInputDirAuto(el: Partial<HTMLInputElement | HTMLTextAreaElement>) {
if (el.type !== 'hidden' &&
el.type !== 'checkbox' &&
el.type !== 'radio' &&
el.type !== 'range' &&
el.type !== 'color') {
el.dir = 'auto';
}
}

export function initGlobalInput() {
registerGlobalSelectorFunc('input, textarea', attachInputDirAuto);
registerGlobalInitFunc('initInputAutoFocusEnd', (el: HTMLInputElement) => {
el.focus(); // expects only one such element on one page. If there are many, then the last one gets the focus.
el.setSelectionRange(el.value.length, el.value.length);
});
}

/**
* Too many users set their ROOT_URL to wrong value, and it causes a lot of problems:
* * Cross-origin API request without correct cookie
Expand Down
4 changes: 2 additions & 2 deletions web_src/js/features/repo-diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {POST, GET} from '../modules/fetch.ts';
import {createTippy} from '../modules/tippy.ts';
import {invertFileFolding} from './file-fold.ts';
import {parseDom} from '../utils.ts';
import {observeAddedElement} from '../modules/observer.ts';
import {registerGlobalSelectorFunc} from '../modules/observer.ts';

const {i18n} = window.config;

Expand Down Expand Up @@ -254,7 +254,7 @@ export function initRepoDiffView() {
initExpandAndCollapseFilesButton();
initRepoDiffHashChangeListener();

observeAddedElement('#diff-file-boxes .diff-file-box', initRepoDiffFileBox);
registerGlobalSelectorFunc('#diff-file-boxes .diff-file-box', initRepoDiffFileBox);
addDelegatedEventListener(document, 'click', '.fold-file', (el) => {
invertFileFolding(el.closest('.file-content'), el);
});
Expand Down
65 changes: 17 additions & 48 deletions web_src/js/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {initImageDiff} from './features/imagediff.ts';
import {initRepoMigration} from './features/repo-migration.ts';
import {initRepoProject} from './features/repo-projects.ts';
import {initTableSort} from './features/tablesort.ts';
import {initAutoFocusEnd} from './features/autofocus-end.ts';
import {initAdminUserListSearchForm} from './features/admin/users.ts';
import {initAdminConfigs} from './features/admin/config.ts';
import {initMarkupAnchors} from './markup/anchors.ts';
Expand Down Expand Up @@ -62,62 +61,23 @@ import {initRepoContributors} from './features/contributors.ts';
import {initRepoCodeFrequency} from './features/code-frequency.ts';
import {initRepoRecentCommits} from './features/recent-commits.ts';
import {initRepoDiffCommitBranchesAndTags} from './features/repo-diff-commit.ts';
import {initAddedElementObserver} from './modules/observer.ts';
import {initGlobalSelectorObserver} from './modules/observer.ts';
import {initRepositorySearch} from './features/repo-search.ts';
import {initColorPickers} from './features/colorpicker.ts';
import {initAdminSelfCheck} from './features/admin/selfcheck.ts';
import {initOAuth2SettingsDisableCheckbox} from './features/oauth2-settings.ts';
import {initGlobalFetchAction} from './features/common-fetch-action.ts';
import {
initFootLanguageMenu,
initGlobalDropdown,
initGlobalTabularMenu,
initHeadNavbarContentToggle,
} from './features/common-page.ts';
import {
initGlobalButtonClickOnEnter,
initGlobalButtons,
initGlobalDeleteButton,
} from './features/common-button.ts';
import {
initGlobalComboMarkdownEditor,
initGlobalEnterQuickSubmit,
initGlobalFormDirtyLeaveConfirm,
} from './features/common-form.ts';
import {initFootLanguageMenu, initGlobalDropdown, initGlobalInput, initGlobalTabularMenu, initHeadNavbarContentToggle} from './features/common-page.ts';
import {initGlobalButtonClickOnEnter, initGlobalButtons, initGlobalDeleteButton} from './features/common-button.ts';
import {initGlobalComboMarkdownEditor, initGlobalEnterQuickSubmit, initGlobalFormDirtyLeaveConfirm} from './features/common-form.ts';
import {callInitFunctions} from './modules/init.ts';

initGiteaFomantic();
initAddedElementObserver();
initSubmitEventPolyfill();

function callInitFunctions(functions: (() => any)[]) {
// Start performance trace by accessing a URL by "https://localhost/?_ui_performance_trace=1" or "https://localhost/?key=value&_ui_performance_trace=1"
// It is a quick check, no side effect so no need to do slow URL parsing.
const initStart = performance.now();
if (window.location.search.includes('_ui_performance_trace=1')) {
let results: {name: string, dur: number}[] = [];
for (const func of functions) {
const start = performance.now();
func();
results.push({name: func.name, dur: performance.now() - start});
}
results = results.sort((a, b) => b.dur - a.dur);
for (let i = 0; i < 20 && i < results.length; i++) {
// eslint-disable-next-line no-console
console.log(`performance trace: ${results[i].name} ${results[i].dur.toFixed(3)}`);
}
} else {
for (const func of functions) {
func();
}
}
const initDur = performance.now() - initStart;
if (initDur > 500) {
console.error(`slow init functions took ${initDur.toFixed(3)}ms`);
}
}

onDomReady(() => {
callInitFunctions([
const initStartTime = performance.now();
const initPerformanceTracer = callInitFunctions([
initGlobalDropdown,
initGlobalTabularMenu,
initGlobalFetchAction,
Expand All @@ -129,6 +89,7 @@ onDomReady(() => {
initGlobalFormDirtyLeaveConfirm,
initGlobalComboMarkdownEditor,
initGlobalDeleteButton,
initGlobalInput,

initCommonOrganization,
initCommonIssueListQuickGoto,
Expand All @@ -150,7 +111,6 @@ onDomReady(() => {
initSshKeyFormParser,
initStopwatch,
initTableSort,
initAutoFocusEnd,
initFindFileInRepo,
initCopyContent,

Expand Down Expand Up @@ -212,4 +172,13 @@ onDomReady(() => {

initOAuth2SettingsDisableCheckbox,
]);

// it must be the last one, then the "querySelectorAll" only needs to be executed once for global init functions.
initGlobalSelectorObserver(initPerformanceTracer);
if (initPerformanceTracer) initPerformanceTracer.printResults();

const initDur = performance.now() - initStartTime;
if (initDur > 500) {
console.error(`slow init functions took ${initDur.toFixed(3)}ms`);
}
});
27 changes: 27 additions & 0 deletions web_src/js/modules/init.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
export class InitPerformanceTracer {
results: {name: string, dur: number}[] = [];
recordCall(name: string, func: ()=>void) {
const start = performance.now();
func();
this.results.push({name, dur: performance.now() - start});
}
printResults() {
this.results = this.results.sort((a, b) => b.dur - a.dur);
for (let i = 0; i < 20 && i < this.results.length; i++) {
// eslint-disable-next-line no-console
console.log(`performance trace: ${this.results[i].name} ${this.results[i].dur.toFixed(3)}`);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (let i = 0; i < 20 && i < this.results.length; i++) {
// eslint-disable-next-line no-console
console.log(`performance trace: ${this.results[i].name} ${this.results[i].dur.toFixed(3)}`);
}
for (const [index, {name, dur}] of this.results.entries()) {
if (index >= 20) break;
console.info(`performance trace: ${name} ${dur.toFixed(3)}`);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think it is performant, by the for ( let ...; i < len; ...) loop, we do not need to create new "entries" object or extra if check

}
}

export function callInitFunctions(functions: (() => any)[]): InitPerformanceTracer | null {
// Start performance trace by accessing a URL by "https://localhost/?_ui_performance_trace=1" or "https://localhost/?key=value&_ui_performance_trace=1"
// It is a quick check, no side effect so no need to do slow URL parsing.
const perfTracer = !window.location.search.includes('_ui_performance_trace=1') ? null : new InitPerformanceTracer();
if (perfTracer) {
for (const func of functions) perfTracer.recordCall(func.name, func);
} else {
for (const func of functions) func();
}
Comment on lines +20 to +24
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (perfTracer) {
for (const func of functions) perfTracer.recordCall(func.name, func);
} else {
for (const func of functions) func();
}
for (const func of functions) {
perfTracer?.recordCall(func.name, func) ?? func();
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think it is performant.

By using the if, we do not need to check perfTracer anymore in the loop

return perfTracer;
}
Loading
Loading