Skip to content

Refactor global init code and add more comments #33755

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

Merged
merged 7 commits into from
Mar 3, 2025
Merged
Show file tree
Hide file tree
Changes from all 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 Original file line 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="javascript:alert('xss')">bad</a>`, `bad`,
`<a href="vbscript:no">bad</a>`, `bad`, `<a href="vbscript:no">bad</a>`, `bad`,
`<a href="data:1234">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 { for i := 0; i < len(testCases); i += 2 {
Expand Down
16 changes: 8 additions & 8 deletions templates/base/alert.tmpl
Original file line number Original file line 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"> <div class="ui negative message flash-message flash-error">
<p>{{.Flash.ErrorMsg | SanitizeHTML}}</p> <p>{{.Flash.ErrorMsg | SanitizeHTML}}</p>
</div> </div>
{{end}} {{- end -}}
{{if .Flash.SuccessMsg}} {{- if .Flash.SuccessMsg -}}
<div class="ui positive message flash-message flash-success"> <div class="ui positive message flash-message flash-success">
<p>{{.Flash.SuccessMsg | SanitizeHTML}}</p> <p>{{.Flash.SuccessMsg | SanitizeHTML}}</p>
</div> </div>
{{end}} {{- end -}}
{{if .Flash.InfoMsg}} {{- if .Flash.InfoMsg -}}
<div class="ui info message flash-message flash-info"> <div class="ui info message flash-message flash-info">
<p>{{.Flash.InfoMsg | SanitizeHTML}}</p> <p>{{.Flash.InfoMsg | SanitizeHTML}}</p>
</div> </div>
{{end}} {{- end -}}
{{if .Flash.WarningMsg}} {{- if .Flash.WarningMsg -}}
<div class="ui warning message flash-message flash-warning"> <div class="ui warning message flash-message flash-warning">
<p>{{.Flash.WarningMsg | SanitizeHTML}}</p> <p>{{.Flash.WarningMsg | SanitizeHTML}}</p>
</div> </div>
{{end}} {{- end -}}
7 changes: 4 additions & 3 deletions templates/repo/issue/new_form.tmpl
Original file line number Original file line Diff line number Diff line change
@@ -1,6 +1,4 @@
{{if .Flash}}
{{template "base/alert" .}} {{template "base/alert" .}}
{{end}}
<form class="issue-content ui comment form form-fetch-action" id="new-issue" action="{{.Link}}" method="post"> <form class="issue-content ui comment form form-fetch-action" id="new-issue" action="{{.Link}}" method="post">
{{.CsrfTokenHtml}} {{.CsrfTokenHtml}}
<div class="issue-content-left"> <div class="issue-content-left">
Expand All @@ -9,7 +7,10 @@
{{ctx.AvatarUtils.Avatar .SignedUser 40}} {{ctx.AvatarUtils.Avatar .SignedUser 40}}
<div class="ui segment content tw-my-0"> <div class="ui segment content tw-my-0">
<div class="field"> <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}} {{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> <div class="title_wip_desc" data-wip-prefixes="{{JsonUtils.EncodeToString .PullRequestWorkInProgressPrefixes}}">{{ctx.Locale.Tr "repo.pulls.title_wip_desc" (index .PullRequestWorkInProgressPrefixes 0)}}</div>
{{end}} {{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 Original file line Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {GET} from '../modules/fetch.ts';
import {showGlobalErrorMessage} from '../bootstrap.ts'; import {showGlobalErrorMessage} from '../bootstrap.ts';
import {fomanticQuery} from '../modules/fomantic/base.ts'; import {fomanticQuery} from '../modules/fomantic/base.ts';
import {queryElems} from '../utils/dom.ts'; import {queryElems} from '../utils/dom.ts';
import {observeAddedElement} from '../modules/observer.ts'; import {registerGlobalInitFunc, registerGlobalSelectorFunc} from '../modules/observer.ts';


const {appUrl} = window.config; const {appUrl} = window.config;


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


export function initGlobalDropdown() { export function initGlobalDropdown() {
// do not init "custom" dropdowns, "custom" dropdowns are managed by their own code. // 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); const $dropdown = fomanticQuery(el);
if ($dropdown.data('module-dropdown')) return; // do not re-init if other code has already initialized it. 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}); 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: * Too many users set their ROOT_URL to wrong value, and it causes a lot of problems:
* * Cross-origin API request without correct cookie * * 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 Original file line 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 {createTippy} from '../modules/tippy.ts';
import {invertFileFolding} from './file-fold.ts'; import {invertFileFolding} from './file-fold.ts';
import {parseDom} from '../utils.ts'; import {parseDom} from '../utils.ts';
import {observeAddedElement} from '../modules/observer.ts'; import {registerGlobalSelectorFunc} from '../modules/observer.ts';


const {i18n} = window.config; const {i18n} = window.config;


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


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


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


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


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


initOAuth2SettingsDisableCheckbox, 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`);
}
}); });
26 changes: 26 additions & 0 deletions web_src/js/modules/init.ts
Original file line number Original file line Diff line number Diff line change
@@ -0,0 +1,26 @@
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++) {
console.info(`performance trace: ${this.results[i].name} ${this.results[i].dur.toFixed(3)}`);
}
}
}

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