fix

Author: wxiaoguang

modules/templates/helper_test.go

@@ -168,3 +168,10 @@ func TestQueryBuild(t *testing.T) {
 		assert.Equal(t, "&a=b&c=d&e=f", string(QueryBuild("&a=b&c=d&e=f", "k", "")))
 	})
 }
+
+func TestQueryEscape(t *testing.T) {
+	// this test is a reference for "urlQueryEscape" in JS
+	in := "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~" // all non-letter & non-number chars
+	expected := "%21%22%23%24%25%26%27%28%29%2A%2B%2C-.%2F%3A%3B%3C%3D%3E%3F%40%5B%5C%5D%5E_%60%7B%7C%7D~"
+	assert.Equal(t, expected, string(queryEscape(in)))
+}

options/locale/locale_en-US.json

@@ -1490,6 +1490,7 @@
   "repo.issues.filter_sort.feweststars": "Fewest stars",
   "repo.issues.filter_sort.mostforks": "Most forks",
   "repo.issues.filter_sort.fewestforks": "Fewest forks",
+  "repo.issues.quick_goto": "Goto issue",
   "repo.issues.action_open": "Open",
   "repo.issues.action_close": "Close",
   "repo.issues.action_label": "Label",

templates/repo/issue/search.tmpl

@@ -12,7 +12,7 @@
 		{{end}}
 		{{template "shared/search/input" dict "Value" .Keyword}}
 		{{if .PageIsIssueList}}
-			<button id="issue-list-quick-goto" class="ui small icon button tw-hidden" data-tooltip-content="{{ctx.Locale.Tr "explore.go_to"}}" data-repo-link="{{.RepoLink}}">{{svg "octicon-hash"}}</button>
+			<button id="issue-list-quick-goto" type="button" class="ui small icon button tw-hidden tw-mr-[-1px]" data-repo-link="{{.RepoLink}}">{{svg "octicon-hash" 12}} {{ctx.Locale.Tr "repo.issues.quick_goto"}}</button>
 		{{end}}
 		{{template "shared/search/button"}}
 	</div>

web_src/js/features/admin/common.ts

@@ -2,6 +2,7 @@ import {checkAppUrl} from '../common-page.ts';
 import {hideElem, queryElems, showElem, toggleElem} from '../../utils/dom.ts';
 import {POST} from '../../modules/fetch.ts';
 import {fomanticQuery} from '../../modules/fomantic/base.ts';
+import {urlQueryEscape} from "../../utils.ts";
 
 const {appSubUrl} = window.config;
 
@@ -230,7 +231,7 @@ function initAdminAuthentication() {
   const elAuthName = document.querySelector<HTMLInputElement>('#auth_name')!;
   const onAuthNameChange = function () {
     // appSubUrl is either empty or is a path that starts with `/` and doesn't have a trailing slash.
-    document.querySelector('#oauth2-callback-url')!.textContent = `${window.location.origin}${appSubUrl}/user/oauth2/${encodeURIComponent(elAuthName.value)}/callback`;
+    document.querySelector('#oauth2-callback-url')!.textContent = `${window.location.origin}${appSubUrl}/user/oauth2/${urlQueryEscape(elAuthName.value)}/callback`;
   };
   elAuthName.addEventListener('input', onAuthNameChange);
   onAuthNameChange();

web_src/js/features/common-issue-list.test.ts

@@ -5,7 +5,7 @@ test('parseIssueListQuickGotoLink', () => {
   expect(parseIssueListQuickGotoLink('/link', 'abc')).toEqual('');
   expect(parseIssueListQuickGotoLink('/link', '123')).toEqual('/link/issues/123');
   expect(parseIssueListQuickGotoLink('/link', '#123')).toEqual('/link/issues/123');
-  expect(parseIssueListQuickGotoLink('/link', 'owner/repo#123')).toEqual('');
+  expect(parseIssueListQuickGotoLink('/link', 'owner/repo#123')).toEqual('/owner/repo/issues/123');
 
   expect(parseIssueListQuickGotoLink('', '')).toEqual('');
   expect(parseIssueListQuickGotoLink('', 'abc')).toEqual('');

web_src/js/features/common-issue-list.ts

@@ -1,4 +1,4 @@
-import {isElemVisible, onInputDebounce, submitEventSubmitter, toggleElem} from '../utils/dom.ts';
+import {onInputDebounce, toggleElem} from '../utils/dom.ts';
 import {GET} from '../modules/fetch.ts';
 
 const {appSubUrl} = window.config;
@@ -17,37 +17,25 @@ export function parseIssueListQuickGotoLink(repoLink: string, searchText: string
     } else if (reIssueSharpIndex.test(searchText)) {
       targetUrl = `${repoLink}/issues/${searchText.substring(1)}`;
     }
-  } else {
-    // try to parse it for a global search (eg: "owner/repo#123")
-    const [_, owner, repo, index] = reIssueOwnerRepoIndex.exec(searchText) || [];
-    if (owner) {
-      targetUrl = `${appSubUrl}/${owner}/${repo}/issues/${index}`;
-    }
+  }
+  // try to parse it for a global search (eg: "owner/repo#123")
+  const [_, owner, repo, index] = reIssueOwnerRepoIndex.exec(searchText) || [];
+  if (owner) {
+    targetUrl = `${appSubUrl}/${owner}/${repo}/issues/${index}`;
   }
   return targetUrl;
 }
 
 export function initCommonIssueListQuickGoto() {
-  const goto = document.querySelector<HTMLElement>('#issue-list-quick-goto');
-  if (!goto) return;
+  const elGotoButton = document.querySelector<HTMLElement>('#issue-list-quick-goto');
+  if (!elGotoButton) return;
 
-  const form = goto.closest('form')!;
+  const form = elGotoButton.closest('form')!;
   const input = form.querySelector<HTMLInputElement>('input[name=q]')!;
-  const repoLink = goto.getAttribute('data-repo-link')!;
+  const repoLink = elGotoButton.getAttribute('data-repo-link') || '';
 
-  form.addEventListener('submit', (e) => {
-    // if there is no goto button, or the form is submitted by non-quick-goto elements, submit the form directly
-    let doQuickGoto = isElemVisible(goto);
-    const submitter = submitEventSubmitter(e);
-    if (submitter !== form && submitter !== input && submitter !== goto) doQuickGoto = false;
-    if (!doQuickGoto) return;
-
-    // if there is a goto button, use its link
-    e.preventDefault();
-    const link = goto.getAttribute('data-issue-goto-link');
-    if (link) {
-      window.location.href = link;
-    }
+  elGotoButton.addEventListener('click', () => {
+    window.location.href = elGotoButton.getAttribute('data-issue-goto-link')!;
   });
 
   const onInput = async () => {
@@ -61,8 +49,8 @@ export function initCommonIssueListQuickGoto() {
     // if the input value has changed, then ignore the result
     if (input.value !== searchText) return;
 
-    toggleElem(goto, Boolean(targetUrl));
-    goto.setAttribute('data-issue-goto-link', targetUrl);
+    toggleElem(elGotoButton, Boolean(targetUrl));
+    elGotoButton.setAttribute('data-issue-goto-link', targetUrl);
   };
 
   input.addEventListener('input', onInputDebounce(onInput));

web_src/js/utils.test.ts

@@ -2,6 +2,7 @@ import {
   dirname, basename, extname, isObject, stripTags, parseIssueHref,
   parseUrl, translateMonth, translateDay, blobToDataURI,
   toAbsoluteUrl, encodeURLEncodedBase64, decodeURLEncodedBase64, isImageFile, isVideoFile, parseRepoOwnerPathInfo,
+  urlQueryEscape,
 } from './utils.ts';
 
 test('dirname', () => {
@@ -33,6 +34,12 @@ test('stripTags', () => {
   expect(stripTags('<a>test</a>')).toEqual('test');
 });
 
+test('urlQueryEscape', () => {
+  const input = "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~";
+  const expected = '%21%22%23%24%25%26%27%28%29%2A%2B%2C-.%2F%3A%3B%3C%3D%3E%3F%40%5B%5C%5D%5E_%60%7B%7C%7D~';
+  expect(urlQueryEscape(input)).toEqual(expected);
+});
+
 test('parseIssueHref', () => {
   expect(parseIssueHref('/owner/repo/issues/1')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'});
   expect(parseIssueHref('/owner/repo/pulls/1?query')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'pulls', indexString: '1'});

web_src/js/utils.ts

@@ -43,6 +43,15 @@ export function stripTags(text: string): string {
   return text;
 }
 
+export function urlQueryEscape(s: string) {
+  // See "TestQueryEscape" in backend
+  // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent#encoding_for_rfc3986
+  return encodeURIComponent(s).replace(
+    /[!'()*]/g,
+    (c) => `%${c.charCodeAt(0).toString(16).toUpperCase()}`,
+  );
+}
+
 export function parseIssueHref(href: string): IssuePathInfo {
   // FIXME: it should use pathname and trim the appSubUrl ahead
   const path = (href || '').replace(/[#?].*$/, '');