Merge changes I855eefa5,I551eafd9
* changes:
Remove experiment PATCHSET_LEVEL_COMMENT_USES_GRCOMMENT
Remove setting explicit focus on comment textarea
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
index f31dc13..088002c 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
@@ -18,7 +18,7 @@
import '../gr-repo-detail-list/gr-repo-detail-list';
import '../gr-repo-list/gr-repo-list';
import {getBaseUrl} from '../../../utils/url-util';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {
AdminNavLinksOption,
@@ -122,6 +122,8 @@
private readonly routerModel = getAppContext().routerModel;
+ private readonly getNavigation = resolve(this, navigationToken);
+
constructor() {
super();
subscribe(
@@ -549,7 +551,7 @@
if (this.selectedIsCurrentPage(selected)) return;
if (selected.url === undefined) return;
if (this.reloading) return;
- GerritNav.navigateToRelativeUrl(selected.url);
+ this.getNavigation().setUrl(selected.url);
}
isAdminView(): boolean {
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts
index 532c17f..d65d171 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts
@@ -6,7 +6,6 @@
import '../../../test/common-test-setup';
import './gr-admin-view';
import {AdminSubsectionLink, GrAdminView} from './gr-admin-view';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {stubBaseUrl, stubElement, stubRestApi} from '../../../test/test-utils';
import {GerritView} from '../../../services/router/router-model';
@@ -19,6 +18,8 @@
import {AdminChildView} from '../../../models/views/admin';
import {GroupDetailView} from '../../../models/views/group';
import {RepoDetailView} from '../../../models/views/repo';
+import {testResolver} from '../../../test/common-test-setup';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
function createAdminCapabilities() {
return {
@@ -456,10 +457,7 @@
parent: 'my-repo' as RepoName,
},
];
- const navigateToRelativeUrlStub = sinon.stub(
- GerritNav,
- 'navigateToRelativeUrl'
- );
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
const selectedIsCurrentPageSpy = sinon.spy(
element,
'selectedIsCurrentPage'
@@ -475,14 +473,14 @@
);
assert.equal(selectedIsCurrentPageSpy.callCount, 1);
// Doesn't trigger navigation from the page select menu.
- assert.isFalse(navigateToRelativeUrlStub.called);
+ assert.isFalse(setUrlStub.called);
// When explicitly changed, navigation is called
queryAndAssert<GrDropdownList>(element, '#pageSelect').value =
'repogeneral';
await queryAndAssert<GrDropdownList>(element, '#pageSelect').updateComplete;
assert.equal(selectedIsCurrentPageSpy.callCount, 2);
- assert.isTrue(navigateToRelativeUrlStub.calledOnce);
+ assert.isTrue(setUrlStub.calledOnce);
});
test('selectedIsCurrentPage', () => {
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
index 202ef88..c716d65 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
@@ -17,6 +17,7 @@
AccountInfo,
GroupInfo,
GroupName,
+ ServerInfo,
} from '../../../types/common';
import {
AutocompleteQuery,
@@ -40,6 +41,9 @@
import {customElement, property, query, state} from 'lit/decorators.js';
import {ifDefined} from 'lit/directives/if-defined.js';
import {getAccountSuggestions} from '../../../utils/account-util';
+import {subscribe} from '../../lit/subscription-controller';
+import {configModelToken} from '../../../models/config/config-model';
+import {resolve} from '../../../models/dependency';
const SAVING_ERROR_TEXT =
'Group may not exist, or you may not have ' + 'permission to add it';
@@ -101,10 +105,21 @@
private readonly restApiService = getAppContext().restApiService;
+ private readonly getConfigModel = resolve(this, configModelToken);
+
+ private serverConfig?: ServerInfo;
+
constructor() {
super();
+ subscribe(
+ this,
+ () => this.getConfigModel().serverConfig$,
+ config => {
+ this.serverConfig = config;
+ }
+ );
this.queryMembers = input =>
- getAccountSuggestions(input, this.restApiService);
+ getAccountSuggestions(input, this.restApiService, this.serverConfig);
this.queryIncludedGroup = input => this.getGroupSuggestions(input);
}
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members_test.ts b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members_test.ts
index 2879c51..6c65dd6 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members_test.ts
@@ -29,6 +29,7 @@
import {getAccountSuggestions} from '../../../utils/account-util';
import {getAppContext} from '../../../services/app-context';
import {fixture, html, assert} from '@open-wc/testing';
+import {createServerInfo} from '../../../test/test-data-generators';
suite('gr-group-members tests', () => {
let element: GrGroupMembers;
@@ -102,6 +103,7 @@
name: 'test-account',
email: 'test.account@example.com' as EmailAddress,
username: 'test123',
+ display_name: 'display-test-account',
},
{
_account_id: 1001439 as AccountId,
@@ -498,7 +500,8 @@
test('getAccountSuggestions empty', async () => {
const accounts = await getAccountSuggestions(
'nonexistent',
- getAppContext().restApiService
+ getAppContext().restApiService,
+ createServerInfo()
);
assert.equal(accounts.length, 0);
});
@@ -506,10 +509,14 @@
test('getAccountSuggestions non-empty', async () => {
const accounts = await getAccountSuggestions(
'test-',
- getAppContext().restApiService
+ getAppContext().restApiService,
+ createServerInfo()
);
assert.equal(accounts.length, 3);
- assert.equal(accounts[0].name, 'test-account <test.account@example.com>');
+ assert.equal(
+ accounts[0].name,
+ 'display-test-account <test.account@example.com>'
+ );
assert.equal(accounts[1].name, 'test-admin <test.admin@example.com>');
assert.equal(accounts[2].name, 'test-git');
});
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts b/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts
index e761d1b..93f9159 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts
@@ -10,7 +10,7 @@
import '../../shared/gr-dialog/gr-dialog';
import '../../shared/gr-overlay/gr-overlay';
import '../gr-create-change-dialog/gr-create-change-dialog';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {
BranchName,
ConfigInfo,
@@ -34,6 +34,7 @@
import {customElement, query, property, state} from 'lit/decorators.js';
import {assertIsDefined} from '../../../utils/common-util';
import {createEditUrl} from '../../../models/views/edit';
+import {resolve} from '../../../models/dependency';
const GC_MESSAGE = 'Garbage collection completed successfully.';
const CONFIG_BRANCH = 'refs/meta/config' as BranchName;
@@ -74,6 +75,8 @@
private readonly restApiService = getAppContext().restApiService;
+ private readonly getNavigation = resolve(this, navigationToken);
+
override connectedCallback() {
super.connectedCallback();
fireTitleChange(this, 'Repo Commands');
@@ -284,7 +287,7 @@
return;
}
- GerritNav.navigateToRelativeUrl(
+ this.getNavigation().setUrl(
createEditUrl({
changeNum: change._number,
project: change.project,
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands_test.ts b/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands_test.ts
index eb2c81b..77caf5e 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands_test.ts
@@ -6,7 +6,6 @@
import '../../../test/common-test-setup';
import './gr-repo-commands';
import {GrRepoCommands} from './gr-repo-commands';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {
addListenerForTest,
mockPromise,
@@ -150,7 +149,6 @@
setup(() => {
createChangeStub = stubRestApi('createChange');
- sinon.stub(GerritNav, 'navigateToRelativeUrl');
handleSpy = sinon.spy(element, 'handleEditRepoConfig');
alertStub = sinon.stub();
element.repo = 'test' as RepoName;
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
index c5a007e..77a0eb2 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
@@ -18,10 +18,7 @@
import '../gr-confirm-revert-dialog/gr-confirm-revert-dialog';
import '../gr-confirm-submit-dialog/gr-confirm-submit-dialog';
import '../../../styles/shared-styles';
-import {
- GerritNav,
- navigationToken,
-} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {getAppContext} from '../../../services/app-context';
import {CURRENT} from '../../../utils/patch-set-util';
@@ -1863,7 +1860,7 @@
}
case ChangeActions.DELETE:
if (action.__type === ActionType.CHANGE) {
- GerritNav.navigateToRelativeUrl(rootUrl());
+ this.getNavigation().setUrl(rootUrl());
}
break;
case ChangeActions.WIP:
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 574991e..ed752e6 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -15,7 +15,7 @@
import '../../shared/gr-change-star/gr-change-star';
import '../../shared/gr-change-status/gr-change-status';
import '../../shared/gr-editable-content/gr-editable-content';
-import '../../shared/gr-linked-text/gr-linked-text';
+import '../../shared/gr-markdown/gr-markdown';
import '../../shared/gr-overlay/gr-overlay';
import '../../shared/gr-tooltip-content/gr-tooltip-content';
import '../gr-change-actions/gr-change-actions';
@@ -37,10 +37,7 @@
import {GrEditConstants} from '../../edit/gr-edit-constants';
import {pluralize} from '../../../utils/string-util';
import {querySelectorAll, windowLocationReload} from '../../../utils/dom-util';
-import {
- GerritNav,
- navigationToken,
-} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {RevisionInfo as RevisionInfoClass} from '../../shared/revision-info/revision-info';
@@ -194,7 +191,6 @@
const MIN_LINES_FOR_COMMIT_COLLAPSE = 18;
-const REVIEWERS_REGEX = /^(R|CC)=/gm;
const MIN_CHECK_INTERVAL_SECS = 0;
const REPLY_REFIT_DEBOUNCE_INTERVAL_MS = 500;
@@ -962,7 +958,7 @@
/* Account for border and padding and rounding errors. */
max-width: calc(72ch + 2px + 2 * var(--spacing-m) + 0.4px);
}
- .commitMessage gr-linked-text {
+ .commitMessage gr-markdown {
word-break: break-word;
}
#commitMessageEditor {
@@ -1463,12 +1459,9 @@
.commitCollapsible=${this.computeCommitCollapsible()}
remove-zero-width-space=""
>
- <gr-linked-text
- pre=""
- .content=${this.latestCommitMessage}
- .config=${this.projectConfig?.commentlinks}
- remove-zero-width-space=""
- ></gr-linked-text>
+ <gr-markdown
+ .content=${this.latestCommitMessage ?? ''}
+ ></gr-markdown>
</gr-editable-content>
</div>
<h3 class="assistive-tech-only">Comments and Checks Summary</h3>
@@ -1827,7 +1820,7 @@
return;
}
- this.latestCommitMessage = this.prepareCommitMsgForLinkify(message);
+ this.latestCommitMessage = message;
this.editingCommitMessage = false;
this.reloadWindow();
})
@@ -2619,7 +2612,7 @@
private determinePageBack() {
// Default backPage to root if user came to change view page
// via an email link, etc.
- GerritNav.navigateToRelativeUrl(this.backPage || rootUrl());
+ this.getNavigation().setUrl(this.backPage || rootUrl());
}
private handleLabelRemoved(
@@ -2677,14 +2670,6 @@
this.changeViewAriaHidden = true;
}
- // Private but used in tests.
- prepareCommitMsgForLinkify(msg: string) {
- // TODO(wyatta) switch linkify sequence, see issue 5526.
- // This is a zero-with space. It is added to prevent the linkify library
- // from including R= or CC= as part of the email address.
- return msg.replace(REVIEWERS_REGEX, '$1=\u200B');
- }
-
/**
* Utility function to make the necessary modifications to a change in the
* case an edit exists.
@@ -2814,9 +2799,7 @@
throw new Error('Could not find latest Revision Sha');
const currentRevision = this.change.revisions[latestRevisionSha];
if (currentRevision.commit && currentRevision.commit.message) {
- this.latestCommitMessage = this.prepareCommitMsgForLinkify(
- currentRevision.commit.message
- );
+ this.latestCommitMessage = currentRevision.commit.message;
} else {
this.latestCommitMessage = null;
}
@@ -2869,9 +2852,7 @@
.getChangeCommitInfo(this.changeNum, lastpatchNum)
.then(commitInfo => {
if (!commitInfo) return;
- this.latestCommitMessage = this.prepareCommitMsgForLinkify(
- commitInfo.message
- );
+ this.latestCommitMessage = commitInfo.message;
});
}
@@ -3232,7 +3213,7 @@
break;
case GrEditConstants.Actions.OPEN.id:
assertIsDefined(this.patchRange.patchNum, 'patchset number');
- GerritNav.navigateToRelativeUrl(
+ this.getNavigation().setUrl(
createEditUrl({
changeNum: this.change._number,
project: this.change.project,
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 79a9dfe..49248ea 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -19,10 +19,7 @@
} from '../../../constants/constants';
import {GrEditConstants} from '../../edit/gr-edit-constants';
import {_testOnly_resetEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
-import {
- GerritNav,
- navigationToken,
-} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {EventType, PluginApi} from '../../../api/plugin';
import {
@@ -436,9 +433,7 @@
id="commitMessageEditor"
remove-zero-width-space=""
>
- <gr-linked-text pre="" remove-zero-width-space="">
- <span id="output" slot="insert"> </span>
- </gr-linked-text>
+ <gr-markdown></gr-markdown>
</gr-editable-content>
</div>
<h3 class="assistive-tech-only">
@@ -797,20 +792,16 @@
});
test('U should navigate to root if no backPage set', () => {
- const relativeNavStub = sinon.stub(GerritNav, 'navigateToRelativeUrl');
pressKey(element, 'u');
- assert.isTrue(relativeNavStub.called);
- assert.isTrue(relativeNavStub.lastCall.calledWithExactly(rootUrl()));
+ assert.isTrue(setUrlStub.called);
+ assert.isTrue(setUrlStub.lastCall.calledWithExactly(rootUrl()));
});
test('U should navigate to backPage if set', () => {
- const relativeNavStub = sinon.stub(GerritNav, 'navigateToRelativeUrl');
element.backPage = '/dashboard/self';
pressKey(element, 'u');
- assert.isTrue(relativeNavStub.called);
- assert.isTrue(
- relativeNavStub.lastCall.calledWithExactly('/dashboard/self')
- );
+ assert.isTrue(setUrlStub.called);
+ assert.isTrue(setUrlStub.lastCall.calledWithExactly('/dashboard/self'));
});
test('A fires an error event when not logged in', async () => {
@@ -1416,20 +1407,6 @@
assert.isTrue(overlayOpenStub.called);
});
- test('prepareCommitMsgForLinkify', () => {
- let commitMessage = 'R=test@google.com';
- let result = element.prepareCommitMsgForLinkify(commitMessage);
- assert.equal(result, 'R=\u200Btest@google.com');
-
- commitMessage = 'R=test@google.com\nR=test@google.com';
- result = element.prepareCommitMsgForLinkify(commitMessage);
- assert.equal(result, 'R=\u200Btest@google.com\nR=\u200Btest@google.com');
-
- commitMessage = 'CC=test@google.com';
- result = element.prepareCommitMsgForLinkify(commitMessage);
- assert.equal(result, 'CC=\u200Btest@google.com');
- });
-
test('_isSubmitEnabled', () => {
assert.isFalse(element.isSubmitEnabled());
element.currentRevisionActions = {submit: {}};
@@ -2114,10 +2091,6 @@
const openDeleteDialogStub = sinon.stub(controls, 'openDeleteDialog');
const openRenameDialogStub = sinon.stub(controls, 'openRenameDialog');
const openRestoreDialogStub = sinon.stub(controls, 'openRestoreDialog');
- const navigateToRelativeUrlStub = sinon.stub(
- GerritNav,
- 'navigateToRelativeUrl'
- );
// Delete
fileList.dispatchEvent(
@@ -2168,7 +2141,7 @@
);
await element.updateComplete;
- assert.isTrue(navigateToRelativeUrlStub.called);
+ assert.isTrue(setUrlStub.called);
});
test('selectedRevision updates when patchNum is changed', async () => {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index 5daa3d2..731f227 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -12,7 +12,6 @@
import '../../shared/gr-button/gr-button';
import '../../shared/gr-cursor-manager/gr-cursor-manager';
import '../../shared/gr-icon/gr-icon';
-import '../../shared/gr-linked-text/gr-linked-text';
import '../../shared/gr-select/gr-select';
import '../../shared/gr-tooltip-content/gr-tooltip-content';
import '../../shared/gr-copy-clipboard/gr-copy-clipboard';
@@ -21,10 +20,7 @@
import {asyncForeach} from '../../../utils/async-util';
import {FilesExpandedState} from '../gr-file-list-constants';
import {diffFilePaths, pluralize} from '../../../utils/string-util';
-import {
- GerritNav,
- navigationToken,
-} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {getAppContext} from '../../../services/app-context';
@@ -2046,11 +2042,13 @@
if (!this.change || !diff || !this.patchRange || !diff.path) {
throw new Error('change, diff and patchRange must be all set and valid');
}
- GerritNav.navigateToDiff(
- this.change,
- diff.path,
- this.patchRange.patchNum,
- this.patchRange.basePatchNum
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: diff.path,
+ patchNum: this.patchRange.patchNum,
+ basePatchNum: this.patchRange.basePatchNum,
+ })
);
}
@@ -2065,11 +2063,13 @@
if (!this.change || !this.patchRange) {
throw new Error('change and patchRange must be set');
}
- GerritNav.navigateToDiff(
- this.change,
- this.files[this.fileCursor.index].__path,
- this.patchRange.patchNum,
- this.patchRange.basePatchNum
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: this.files[this.fileCursor.index].__path,
+ patchNum: this.patchRange.patchNum,
+ basePatchNum: this.patchRange.basePatchNum,
+ })
);
}
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
index c693d74..76335cb 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
@@ -7,7 +7,7 @@
import '../../shared/gr-date-formatter/gr-date-formatter';
import './gr-file-list';
import {FilesExpandedState} from '../gr-file-list-constants';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {
mockPromise,
query,
@@ -55,6 +55,7 @@
import {GrIcon} from '../../shared/gr-icon/gr-icon';
import {fixture, html, assert} from '@open-wc/testing';
import {Modifier} from '../../../utils/dom-util';
+import {testResolver} from '../../../test/common-test-setup';
suite('gr-diff a11y test', () => {
test('audit', async () => {
@@ -928,7 +929,10 @@
basePatchNum: PARENT,
patchNum: 2 as RevisionPatchSetNum,
};
- element.change = {_number: 42 as NumericChangeId} as ParsedChangeInfo;
+ element.change = {
+ _number: 42 as NumericChangeId,
+ project: 'test-project',
+ } as ParsedChangeInfo;
element.fileCursor.setCursorAtIndex(0);
await element.updateComplete;
await waitEventLoop();
@@ -966,7 +970,7 @@
assert.equal(element.selectedIndex, 1);
pressKey(element, 'j');
- const navStub = sinon.stub(GerritNav, 'navigateToDiff');
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
assert.equal(element.fileCursor.index, 2);
assert.equal(element.selectedIndex, 2);
@@ -983,13 +987,10 @@
assert.equal(element.selectedIndex, 1);
pressKey(element, 'o');
- assert(
- navStub.lastCall.calledWith(
- element.change,
- 'file_added_in_rev2.txt',
- 2 as RevisionPatchSetNum
- ),
- 'Should navigate to /c/42/2/file_added_in_rev2.txt'
+ assert.equal(setUrlStub.callCount, 1);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/2/file_added_in_rev2.txt'
);
pressKey(element, 'k');
@@ -2241,16 +2242,16 @@
const files = element.files;
element.files = [];
await element.updateComplete;
- const navStub = sinon.stub(GerritNav, 'navigateToDiff');
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
// Noop when there are no files.
element.openSelectedFile();
- assert.isFalse(navStub.called);
+ assert.isFalse(setUrlStub.calledOnce);
element.files = files;
await element.updateComplete;
// Navigates when a file is selected.
element.openSelectedFile();
- assert.isTrue(navStub.called);
+ assert.isTrue(setUrlStub.calledOnce);
});
test('displayLine', () => {
diff --git a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
index 69a7b1d..4af24cc 100644
--- a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
+++ b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
@@ -3,26 +3,8 @@
* Copyright 2017 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {
- BasePatchSetNum,
- ChangeInfo,
- RevisionPatchSetNum,
-} from '../../../types/common';
-import {ParsedChangeInfo} from '../../../types/types';
-import {createDiffUrl} from '../../../models/views/diff';
import {define} from '../../../models/dependency';
-const uninitialized = () => {
- console.warn('Use of uninitialized routing');
-};
-
-const uninitializedNavigate: NavigateCallback = () => {
- uninitialized();
- return '';
-};
-
-export type NavigateCallback = (target: string, redirect?: boolean) => void;
-
export const navigationToken = define<NavigationService>('navigation');
export interface NavigationService {
@@ -45,57 +27,3 @@
*/
replaceUrl(url: string): void;
}
-
-// TODO(dmfilippov) Convert to class, extract consts, give better name and
-// expose as a service from appContext
-export const GerritNav = {
- _navigate: uninitializedNavigate,
-
- /**
- * Setup router implementation.
- *
- * @param navigate the router-abstracted equivalent of
- * `window.location.href = ...` or window.location.replace(...). The
- * string is a new location and boolean defines is it redirect or not
- * (true means redirect, i.e. equivalent of window.location.replace).
- */
- setup(navigate: NavigateCallback) {
- this._navigate = navigate;
- },
-
- destroy() {
- this._navigate = uninitializedNavigate;
- },
-
- /**
- * @param basePatchNum The string PARENT can be used for none.
- */
- navigateToDiff(
- change: ChangeInfo | ParsedChangeInfo,
- filePath: string,
- patchNum?: RevisionPatchSetNum,
- basePatchNum?: BasePatchSetNum,
- lineNum?: number
- ) {
- this._navigate(
- createDiffUrl({
- changeNum: change._number,
- project: change.project,
- path: filePath,
- patchNum,
- basePatchNum,
- lineNum,
- })
- );
- },
-
- /**
- * Navigate to an arbitrary relative URL.
- */
- navigateToRelativeUrl(relativeUrl: string) {
- if (!relativeUrl.startsWith('/')) {
- throw new Error('navigateToRelativeUrl with non-relative URL');
- }
- this._navigate(relativeUrl);
- },
-};
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index c1728a2..762f6b7 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -8,7 +8,7 @@
PageContext,
PageNextCallback,
} from '../../../utils/page-wrapper-utils';
-import {GerritNav, NavigationService} from '../gr-navigation/gr-navigation';
+import {NavigationService} from '../gr-navigation/gr-navigation';
import {getAppContext} from '../../../services/app-context';
import {convertToPatchSetNum} from '../../../utils/patch-set-util';
import {assertIsDefined} from '../../../utils/common-util';
@@ -477,14 +477,6 @@
page.base(base);
}
- GerritNav.setup((url, redirect?) => {
- if (redirect) {
- page.redirect(url);
- } else {
- page.show(url);
- }
- });
-
page.exit('*', (_, next) => {
if (!this._isRedirecting) {
this.reporting.beforeLocationChanged();
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
index 47461eb..ce3ed0d 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
@@ -6,7 +6,6 @@
import '../../../test/common-test-setup';
import './gr-router';
import {page, PageContext} from '../../../utils/page-wrapper-utils';
-import {GerritNav} from '../gr-navigation/gr-navigation';
import {
stubBaseUrl,
stubRestApi,
@@ -104,7 +103,6 @@
const requiresAuth: any = {};
const doesNotRequireAuth: any = {};
- sinon.stub(GerritNav, 'setup');
sinon.stub(page, 'start');
sinon.stub(page, 'base');
sinon
@@ -368,7 +366,6 @@
onExit = _onExit;
};
sinon.stub(page, 'exit').callsFake(onRegisteringExit);
- sinon.stub(GerritNav, 'setup');
sinon.stub(page, 'start');
sinon.stub(page, 'base');
router.startRouter();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 295d4b9..18322fe 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -21,10 +21,7 @@
import '../gr-patch-range-select/gr-patch-range-select';
import '../../change/gr-download-dialog/gr-download-dialog';
import '../../shared/gr-overlay/gr-overlay';
-import {
- GerritNav,
- navigationToken,
-} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getAppContext} from '../../../services/app-context';
import {
computeAllPatchSets,
@@ -1163,11 +1160,13 @@
return;
}
- GerritNav.navigateToDiff(
- this.change,
- this.commentSkips.previous,
- this.patchRange.patchNum,
- this.patchRange.basePatchNum
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: this.commentSkips.previous,
+ patchNum: this.patchRange.patchNum,
+ basePatchNum: this.patchRange.basePatchNum,
+ })
);
}
@@ -1183,11 +1182,13 @@
return;
}
- GerritNav.navigateToDiff(
- this.change,
- this.commentSkips.next,
- this.patchRange.patchNum,
- this.patchRange.basePatchNum
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: this.commentSkips.next,
+ patchNum: this.patchRange.patchNum,
+ basePatchNum: this.patchRange.basePatchNum,
+ })
);
}
@@ -1362,12 +1363,14 @@
newPath.path,
this.patchRange
)?.[0].line;
- GerritNav.navigateToDiff(
- this.change,
- newPath.path,
- this.patchRange.patchNum,
- this.patchRange.basePatchNum,
- lineNum
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: newPath.path,
+ patchNum: this.patchRange.patchNum,
+ basePatchNum: this.patchRange.basePatchNum,
+ lineNum,
+ })
);
}
@@ -1407,7 +1410,7 @@
patchNum: this.patchRange.patchNum,
lineNum: cursorAddress?.number,
});
- GerritNav.navigateToRelativeUrl(editUrl);
+ this.getNavigation().setUrl(editUrl);
}
/**
@@ -1712,12 +1715,14 @@
${this.patchRange.patchNum}. Showing diff of Base vs
${this.patchRange.basePatchNum}`
);
- GerritNav.navigateToDiff(
- this.change,
- this.path,
- this.patchRange.basePatchNum as RevisionPatchSetNum,
- PARENT,
- this.focusLineNum
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: this.path,
+ patchNum: this.patchRange.basePatchNum as RevisionPatchSetNum,
+ basePatchNum: PARENT,
+ lineNum: this.focusLineNum,
+ })
);
return;
}
@@ -1884,11 +1889,13 @@
return;
}
- GerritNav.navigateToDiff(
- this.change,
- path,
- this.patchRange.patchNum,
- this.patchRange.basePatchNum
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path,
+ patchNum: this.patchRange.patchNum,
+ basePatchNum: this.patchRange.basePatchNum,
+ })
);
}
@@ -1905,7 +1912,14 @@
) {
return;
}
- GerritNav.navigateToDiff(this.change, this.path, patchNum, basePatchNum);
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: this.path,
+ patchNum,
+ basePatchNum,
+ })
+ );
}
// Private but used in tests.
@@ -2111,7 +2125,13 @@
fireAlert(this, 'Base is already selected.');
return;
}
- GerritNav.navigateToDiff(this.change, this.path, this.patchRange.patchNum);
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: this.path,
+ patchNum: this.patchRange.patchNum,
+ })
+ );
}
// Private but used in tests.
@@ -2124,14 +2144,18 @@
fireAlert(this, 'Left is already base.');
return;
}
- GerritNav.navigateToDiff(
- this.change,
- this.path,
- this.patchRange.basePatchNum as RevisionPatchSetNum,
- PARENT,
+ const lineNum =
this.viewState?.view === GerritView.DIFF && this.viewState?.commentLink
? this.focusLineNum
- : undefined
+ : undefined;
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: this.path,
+ patchNum: this.patchRange.basePatchNum as RevisionPatchSetNum,
+ basePatchNum: PARENT,
+ lineNum,
+ })
);
}
@@ -2147,11 +2171,13 @@
return;
}
- GerritNav.navigateToDiff(
- this.change,
- this.path,
- latestPatchNum,
- this.patchRange.basePatchNum
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: this.path,
+ patchNum: latestPatchNum,
+ basePatchNum: this.patchRange.basePatchNum,
+ })
);
}
@@ -2166,11 +2192,13 @@
fireAlert(this, 'Right is already latest.');
return;
}
- GerritNav.navigateToDiff(
- this.change,
- this.path,
- latestPatchNum,
- this.patchRange.patchNum as BasePatchSetNum
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: this.path,
+ patchNum: latestPatchNum,
+ basePatchNum: this.patchRange.patchNum as BasePatchSetNum,
+ })
);
}
@@ -2188,7 +2216,13 @@
fireAlert(this, 'Already diffing base against latest.');
return;
}
- GerritNav.navigateToDiff(this.change, this.path, latestPatchNum);
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: this.path,
+ patchNum: latestPatchNum,
+ })
+ );
}
// Private but used in tests.
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
index 134f23e..b6e26ab 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
@@ -5,10 +5,7 @@
*/
import '../../../test/common-test-setup';
import './gr-diff-view';
-import {
- GerritNav,
- navigationToken,
-} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {
ChangeStatus,
DiffViewMode,
@@ -72,7 +69,6 @@
import {EventType} from '../../../types/events';
import {Key} from '../../../utils/dom-util';
import {GrButton} from '../../shared/gr-button/gr-button';
-import {createEditUrl} from '../../../models/views/edit';
import {testResolver} from '../../../test/common-test-setup';
function createComment(
@@ -268,8 +264,7 @@
});
});
- test('unchanged diff X vs latest from comment links navigates to base vs X', () => {
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
+ test('unchanged diff X vs latest from comment links navigates to base vs X', async () => {
element.getCommentsModel().setState({
comments: {
'/COMMIT_MSG': [
@@ -307,21 +302,15 @@
...createParsedChange(),
revisions: createRevisions(11),
};
- return viewStateChangedSpy.returnValues[0]?.then(() => {
- assert.isTrue(
- diffNavStub.lastCall.calledWithExactly(
- element.change!,
- '/COMMIT_MSG',
- 2 as RevisionPatchSetNum,
- PARENT,
- 10
- )
- );
- });
+ await viewStateChangedSpy.returnValues[0];
+ assert.isTrue(setUrlStub.calledOnce);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/2//COMMIT_MSG#10'
+ );
});
- test('unchanged diff Base vs latest from comment does not navigate', () => {
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
+ test('unchanged diff Base vs latest from comment does not navigate', async () => {
element.getCommentsModel().setState({
comments: {
'/COMMIT_MSG': [
@@ -359,9 +348,8 @@
...createParsedChange(),
revisions: createRevisions(11),
};
- return viewStateChangedSpy.returnValues[0]!.then(() => {
- assert.isFalse(diffNavStub.called);
- });
+ await viewStateChangedSpy.returnValues[0];
+ assert.isFalse(setUrlStub.calledOnce);
});
test('isFileUnchanged', () => {
@@ -655,23 +643,18 @@
element.path = 'glados.txt';
element.loggedIn = true;
await element.updateComplete;
-
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
+ setUrlStub.reset();
pressKey(element, 'u');
- assert.isTrue(setUrlStub.calledOnce);
+ assert.equal(setUrlStub.callCount, 1);
assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42');
await element.updateComplete;
pressKey(element, ']');
- assert(
- diffNavStub.lastCall.calledWith(
- element.change,
- 'wheatley.md',
- 10 as RevisionPatchSetNum,
- PARENT
- ),
- 'Should navigate to /c/42/10/wheatley.md'
+ assert.equal(setUrlStub.callCount, 2);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/10/wheatley.md'
);
element.path = 'wheatley.md';
await element.updateComplete;
@@ -679,14 +662,10 @@
assert.isTrue(element.loading);
pressKey(element, '[');
- assert(
- diffNavStub.lastCall.calledWith(
- element.change,
- 'glados.txt',
- 10 as RevisionPatchSetNum,
- PARENT
- ),
- 'Should navigate to /c/42/10/glados.txt'
+ assert.equal(setUrlStub.callCount, 3);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/10/glados.txt'
);
element.path = 'glados.txt';
await element.updateComplete;
@@ -694,14 +673,10 @@
assert.isTrue(element.loading);
pressKey(element, '[');
- assert(
- diffNavStub.lastCall.calledWith(
- element.change,
- 'chell.go',
- 10 as RevisionPatchSetNum,
- PARENT
- ),
- 'Should navigate to /c/42/10/chell.go'
+ assert.equal(setUrlStub.callCount, 4);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/10/chell.go'
);
element.path = 'chell.go';
await element.updateComplete;
@@ -709,7 +684,7 @@
assert.isTrue(element.loading);
pressKey(element, '[');
- assert.isTrue(setUrlStub.calledTwice);
+ assert.equal(setUrlStub.callCount, 5);
assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42');
await element.updateComplete;
assert.isTrue(element.loading);
@@ -790,7 +765,6 @@
});
test('moveToNextCommentThread navigates to next file', async () => {
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
assertIsDefined(element.cursor);
sinon.stub(element.cursor, 'isAtEnd').returns(true);
element.changeNum = 42 as NumericChangeId;
@@ -816,17 +790,15 @@
]);
element.path = 'glados.txt';
element.loggedIn = true;
+ await element.updateComplete;
+ setUrlStub.reset();
pressKey(element, 'N');
await element.updateComplete;
- assert.isTrue(
- diffNavStub.calledWithExactly(
- element.change,
- 'wheatley.md',
- 10 as RevisionPatchSetNum,
- PARENT,
- 21
- )
+ assert.equal(setUrlStub.callCount, 1);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/10/wheatley.md#21'
);
element.path = 'wheatley.md'; // navigated to next file
@@ -834,7 +806,8 @@
pressKey(element, 'N');
await element.updateComplete;
- assert.isTrue(setUrlStub.calledOnce);
+ assert.equal(setUrlStub.callCount, 2);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42');
});
test('shift+x shortcut toggles all diff context', async () => {
@@ -851,11 +824,11 @@
patchNum: 10 as RevisionPatchSetNum,
};
await element.updateComplete;
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element.handleDiffAgainstBase();
- const args = diffNavStub.getCall(0).args;
- assert.equal(args[2], 10 as RevisionPatchSetNum);
- assert.isNotOk(args[3]);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/10/some/path.txt'
+ );
});
test('diff against latest', async () => {
@@ -869,11 +842,11 @@
patchNum: 10 as RevisionPatchSetNum,
};
await element.updateComplete;
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element.handleDiffAgainstLatest();
- const args = diffNavStub.getCall(0).args;
- assert.equal(args[2], 12 as RevisionPatchSetNum);
- assert.equal(args[3], 5 as BasePatchSetNum);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/5..12/foo'
+ );
});
test('handleDiffBaseAgainstLeft', async () => {
@@ -894,13 +867,8 @@
path: 'foo',
};
await element.updateComplete;
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element.handleDiffBaseAgainstLeft();
- assert(diffNavStub.called);
- const args = diffNavStub.getCall(0).args;
- assert.equal(args[2], 1 as RevisionPatchSetNum);
- assert.equal(args[3], PARENT);
- assert.isNotOk(args[4]);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/1/foo');
});
test('handleDiffBaseAgainstLeft when initially navigating to a comment', () => {
@@ -919,13 +887,11 @@
changeNum: 42 as NumericChangeId,
};
element.focusLineNum = 10;
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element.handleDiffBaseAgainstLeft();
- assert(diffNavStub.called);
- const args = diffNavStub.getCall(0).args;
- assert.equal(args[2], 1 as RevisionPatchSetNum);
- assert.equal(args[3], PARENT);
- assert.equal(args[4], 10);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/1/some/path.txt#10'
+ );
});
test('handleDiffRightAgainstLatest', async () => {
@@ -939,12 +905,11 @@
patchNum: 3 as RevisionPatchSetNum,
};
await element.updateComplete;
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element.handleDiffRightAgainstLatest();
- assert(diffNavStub.called);
- const args = diffNavStub.getCall(0).args;
- assert.equal(args[2], 10 as RevisionPatchSetNum);
- assert.equal(args[3], 3 as BasePatchSetNum);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/3..10/foo'
+ );
});
test('handleDiffBaseAgainstLatest', async () => {
@@ -957,12 +922,11 @@
patchNum: 3 as RevisionPatchSetNum,
};
await element.updateComplete;
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element.handleDiffBaseAgainstLatest();
- assert(diffNavStub.called);
- const args = diffNavStub.getCall(0).args;
- assert.equal(args[2], 10 as RevisionPatchSetNum);
- assert.isNotOk(args[3]);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/10/some/path.txt'
+ );
});
test('A fires an error event when not logged in', async () => {
@@ -990,11 +954,15 @@
},
};
element.loggedIn = true;
+ await element.updateComplete;
const loggedInErrorSpy = sinon.spy();
element.addEventListener('show-auth-required', loggedInErrorSpy);
+ setUrlStub.reset();
+
pressKey(element, 'a');
+
await element.updateComplete;
- assert.isTrue(setUrlStub.calledOnce);
+ assert.equal(setUrlStub.callCount, 1);
assert.equal(
setUrlStub.lastCall.firstArg,
'/c/test-project/+/42/5..10?openReplyDialog=true'
@@ -1050,57 +1018,40 @@
]);
element.path = 'glados.txt';
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
-
pressKey(element, 'u');
- assert.isTrue(setUrlStub.calledOnce);
+ assert.equal(setUrlStub.callCount, 1);
assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/5..10');
pressKey(element, ']');
assert.isTrue(element.loading);
- assert(
- diffNavStub.lastCall.calledWithExactly(
- element.change,
- 'wheatley.md',
- 10 as RevisionPatchSetNum,
- 5 as BasePatchSetNum,
- undefined
- ),
- 'Should navigate to /c/42/5..10/wheatley.md'
+ assert.equal(setUrlStub.callCount, 2);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/5..10/wheatley.md'
);
element.path = 'wheatley.md';
pressKey(element, '[');
assert.isTrue(element.loading);
- assert(
- diffNavStub.lastCall.calledWithExactly(
- element.change,
- 'glados.txt',
- 10 as RevisionPatchSetNum,
- 5 as BasePatchSetNum,
- undefined
- ),
- 'Should navigate to /c/42/5..10/glados.txt'
+ assert.equal(setUrlStub.callCount, 3);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/5..10/glados.txt'
);
element.path = 'glados.txt';
pressKey(element, '[');
assert.isTrue(element.loading);
- assert(
- diffNavStub.lastCall.calledWithExactly(
- element.change,
- 'chell.go',
- 10 as RevisionPatchSetNum,
- 5 as BasePatchSetNum,
- undefined
- ),
- 'Should navigate to /c/42/5..10/chell.go'
+ assert.equal(setUrlStub.callCount, 4);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/5..10/chell.go'
);
element.path = 'chell.go';
pressKey(element, '[');
assert.isTrue(element.loading);
- assert.isTrue(setUrlStub.calledTwice);
+ assert.equal(setUrlStub.callCount, 5);
assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/5..10');
assertIsDefined(element.downloadOverlay);
@@ -1132,48 +1083,28 @@
]);
element.path = 'glados.txt';
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
-
pressKey(element, 'u');
assert.isTrue(setUrlStub.calledOnce);
assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/1');
pressKey(element, ']');
- assert(
- diffNavStub.lastCall.calledWithExactly(
- element.change,
- 'wheatley.md',
- 1 as RevisionPatchSetNum,
- PARENT,
- undefined
- ),
- 'Should navigate to /c/42/1/wheatley.md'
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/1/wheatley.md'
);
element.path = 'wheatley.md';
pressKey(element, '[');
- assert(
- diffNavStub.lastCall.calledWithExactly(
- element.change,
- 'glados.txt',
- 1 as RevisionPatchSetNum,
- PARENT,
- undefined
- ),
- 'Should navigate to /c/42/1/glados.txt'
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/1/glados.txt'
);
element.path = 'glados.txt';
pressKey(element, '[');
- assert(
- diffNavStub.lastCall.calledWithExactly(
- element.change,
- 'chell.go',
- 1 as RevisionPatchSetNum,
- PARENT,
- undefined
- ),
- 'Should navigate to /c/42/1/chell.go'
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/1/chell.go'
);
element.path = 'chell.go';
@@ -1200,7 +1131,6 @@
b: createRevision(2),
},
};
- const redirectStub = sinon.stub(GerritNav, 'navigateToRelativeUrl');
await element.updateComplete;
const editBtn = queryAndAssert<GrButton>(
element,
@@ -1208,17 +1138,8 @@
);
assert.isTrue(!!editBtn);
editBtn.click();
- assert.isTrue(redirectStub.called);
- assert.isTrue(
- redirectStub.lastCall.calledWithExactly(
- createEditUrl({
- changeNum: element.change._number,
- project: element.change.project,
- path: element.path,
- patchNum: element.patchRange.patchNum,
- })
- )
- );
+ assert.equal(setUrlStub.callCount, 1);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/gerrit/+/42/1/t.txt,edit');
});
test('edit should redirect to edit page with line number', async () => {
@@ -1243,7 +1164,6 @@
sinon
.stub(element.cursor, 'getAddress')
.returns({number: lineNumber, leftSide: false});
- const redirectStub = sinon.stub(GerritNav, 'navigateToRelativeUrl');
await element.updateComplete;
const editBtn = queryAndAssert<GrButton>(
element,
@@ -1251,17 +1171,10 @@
);
assert.isTrue(!!editBtn);
editBtn.click();
- assert.isTrue(redirectStub.called);
- assert.isTrue(
- redirectStub.lastCall.calledWithExactly(
- createEditUrl({
- changeNum: element.change._number,
- project: element.change.project,
- path: element.path,
- patchNum: element.patchRange.patchNum,
- lineNum: lineNumber,
- })
- )
+ assert.equal(setUrlStub.callCount, 1);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/gerrit/+/42/1/t.txt,edit#42'
);
});
@@ -1582,8 +1495,7 @@
});
});
- test('handlePatchChange calls navigateToDiff correctly', async () => {
- const navigateStub = sinon.stub(GerritNav, 'navigateToDiff');
+ test('handlePatchChange calls setUrl correctly', async () => {
element.change = {
...createParsedChange(),
_number: 321 as NumericChangeId,
@@ -1606,13 +1518,9 @@
new CustomEvent('patch-range-change', {detail, bubbles: false})
);
- assert(
- navigateStub.lastCall.calledWithExactly(
- element.change,
- element.path,
- 1 as RevisionPatchSetNum,
- PARENT
- )
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/foo/bar/+/321/1/path/to/file.txt'
);
});
@@ -2126,11 +2034,9 @@
suite('skip next/previous', () => {
let navToChangeStub: SinonStub;
- let navToDiffStub: SinonStub;
setup(() => {
navToChangeStub = sinon.stub(element, 'navToChangeView');
- navToDiffStub = sinon.stub(GerritNav, 'navigateToDiff');
element.files = getFilesFromFileList([
'path/one.jpg',
'path/two.m4v',
@@ -2146,7 +2052,7 @@
test('no skips', () => {
element.moveToPreviousFileWithComment();
assert.isFalse(navToChangeStub.called);
- assert.isFalse(navToDiffStub.called);
+ assert.isFalse(setUrlStub.called);
});
test('no previous', async () => {
@@ -2160,7 +2066,7 @@
element.moveToPreviousFileWithComment();
assert.isTrue(navToChangeStub.calledOnce);
- assert.isFalse(navToDiffStub.called);
+ assert.isFalse(setUrlStub.called);
});
test('w/ previous', async () => {
@@ -2174,7 +2080,7 @@
element.moveToPreviousFileWithComment();
assert.isFalse(navToChangeStub.called);
- assert.isTrue(navToDiffStub.calledOnce);
+ assert.isTrue(setUrlStub.calledOnce);
});
});
@@ -2182,7 +2088,7 @@
test('no skips', () => {
element.moveToNextFileWithComment();
assert.isFalse(navToChangeStub.called);
- assert.isFalse(navToDiffStub.called);
+ assert.isFalse(setUrlStub.called);
});
test('no previous', async () => {
@@ -2196,7 +2102,7 @@
element.moveToNextFileWithComment();
assert.isTrue(navToChangeStub.calledOnce);
- assert.isFalse(navToDiffStub.called);
+ assert.isFalse(setUrlStub.called);
});
test('w/ previous', async () => {
@@ -2210,7 +2116,7 @@
element.moveToNextFileWithComment();
assert.isFalse(navToChangeStub.called);
- assert.isTrue(navToDiffStub.calledOnce);
+ assert.isTrue(setUrlStub.calledOnce);
});
});
});
@@ -2452,10 +2358,9 @@
assert.deepEqual(navStub.lastCall.args, [['file1', 'file3'], 1]);
});
- test('File change should trigger navigateToDiff once', async () => {
+ test('File change should trigger setUrl once', async () => {
element.files = getFilesFromFileList(['file1', 'file2', 'file3']);
sinon.stub(element, 'initLineOfInterestAndCursor');
- const navigateToDiffStub = sinon.stub(GerritNav, 'navigateToDiff');
// Load file1
element.viewState = {
@@ -2474,13 +2379,13 @@
revisions: createRevisions(1),
};
await element.updateComplete;
- assert.isTrue(navigateToDiffStub.notCalled);
+ assert.isFalse(setUrlStub.called);
// Switch to file2
element.handleFileChange(
new CustomEvent('value-change', {detail: {value: 'file2'}})
);
- assert.isTrue(navigateToDiffStub.calledOnce);
+ assert.isTrue(setUrlStub.calledOnce);
// This is to mock the param change triggered by above navigate
element.viewState = {
@@ -2496,7 +2401,7 @@
};
// No extra call
- assert.isTrue(navigateToDiffStub.calledOnce);
+ assert.isTrue(setUrlStub.calledOnce);
});
test('_computeDownloadDropdownLinks', () => {
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
index 6ef5406..a273a3e 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
@@ -10,7 +10,7 @@
import '../../shared/gr-dropdown/gr-dropdown';
import '../../shared/gr-overlay/gr-overlay';
import {GrEditAction, GrEditConstants} from '../gr-edit-constants';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {ChangeInfo, RevisionPatchSetNum} from '../../../types/common';
import {GrDialog} from '../../shared/gr-dialog/gr-dialog';
import {
@@ -32,6 +32,7 @@
import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
import {IronInputElement} from '@polymer/iron-input/iron-input';
import {createEditUrl} from '../../../models/views/edit';
+import {resolve} from '../../../models/dependency';
@customElement('gr-edit-controls')
export class GrEditControls extends LitElement {
@@ -75,6 +76,8 @@
private readonly restApiService = getAppContext().restApiService;
+ private readonly getNavigation = resolve(this, navigationToken);
+
static override get styles() {
return [
sharedStyles,
@@ -431,7 +434,7 @@
patchNum: this.patchNum,
});
- GerritNav.navigateToRelativeUrl(url);
+ this.getNavigation().setUrl(url);
this.closeDialog(this.getDialogFromEvent(e));
};
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
index ee83e80..b4469db 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
@@ -6,7 +6,7 @@
import '../../../test/common-test-setup';
import './gr-edit-controls';
import {GrEditControls} from './gr-edit-controls';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {queryAll, stubRestApi, waitUntil} from '../../../test/test-utils';
import {createChange, createRevision} from '../../../test/test-data-generators';
import {GrAutocomplete} from '../../shared/gr-autocomplete/gr-autocomplete';
@@ -22,6 +22,7 @@
import {GrButton} from '../../shared/gr-button/gr-button';
import '../../shared/gr-dialog/gr-dialog';
import {waitForEventOnce} from '../../../utils/event-util';
+import {testResolver} from '../../../test/common-test-setup';
suite('gr-edit-controls tests', () => {
let element: GrEditControls;
@@ -194,11 +195,11 @@
});
suite('edit button CUJ', () => {
- let navStub: sinon.SinonStub;
+ let setUrlStub: sinon.SinonStub;
let openAutoComplete: GrAutocomplete;
setup(() => {
- navStub = sinon.stub(GerritNav, 'navigateToRelativeUrl');
+ setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
openAutoComplete = queryAndAssert<GrAutocomplete>(
element.openDialog,
'gr-autocomplete'
@@ -234,7 +235,7 @@
'gr-button[primary]'
).click();
- assert.isTrue(navStub.called);
+ assert.isTrue(setUrlStub.called);
assert.isTrue(closeDialogSpy.called);
});
@@ -247,7 +248,7 @@
await element.updateComplete;
await waitUntil(() => !element.openDialog!.disabled);
queryAndAssert<GrButton>(element.openDialog, 'gr-button').click();
- assert.isFalse(navStub.called);
+ assert.isFalse(setUrlStub.called);
await waitUntil(() => closeDialogSpy.called);
assert.equal(element.path, '');
});
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
index d6a4d94..8b651f1 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
@@ -370,10 +370,7 @@
content = this.content || '';
}
- // TODO(wyatta) switch linkify sequence, see issue 5526.
- this.newContent = this.removeZeroWidthSpace
- ? content.replace(/^R=\u200B/gm, 'R=')
- : content;
+ this.newContent = content;
}
computeSaveDisabled(): boolean {
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.ts b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.ts
index 6389955..fec347c6 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.ts
@@ -140,25 +140,6 @@
assert.equal(element.newContent, 'stale content');
});
- test('zero width spaces are removed properly', async () => {
- element.removeZeroWidthSpace = true;
- element.content = 'R=\u200Btest@google.com';
-
- // Needed because contentChanged resets newContent
- // We want contentChanged observer to finish before editingChanged is
- // called
-
- await element.updateComplete;
-
- element.editing = true;
-
- // editingChanged updates newContent so wait for it's observer
- // to finish
- await element.updateComplete;
-
- assert.equal(element.newContent, 'R=test@google.com');
- });
-
suite('editing', () => {
setup(async () => {
element.content = 'current content';
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
index 23b4e81..11e7152 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
@@ -3,7 +3,6 @@
* Copyright 2016 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import '../gr-linked-text/gr-linked-text';
import '../gr-markdown/gr-markdown';
import {CommentLinks} from '../../../types/common';
import {LitElement, css, html, TemplateResult} from 'lit';
@@ -91,7 +90,7 @@
ul,
code,
blockquote,
- gr-linked-text.pre {
+ gr-markdown.pre {
margin: 0 0 var(--spacing-m) 0;
}
p,
@@ -103,7 +102,7 @@
:host([noTrailingMargin]) p:last-child,
:host([noTrailingMargin]) ul:last-child,
:host([noTrailingMargin]) blockquote:last-child,
- :host([noTrailingMargin]) gr-linked-text.pre:last-child {
+ :host([noTrailingMargin]) gr-markdown.pre:last-child {
margin: 0;
}
blockquote {
@@ -142,7 +141,10 @@
if (!this.content) return;
if (this.flagsService.isEnabled(KnownExperimentId.RENDER_MARKDOWN)) {
- return html`<gr-markdown .markdown=${this.content}></gr-markdown>`;
+ return html`<gr-markdown
+ .markdown=${true}
+ .content=${this.content}
+ ></gr-markdown>`;
} else {
const blocks = this._computeBlocks(this.content);
return html`${blocks.map(block => this.renderBlock(block))}`;
@@ -354,14 +356,7 @@
}
private renderInlineText(content: string): TemplateResult {
- return html`
- <gr-linked-text
- .config=${this.config}
- content=${content}
- pre
- inline
- ></gr-linked-text>
- `;
+ return html`<gr-markdown .content=${content}></gr-markdown>`;
}
private renderLink(text: string, url: string): TemplateResult {
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
index 191886e..62cb7c8 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
@@ -103,9 +103,7 @@
element,
/* HTML */ `
<p>
- <gr-linked-text content="text " inline="" pre="">
- <span id="output" slot="insert"> text </span>
- </gr-linked-text>
+ <gr-markdown></gr-markdown>
<span class="inline-code"> code </span>
</p>
`
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.ts
index 655c538..4900ed5 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.ts
@@ -21,7 +21,6 @@
EventCallback,
EventEmitterService,
} from '../../../services/gr-event-interface/gr-event-interface';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {Gerrit} from '../../../api/gerrit';
import {fontStyles} from '../../../styles/gr-font-styles';
import {formStyles} from '../../../styles/gr-form-styles';
@@ -67,7 +66,6 @@
_customStyleSheet?: CSSStyleSheet;
// exposed methods
- Nav: typeof GerritNav;
Auth: AuthService;
}
@@ -112,8 +110,6 @@
class GerritImpl implements GerritInternal {
_customStyleSheet?: CSSStyleSheet;
- public readonly Nav = GerritNav;
-
public readonly Auth: AuthService;
private readonly reportingService: ReportingService;
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.ts b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.ts
deleted file mode 100644
index 16a60e7..0000000
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.ts
+++ /dev/null
@@ -1,178 +0,0 @@
-/**
- * @license
- * Copyright 2015 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import '../../../styles/shared-styles';
-import {GrLinkTextParser, LinkTextParserConfig} from './link-text-parser';
-import {LitElement, css, html, PropertyValues} from 'lit';
-import {customElement, property} from 'lit/decorators.js';
-import {assertIsDefined} from '../../../utils/common-util';
-
-declare global {
- interface HTMLElementTagNameMap {
- 'gr-linked-text': GrLinkedText;
- }
-}
-
-@customElement('gr-linked-text')
-export class GrLinkedText extends LitElement {
- private outputElement?: HTMLSpanElement;
-
- @property({type: Boolean, attribute: 'remove-zero-width-space'})
- removeZeroWidthSpace?: boolean;
-
- @property({type: String})
- content = '';
-
- @property({type: Boolean, attribute: true})
- pre = false;
-
- @property({type: Boolean, attribute: true})
- disabled = false;
-
- @property({type: Boolean, attribute: true})
- inline = false;
-
- @property({type: Object})
- config?: LinkTextParserConfig;
-
- static override get styles() {
- return css`
- :host {
- display: block;
- }
- :host([inline]) {
- display: inline;
- }
- :host([pre]) ::slotted(span) {
- white-space: var(--linked-text-white-space, pre-wrap);
- word-wrap: var(--linked-text-word-wrap, break-word);
- }
- `;
- }
-
- override render() {
- return html`<slot name="insert"></slot>`;
- }
-
- // NOTE: LinkTextParser dynamically creates HTML fragments based on backend
- // configuration commentLinks. These commentLinks can contain arbitrary HTML
- // fragments. This means that arbitrary HTML needs to be injected into the
- // DOM-tree, where this HTML is is controlled on the server-side in the
- // server-configuration rather than by arbitrary users.
- // To enable this injection of 'unsafe' HTML, LinkTextParser generates
- // HTML fragments. Lit does not support inserting html fragments directly
- // into its DOM-tree as it controls the DOM-tree that it generates.
- // Therefore, to get around this we create a single element that we slot into
- // the Lit-owned DOM. This element will not be part of this LitElement as
- // it's slotted in and thus can be modified on the fly by handleParseResult.
- override firstUpdated(_changedProperties: PropertyValues): void {
- this.outputElement = document.createElement('span');
- this.outputElement.id = 'output';
- this.outputElement.slot = 'insert';
- this.append(this.outputElement);
- }
-
- override updated(changedProperties: PropertyValues): void {
- if (changedProperties.has('content') || changedProperties.has('config')) {
- this._contentOrConfigChanged();
- } else if (changedProperties.has('disabled')) {
- this.styleLinks();
- }
- }
-
- /**
- * Because either the source text or the linkification config has changed,
- * the content should be re-parsed.
- * Private but used in tests.
- *
- * @param content The raw, un-linkified source string to parse.
- * @param config The server config specifying commentLink patterns
- */
- _contentOrConfigChanged() {
- if (!this.config) {
- assertIsDefined(this.outputElement);
- this.outputElement.textContent = this.content;
- return;
- }
-
- assertIsDefined(this.outputElement);
- this.outputElement.textContent = '';
- const parser = new GrLinkTextParser(
- this.config,
- (text: string | null, href: string | null, fragment?: DocumentFragment) =>
- this.handleParseResult(text, href, fragment),
- this.removeZeroWidthSpace
- );
- parser.parse(this.content);
-
- // Ensure that external links originating from HTML commentlink configs
- // open in a new tab. @see Issue 5567
- // Ensure links to the same host originating from commentlink configs
- // open in the same tab. When target is not set - default is _self
- // @see Issue 4616
- this.outputElement.querySelectorAll('a').forEach(anchor => {
- if (anchor.hostname === window.location.hostname) {
- anchor.removeAttribute('target');
- } else {
- anchor.setAttribute('target', '_blank');
- }
- anchor.setAttribute('rel', 'noopener');
- });
-
- this.styleLinks();
- }
-
- /**
- * Styles the links based on whether gr-linked-text is disabled or not
- */
- private styleLinks() {
- assertIsDefined(this.outputElement);
- this.outputElement.querySelectorAll('a').forEach(anchor => {
- anchor.setAttribute('style', this.computeLinkStyle());
- });
- }
-
- private computeLinkStyle() {
- if (this.disabled) {
- return `
- color: inherit;
- text-decoration: none;
- pointer-events: none;
- `;
- } else {
- return 'color: var(--link-color)';
- }
- }
-
- /**
- * This method is called when the GrLikTextParser emits a partial result
- * (used as the "callback" parameter). It will be called in either of two
- * ways:
- * - To create a link: when called with `text` and `href` arguments, a link
- * element should be created and attached to the resulting DOM.
- * - To attach an arbitrary fragment: when called with only the `fragment`
- * argument, the fragment should be attached to the resulting DOM as is.
- */
- private handleParseResult(
- text: string | null,
- href: string | null,
- fragment?: DocumentFragment
- ) {
- assertIsDefined(this.outputElement);
- const output = this.outputElement;
- if (href) {
- const a = document.createElement('a');
- a.setAttribute('href', href);
- // GrLinkTextParser either pass text and href together or
- // only DocumentFragment - see LinkTextParserCallback
- a.textContent = text!;
- a.target = '_blank';
- a.setAttribute('rel', 'noopener');
- output.appendChild(a);
- } else if (fragment) {
- output.appendChild(fragment);
- }
- }
-}
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.ts b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.ts
deleted file mode 100644
index 00e0313..0000000
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.ts
+++ /dev/null
@@ -1,471 +0,0 @@
-/**
- * @license
- * Copyright 2015 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import '../../../test/common-test-setup';
-import './gr-linked-text';
-import {fixture, html, assert} from '@open-wc/testing';
-import {GrLinkedText} from './gr-linked-text';
-import {queryAndAssert} from '../../../test/test-utils';
-
-suite('gr-linked-text tests', () => {
- let element: GrLinkedText;
-
- let originalCanonicalPath: string | undefined;
-
- setup(async () => {
- originalCanonicalPath = window.CANONICAL_PATH;
- element = await fixture<GrLinkedText>(html`
- <gr-linked-text>
- <div id="output"></div>
- </gr-linked-text>
- `);
-
- element.config = {
- ph: {
- match: '([Bb]ug|[Ii]ssue)\\s*#?(\\d+)',
- link: 'https://bugs.chromium.org/p/gerrit/issues/detail?id=$2',
- },
- prefixsameinlinkandpattern: {
- match: '([Hh][Tt][Tt][Pp]example)\\s*#?(\\d+)',
- link: 'https://bugs.chromium.org/p/gerrit/issues/detail?id=$2',
- },
- changeid: {
- match: '(I[0-9a-f]{8,40})',
- link: '#/q/$1',
- },
- changeid2: {
- match: 'Change-Id: +(I[0-9a-f]{8,40})',
- link: '#/q/$1',
- },
- googlesearch: {
- match: 'google:(.+)',
- link: 'https://bing.com/search?q=$1', // html should supersede link.
- html: '<a href="https://google.com/search?q=$1">$1</a>',
- },
- hashedhtml: {
- match: 'hash:(.+)',
- html: '<a href="#/awesomesauce">$1</a>',
- },
- baseurl: {
- match: 'test (.+)',
- html: '<a href="/r/awesomesauce">$1</a>',
- },
- anotatstartwithbaseurl: {
- match: 'a test (.+)',
- html: '[Lookup: <a href="/r/awesomesauce">$1</a>]',
- },
- disabledconfig: {
- match: 'foo:(.+)',
- link: 'https://google.com/search?q=$1',
- enabled: false,
- },
- };
- });
-
- teardown(() => {
- window.CANONICAL_PATH = originalCanonicalPath;
- });
-
- test('render', async () => {
- element.content =
- 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
- await element.updateComplete;
- assert.lightDom.equal(
- element,
- /* HTML */ `
- <div id="output"></div>
- <span id="output" slot="insert">
- <a
- href="https://bugs.chromium.org/p/gerrit/issues/detail?id=3650"
- rel="noopener"
- style="color: var(--link-color)"
- target="_blank"
- >
- https://bugs.chromium.org/p/gerrit/issues/detail?id=3650
- </a>
- </span>
- `
- );
- });
-
- test('URL pattern was parsed and linked.', async () => {
- // Regular inline link.
- const url = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
- element.content = url;
- await element.updateComplete;
-
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- assert.equal(linkEl.target, '_blank');
- assert.equal(linkEl.rel, 'noopener');
- assert.equal(linkEl.href, url);
- assert.equal(linkEl.textContent, url);
- });
-
- test('Bug pattern was parsed and linked', async () => {
- // "Issue/Bug" pattern.
- element.content = 'Issue 3650';
- await element.updateComplete;
-
- let linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- const url = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
- assert.equal(linkEl.target, '_blank');
- assert.equal(linkEl.href, url);
- assert.equal(linkEl.textContent, 'Issue 3650');
-
- element.content = 'Bug 3650';
- await element.updateComplete;
-
- linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- assert.equal(linkEl.target, '_blank');
- assert.equal(linkEl.rel, 'noopener');
- assert.equal(linkEl.href, url);
- assert.equal(linkEl.textContent, 'Bug 3650');
- });
-
- test('Pattern with same prefix as link was correctly parsed', async () => {
- // Pattern starts with the same prefix (`http`) as the url.
- element.content = 'httpexample 3650';
- await element.updateComplete;
-
- assert.equal(queryAndAssert(element, 'span#output').childNodes.length, 1);
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- const url = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
- assert.equal(linkEl.target, '_blank');
- assert.equal(linkEl.href, url);
- assert.equal(linkEl.textContent, 'httpexample 3650');
- });
-
- test('Change-Id pattern was parsed and linked', async () => {
- // "Change-Id:" pattern.
- const changeID = 'I11d6a37f5e9b5df0486f6c922d8836dfa780e03e';
- const prefix = 'Change-Id: ';
- element.content = prefix + changeID;
- await element.updateComplete;
-
- const textNode = queryAndAssert(element, 'span#output').childNodes[0];
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[1] as HTMLAnchorElement;
- assert.equal(textNode.textContent, prefix);
- const url = '/q/' + changeID;
- assert.isFalse(linkEl.hasAttribute('target'));
- // Since url is a path, the host is added automatically.
- assert.isTrue(linkEl.href.endsWith(url));
- assert.equal(linkEl.textContent, changeID);
- });
-
- test('Change-Id pattern was parsed and linked with base url', async () => {
- window.CANONICAL_PATH = '/r';
-
- // "Change-Id:" pattern.
- const changeID = 'I11d6a37f5e9b5df0486f6c922d8836dfa780e03e';
- const prefix = 'Change-Id: ';
- element.content = prefix + changeID;
- await element.updateComplete;
-
- const textNode = queryAndAssert(element, 'span#output').childNodes[0];
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[1] as HTMLAnchorElement;
- assert.equal(textNode.textContent, prefix);
- const url = '/r/q/' + changeID;
- assert.isFalse(linkEl.hasAttribute('target'));
- // Since url is a path, the host is added automatically.
- assert.isTrue(linkEl.href.endsWith(url));
- assert.equal(linkEl.textContent, changeID);
- });
-
- test('Multiple matches', async () => {
- element.content = 'Issue 3650\nIssue 3450';
- await element.updateComplete;
-
- const linkEl1 = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- const linkEl2 = queryAndAssert(element, 'span#output')
- .childNodes[2] as HTMLAnchorElement;
-
- assert.equal(linkEl1.target, '_blank');
- assert.equal(
- linkEl1.href,
- 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650'
- );
- assert.equal(linkEl1.textContent, 'Issue 3650');
-
- assert.equal(linkEl2.target, '_blank');
- assert.equal(
- linkEl2.href,
- 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3450'
- );
- assert.equal(linkEl2.textContent, 'Issue 3450');
- });
-
- test('Change-Id pattern parsed before bug pattern', async () => {
- // "Change-Id:" pattern.
- const changeID = 'I11d6a37f5e9b5df0486f6c922d8836dfa780e03e';
- const prefix = 'Change-Id: ';
-
- // "Issue/Bug" pattern.
- const bug = 'Issue 3650';
-
- const changeUrl = '/q/' + changeID;
- const bugUrl = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
-
- element.content = prefix + changeID + bug;
- await element.updateComplete;
-
- const textNode = queryAndAssert(element, 'span#output').childNodes[0];
- const changeLinkEl = queryAndAssert(element, 'span#output')
- .childNodes[1] as HTMLAnchorElement;
- const bugLinkEl = queryAndAssert(element, 'span#output')
- .childNodes[2] as HTMLAnchorElement;
-
- assert.equal(textNode.textContent, prefix);
-
- assert.isFalse(changeLinkEl.hasAttribute('target'));
- assert.isTrue(changeLinkEl.href.endsWith(changeUrl));
- assert.equal(changeLinkEl.textContent, changeID);
-
- assert.equal(bugLinkEl.target, '_blank');
- assert.equal(bugLinkEl.href, bugUrl);
- assert.equal(bugLinkEl.textContent, 'Issue 3650');
- });
-
- test('html field in link config', async () => {
- element.content = 'google:do a barrel roll';
- await element.updateComplete;
-
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- assert.equal(
- linkEl.getAttribute('href'),
- 'https://google.com/search?q=do a barrel roll'
- );
- assert.equal(linkEl.textContent, 'do a barrel roll');
- });
-
- test('removing hash from links', async () => {
- element.content = 'hash:foo';
- await element.updateComplete;
-
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- assert.isTrue(linkEl.href.endsWith('/awesomesauce'));
- assert.equal(linkEl.textContent, 'foo');
- });
-
- test('html with base url', async () => {
- window.CANONICAL_PATH = '/r';
-
- element.content = 'test foo';
- await element.updateComplete;
-
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- assert.isTrue(linkEl.href.endsWith('/r/awesomesauce'));
- assert.equal(linkEl.textContent, 'foo');
- });
-
- test('a is not at start', async () => {
- window.CANONICAL_PATH = '/r';
-
- element.content = 'a test foo';
- await element.updateComplete;
-
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[1] as HTMLAnchorElement;
- assert.isTrue(linkEl.href.endsWith('/r/awesomesauce'));
- assert.equal(linkEl.textContent, 'foo');
- });
-
- test('hash html with base url', async () => {
- window.CANONICAL_PATH = '/r';
-
- element.content = 'hash:foo';
- await element.updateComplete;
-
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- assert.isTrue(linkEl.href.endsWith('/r/awesomesauce'));
- assert.equal(linkEl.textContent, 'foo');
- });
-
- test('disabled config', async () => {
- element.content = 'foo:baz';
- await element.updateComplete;
-
- assert.equal(queryAndAssert(element, 'span#output').innerHTML, 'foo:baz');
- });
-
- test('R=email labels link correctly', async () => {
- element.removeZeroWidthSpace = true;
- element.content = 'R=\u200Btest@google.com';
- await element.updateComplete;
-
- assert.equal(
- queryAndAssert(element, 'span#output').textContent,
- 'R=test@google.com'
- );
- assert.equal(
- queryAndAssert(element, 'span#output').innerHTML.match(/(R=<a)/g)!.length,
- 1
- );
- });
-
- test('CC=email labels link correctly', async () => {
- element.removeZeroWidthSpace = true;
- element.content = 'CC=\u200Btest@google.com';
- await element.updateComplete;
-
- assert.equal(
- queryAndAssert(element, 'span#output').textContent,
- 'CC=test@google.com'
- );
- assert.equal(
- queryAndAssert(element, 'span#output').innerHTML.match(/(CC=<a)/g)!
- .length,
- 1
- );
- });
-
- test('only {http,https,mailto} protocols are linkified', async () => {
- element.content = 'xx mailto:test@google.com yy';
- await element.updateComplete;
-
- let links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'mailto:test@google.com');
- assert.equal(links[0].innerHTML, 'mailto:test@google.com');
-
- element.content = 'xx http://google.com yy';
- await element.updateComplete;
-
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'http://google.com');
- assert.equal(links[0].innerHTML, 'http://google.com');
-
- element.content = 'xx https://google.com yy';
- await element.updateComplete;
-
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'https://google.com');
- assert.equal(links[0].innerHTML, 'https://google.com');
-
- element.content = 'xx ssh://google.com yy';
- await element.updateComplete;
-
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 0);
-
- element.content = 'xx ftp://google.com yy';
- await element.updateComplete;
-
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 0);
- });
-
- test('links without leading whitespace are linkified', async () => {
- element.content = 'xx abcmailto:test@google.com yy';
- await element.updateComplete;
-
- assert.equal(
- queryAndAssert(element, 'span#output').innerHTML.substr(0, 6),
- 'xx abc'
- );
- let links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'mailto:test@google.com');
- assert.equal(links[0].innerHTML, 'mailto:test@google.com');
-
- element.content = 'xx defhttp://google.com yy';
- await element.updateComplete;
-
- assert.equal(
- queryAndAssert(element, 'span#output').innerHTML.substr(0, 6),
- 'xx def'
- );
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'http://google.com');
- assert.equal(links[0].innerHTML, 'http://google.com');
-
- element.content = 'xx qwehttps://google.com yy';
- await element.updateComplete;
-
- assert.equal(
- queryAndAssert(element, 'span#output').innerHTML.substr(0, 6),
- 'xx qwe'
- );
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'https://google.com');
- assert.equal(links[0].innerHTML, 'https://google.com');
-
- // Non-latin character
- element.content = 'xx абвhttps://google.com yy';
- await element.updateComplete;
-
- assert.equal(
- queryAndAssert(element, 'span#output').innerHTML.substr(0, 6),
- 'xx абв'
- );
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'https://google.com');
- assert.equal(links[0].innerHTML, 'https://google.com');
-
- element.content = 'xx ssh://google.com yy';
- await element.updateComplete;
-
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 0);
-
- element.content = 'xx ftp://google.com yy';
- await element.updateComplete;
-
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 0);
- });
-
- test('overlapping links', async () => {
- element.config = {
- b1: {
- match: '(B:\\s*)(\\d+)',
- html: '$1<a href="ftp://foo/$2">$2</a>',
- },
- b2: {
- match: '(B:\\s*\\d+\\s*,\\s*)(\\d+)',
- html: '$1<a href="ftp://foo/$2">$2</a>',
- },
- };
- element.content = '- B: 123, 45';
- await element.updateComplete;
-
- const links = element.querySelectorAll('a');
-
- assert.equal(links.length, 2);
- assert.equal(
- queryAndAssert<HTMLSpanElement>(element, 'span').textContent,
- '- B: 123, 45'
- );
-
- assert.equal(links[0].href, 'ftp://foo/123');
- assert.equal(links[0].textContent, '123');
-
- assert.equal(links[1].href, 'ftp://foo/45');
- assert.equal(links[1].textContent, '45');
- });
-
- test('_contentOrConfigChanged called with config', async () => {
- const contentConfigStub = sinon.stub(element, '_contentOrConfigChanged');
- element.content = 'some text';
- await element.updateComplete;
-
- assert.isTrue(contentConfigStub.called);
- });
-});
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.ts b/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.ts
deleted file mode 100644
index 73cf58b..0000000
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.ts
+++ /dev/null
@@ -1,415 +0,0 @@
-/**
- * @license
- * Copyright 2015 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import 'ba-linkify/ba-linkify';
-import {getBaseUrl} from '../../../utils/url-util';
-import {CommentLinkInfo} from '../../../types/common';
-
-/**
- * Pattern describing URLs with supported protocols.
- */
-const URL_PROTOCOL_PATTERN = /^(.*)(https?:\/\/|mailto:)/;
-
-export type LinkTextParserCallback = ((text: string, href: string) => void) &
- ((text: null, href: null, fragment: DocumentFragment) => void);
-
-export interface CommentLinkItem {
- position: number;
- length: number;
- html: HTMLAnchorElement | DocumentFragment;
-}
-
-export type LinkTextParserConfig = {[name: string]: CommentLinkInfo};
-
-export class GrLinkTextParser {
- private readonly baseUrl = getBaseUrl();
-
- /**
- * Construct a parser for linkifying text. Will linkify plain URLs that appear
- * in the text as well as custom links if any are specified in the linkConfig
- * parameter.
- *
- * @param linkConfig Comment links as specified by the commentlinks field on a
- * project config.
- * @param callback The callback to be fired when an intermediate parse result
- * is emitted. The callback is passed text and href strings if a link is to
- * be created, or a document fragment otherwise.
- * @param removeZeroWidthSpace If true, zero-width spaces will be removed from
- * R=<email> and CC=<email> expressions.
- */
- constructor(
- private readonly linkConfig: LinkTextParserConfig,
- private readonly callback: LinkTextParserCallback,
- private readonly removeZeroWidthSpace?: boolean
- ) {
- Object.preventExtensions(this);
- }
-
- /**
- * Emit a callback to create a link element.
- *
- * @param text The text of the link.
- * @param href The URL to use as the href of the link.
- */
- addText(text: string, href: string) {
- if (!text) {
- return;
- }
- this.callback(text, href);
- }
-
- /**
- * Given the source text and a list of CommentLinkItem objects that were
- * generated by the commentlinks config, emit parsing callbacks.
- *
- * @param text The chuml of source text over which the outputArray items range.
- * @param outputArray The list of items to add resulting from commentlink
- * matches.
- */
- processLinks(text: string, outputArray: CommentLinkItem[]) {
- this.sortArrayReverse(outputArray);
- const fragment = document.createDocumentFragment();
- let cursor = text.length;
-
- // Start inserting linkified URLs from the end of the String. That way, the
- // string positions of the items don't change as we iterate through.
- outputArray.forEach(item => {
- // Add any text between the current linkified item and the item added
- // before if it exists.
- if (item.position + item.length !== cursor) {
- fragment.insertBefore(
- document.createTextNode(
- text.slice(item.position + item.length, cursor)
- ),
- fragment.firstChild
- );
- }
- fragment.insertBefore(item.html, fragment.firstChild);
- cursor = item.position;
- });
-
- // Add the beginning portion at the end.
- if (cursor !== 0) {
- fragment.insertBefore(
- document.createTextNode(text.slice(0, cursor)),
- fragment.firstChild
- );
- }
-
- this.callback(null, null, fragment);
- }
-
- /**
- * Sort the given array of CommentLinkItems such that the positions are in
- * reverse order.
- */
- sortArrayReverse(outputArray: CommentLinkItem[]) {
- outputArray.sort((a, b) => b.position - a.position);
- }
-
- addItem(
- text: string,
- href: string,
- html: null,
- position: number,
- length: number,
- outputArray: CommentLinkItem[]
- ): void;
-
- addItem(
- text: null,
- href: null,
- html: string,
- position: number,
- length: number,
- outputArray: CommentLinkItem[]
- ): void;
-
- /**
- * Create a CommentLinkItem and append it to the given output array. This
- * method can be called in either of two ways:
- * - With `text` and `href` parameters provided, and the `html` parameter
- * passed as `null`. In this case, the new CommentLinkItem will be a link
- * element with the given text and href value.
- * - With the `html` paremeter provided, and the `text` and `href` parameters
- * passed as `null`. In this case, the string of HTML will be parsed and the
- * first resulting node will be used as the resulting content.
- *
- * @param text The text to use if creating a link.
- * @param href The href to use as the URL if creating a link.
- * @param html The html to parse and use as the result.
- * @param position The position inside the source text where the item
- * starts.
- * @param length The number of characters in the source text
- * represented by the item.
- * @param outputArray The array to which the
- * new item is to be appended.
- */
- addItem(
- text: string | null,
- href: string | null,
- html: string | null,
- position: number,
- length: number,
- outputArray: CommentLinkItem[]
- ): void {
- if (href) {
- const a = document.createElement('a');
- a.setAttribute('href', href);
- a.textContent = text;
- a.target = '_blank';
- a.rel = 'noopener';
- outputArray.push({
- html: a,
- position,
- length,
- });
- } else if (html) {
- // addItem has 2 overloads. If href is null, then html
- // can't be null.
- // TODO(TS): remove if(html) and keep else block without condition
- const fragment = document.createDocumentFragment();
- // Create temporary div to hold the nodes in.
- const div = document.createElement('div');
- div.innerHTML = html;
- while (div.firstChild) {
- fragment.appendChild(div.firstChild);
- }
- outputArray.push({
- html: fragment,
- position,
- length,
- });
- }
- }
-
- /**
- * Create a CommentLinkItem for a link and append it to the given output
- * array.
- *
- * @param text The text for the link.
- * @param href The href to use as the URL of the link.
- * @param position The position inside the source text where the link
- * starts.
- * @param length The number of characters in the source text
- * represented by the link.
- * @param outputArray The array to which the
- * new item is to be appended.
- */
- addLink(
- text: string,
- href: string,
- position: number,
- length: number,
- outputArray: CommentLinkItem[]
- ) {
- // TODO(TS): remove !test condition
- if (!text || this.hasOverlap(position, length, outputArray)) {
- return;
- }
- if (
- !!this.baseUrl &&
- href.startsWith('/') &&
- !href.startsWith(this.baseUrl)
- ) {
- href = this.baseUrl + href;
- }
- this.addItem(text, href, null, position, length, outputArray);
- }
-
- /**
- * Create a CommentLinkItem specified by an HTMl string and append it to the
- * given output array.
- *
- * @param html The html to parse and use as the result.
- * @param position The position inside the source text where the item
- * starts.
- * @param length The number of characters in the source text
- * represented by the item.
- * @param outputArray The array to which the
- * new item is to be appended.
- */
- addHTML(
- html: string,
- position: number,
- length: number,
- outputArray: CommentLinkItem[]
- ) {
- if (this.hasOverlap(position, length, outputArray)) {
- return;
- }
- if (
- !!this.baseUrl &&
- html.match(/<a href="\//g) &&
- !new RegExp(`<a href="${this.baseUrl}`, 'g').test(html)
- ) {
- html = html.replace(/<a href="\//g, `<a href="${this.baseUrl}/`);
- }
- this.addItem(null, null, html, position, length, outputArray);
- }
-
- /**
- * Does the given range overlap with anything already in the item list.
- */
- hasOverlap(position: number, length: number, outputArray: CommentLinkItem[]) {
- const endPosition = position + length;
- for (let i = 0; i < outputArray.length; i++) {
- const arrayItemStart = outputArray[i].position;
- const arrayItemEnd = outputArray[i].position + outputArray[i].length;
- if (
- (position >= arrayItemStart && position < arrayItemEnd) ||
- (endPosition > arrayItemStart && endPosition <= arrayItemEnd) ||
- (position === arrayItemStart && position === arrayItemEnd)
- ) {
- return true;
- }
- }
- return false;
- }
-
- /**
- * Parse the given source text and emit callbacks for the items that are
- * parsed.
- */
- parse(text?: string | null) {
- if (text) {
- window.linkify(text, {
- callback: (text: string, href?: string) => this.parseChunk(text, href),
- });
- }
- }
-
- /**
- * Callback that is pased into the linkify function. ba-linkify will call this
- * method in either of two ways:
- * - With both a `text` and `href` parameter provided: this indicates that
- * ba-linkify has found a plain URL and wants it linkified.
- * - With only a `text` parameter provided: this represents the non-link
- * content that lies between the links the library has found.
- *
- */
- parseChunk(text: string, href?: string) {
- // TODO(wyatta) switch linkify sequence, see issue 5526.
- if (this.removeZeroWidthSpace) {
- // Remove the zero-width space added in gr-change-view.
- text = text.replace(/^(CC|R)=\u200B/gm, '$1=');
- }
-
- // If the href is provided then ba-linkify has recognized it as a URL. If
- // the source text does not include a protocol, the protocol will be added
- // by ba-linkify. Create the link if the href is provided and its protocol
- // matches the expected pattern.
- if (href) {
- const result = URL_PROTOCOL_PATTERN.exec(href);
- if (result) {
- const prefixText = result[1];
- if (prefixText.length > 0) {
- // Fix for simple cases from
- // https://bugs.chromium.org/p/gerrit/issues/detail?id=11697
- // When leading whitespace is missed before link,
- // linkify add this text before link as a schema name to href.
- // We suppose, that prefixText just a single word
- // before link and add this word as is, without processing
- // any patterns in it.
- this.parseLinks(prefixText, {});
- text = text.substring(prefixText.length);
- href = href.substring(prefixText.length);
- }
- this.addText(text, href);
- return;
- }
- }
- // For the sections of text that lie between the links found by
- // ba-linkify, we search for the project-config-specified link patterns.
- this.parseLinks(text, this.linkConfig);
- }
-
- /**
- * Walk over the given source text to find matches for comemntlink patterns
- * and emit parse result callbacks.
- *
- * @param text The raw source text.
- * @param config A comment links specification object.
- */
- parseLinks(text: string, config: LinkTextParserConfig) {
- // The outputArray is used to store all of the matches found for all
- // patterns.
- const outputArray: CommentLinkItem[] = [];
- for (const [configName, linkInfo] of Object.entries(config)) {
- // TODO(TS): it seems, the following line can be rewritten as:
- // if(enabled === false || enabled === 0 || enabled === '')
- // Should be double-checked before update
- // eslint-disable-next-line eqeqeq
- if (linkInfo.enabled != null && linkInfo.enabled == false) {
- continue;
- }
- // PolyGerrit doesn't use hash-based navigation like the GWT UI.
- // Account for this.
- const html = linkInfo.html;
- const link = linkInfo.link;
- if (html) {
- linkInfo.html = html.replace(/<a href="#\//g, '<a href="/');
- } else if (link) {
- if (link[0] === '#') {
- linkInfo.link = link.substr(1);
- }
- }
-
- const pattern = new RegExp(linkInfo.match, 'g');
-
- let match;
- let textToCheck = text;
- let susbtrIndex = 0;
-
- while ((match = pattern.exec(textToCheck))) {
- textToCheck = textToCheck.substr(match.index + match[0].length);
- let result = match[0].replace(
- pattern,
- // Either html or link has a value. Otherwise an exception is thrown
- // in the code below.
- (linkInfo.html || linkInfo.link)!
- );
-
- if (linkInfo.html) {
- let i;
- // Skip portion of replacement string that is equal to original to
- // allow overlapping patterns.
- for (i = 0; i < result.length; i++) {
- if (result[i] !== match[0][i]) {
- break;
- }
- }
- result = result.slice(i);
-
- this.addHTML(
- result,
- susbtrIndex + match.index + i,
- match[0].length - i,
- outputArray
- );
- } else if (linkInfo.link) {
- this.addLink(
- match[0],
- result,
- susbtrIndex + match.index,
- match[0].length,
- outputArray
- );
- } else {
- throw Error(
- 'linkconfig entry ' +
- configName +
- ' doesn’t contain a link or html attribute.'
- );
- }
-
- // Update the substring location so we know where we are in relation to
- // the initial full text string.
- susbtrIndex = susbtrIndex + match.index + match[0].length;
- }
- }
- this.processLinks(text, outputArray);
- }
-}
diff --git a/polygerrit-ui/app/elements/shared/gr-markdown/gr-markdown.ts b/polygerrit-ui/app/elements/shared/gr-markdown/gr-markdown.ts
index ae4b004..50f0602 100644
--- a/polygerrit-ui/app/elements/shared/gr-markdown/gr-markdown.ts
+++ b/polygerrit-ui/app/elements/shared/gr-markdown/gr-markdown.ts
@@ -5,7 +5,11 @@
*/
import {css, html, LitElement} from 'lit';
import {customElement, property, state} from 'lit/decorators.js';
-import {htmlEscape} from '../../../utils/inner-html-util';
+import {
+ htmlEscape,
+ sanitizeHtml,
+ sanitizeHtmlToFragment,
+} from '../../../utils/inner-html-util';
import {unescapeHTML} from '../../../utils/syntax-util';
import '@polymer/marked-element';
import {resolve} from '../../../models/dependency';
@@ -22,12 +26,15 @@
* This element renders markdown and also applies some regex replacements to
* linkify key parts of the text defined by the host's config.
*
- * TODO: Remove gr-formatted-text once this is rolled out.
+ * TODO: Replace gr-formatted-text with this once markdown flag is rolled out.
*/
@customElement('gr-markdown')
export class GrMarkdown extends LitElement {
@property({type: String})
- markdown?: string;
+ content = '';
+
+ @property({type: Boolean})
+ markdown = false;
@state()
private repoCommentLinks: CommentLinks = {};
@@ -86,6 +93,11 @@
li {
margin-left: var(--spacing-xl);
}
+ .plaintext {
+ font: inherit;
+ white-space: var(--linked-text-white-space, pre-wrap);
+ word-wrap: var(--linked-text-word-wrap, break-word);
+ }
`,
];
@@ -99,9 +111,25 @@
}
override render() {
- // Note: Handling \u200B added in gr-change-view.ts is not needed here
- // because the commit message is not markdown formatted.
+ if (this.markdown) {
+ return this.renderAsMarkdown();
+ } else {
+ return this.renderAsPlaintext();
+ }
+ }
+ private renderAsPlaintext() {
+ const linkedText = this.rewriteText(
+ htmlEscape(this.content).toString(),
+ this.repoCommentLinks
+ );
+
+ return html`
+ <pre class="plaintext">${sanitizeHtmlToFragment(linkedText)}</pre>
+ `;
+ }
+
+ private renderAsMarkdown() {
// <marked-element> internals will be in charge of calling our custom
// renderer so we wrap 'this.rewriteText' so that 'this' is preserved via
// closure.
@@ -130,11 +158,16 @@
}
// The child with slot is optional but allows us control over the styling.
+ // The `callback` property lets us do a final sanitization of the output
+ // HTML string before it is rendered by `<marked-element>` in case any
+ // rewrites have been abused to attempt an XSS attack.
return html`
<marked-element
- .markdown=${this.escapeAllButBlockQuotes(this.markdown ?? '')}
+ .markdown=${this.escapeAllButBlockQuotes(this.content)}
.breaks=${true}
.renderer=${customRenderer}
+ .callback=${(_error: string | null, contents: string) =>
+ sanitizeHtml(contents)}
>
<div slot="markdown-html"></div>
</marked-element>
diff --git a/polygerrit-ui/app/elements/shared/gr-markdown/gr-markdown_test.ts b/polygerrit-ui/app/elements/shared/gr-markdown/gr-markdown_test.ts
index 1128b3a..2327dfe 100644
--- a/polygerrit-ui/app/elements/shared/gr-markdown/gr-markdown_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-markdown/gr-markdown_test.ts
@@ -57,212 +57,89 @@
).querySelector('gr-markdown')!;
});
- test('renders plain text with links and rewrites', async () => {
- element.markdown = `text
- \ntext with plain link: google.com
- \ntext with config link: LinkRewriteMe
- \ntext with config html: HTMLRewriteMe`;
- await element.updateComplete;
+ suite('as plaintext', () => {
+ setup(async () => {
+ element.markdown = false;
+ await element.updateComplete;
+ });
- assert.shadowDom.equal(
- element,
- /* HTML */ `
- <marked-element>
- <div slot="markdown-html">
- <p>text</p>
- <p>
- text with plain link:
- <a href="http://google.com" rel="noopener" target="_blank">
- google.com
- </a>
- </p>
- <p>
- text with config link:
- <a
- href="http://google.com/LinkRewriteMe"
- rel="noopener"
- target="_blank"
- >
- LinkRewriteMe
- </a>
- </p>
- <p>text with config html:</p>
+ test('renders text with links and rewrites', async () => {
+ element.content = `text with plain link: google.com
+ \ntext with config link: LinkRewriteMe
+ \ntext with config html: HTMLRewriteMe`;
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <pre class="plaintext">
+ text with plain link:
+ <a href="http://google.com" rel="noopener" target="_blank">
+ google.com
+ </a>
+ text with config link:
+ <a
+ href="http://google.com/LinkRewriteMe"
+ rel="noopener"
+ target="_blank"
+ >
+ LinkRewriteMe
+ </a>
+ text with config html:
<div>HTMLRewritten</div>
- <p></p>
- </div>
- </marked-element>
- `
- );
+ </pre>
+ `
+ );
+ });
+
+ test('does not render typed html', async () => {
+ element.content = 'plain text <div>foo</div>';
+ await element.updateComplete;
+
+ const escapedDiv = '<div>foo</div>';
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `<pre class="plaintext">plain text ${escapedDiv}</pre>`
+ );
+ });
+
+ test('does not render markdown', async () => {
+ element.content = '# A Markdown Heading';
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ '<pre class="plaintext"># A Markdown Heading</pre>'
+ );
+ });
});
- test('renders headings with links and rewrites', async () => {
- element.markdown = `# h1-heading
- \n## h2-heading
- \n### h3-heading
- \n#### h4-heading
- \n##### h5-heading
- \n###### h6-heading
- \n# heading with plain link: google.com
- \n# heading with config link: LinkRewriteMe
- \n# heading with config html: HTMLRewriteMe`;
- await element.updateComplete;
+ suite('as markdown', () => {
+ setup(async () => {
+ element.markdown = true;
+ await element.updateComplete;
+ });
+ test('renders text with links and rewrites', async () => {
+ element.content = `text
+ \ntext with plain link: google.com
+ \ntext with config link: LinkRewriteMe
+ \ntext with config html: HTMLRewriteMe`;
+ await element.updateComplete;
- assert.shadowDom.equal(
- element,
- /* HTML */ `
- <marked-element>
- <div slot="markdown-html">
- <h1 id="h1-heading">h1-heading</h1>
- <h2 id="h2-heading">h2-heading</h2>
- <h3 id="h3-heading">h3-heading</h3>
- <h4 id="h4-heading">h4-heading</h4>
- <h5 id="h5-heading">h5-heading</h5>
- <h6 id="h6-heading">h6-heading</h6>
- <h1 id="heading-with-plain-link-google-com">
- heading with plain link:
- <a href="http://google.com" rel="noopener" target="_blank">
- google.com
- </a>
- </h1>
- <h1 id="heading-with-config-link-linkrewriteme">
- heading with config link:
- <a
- href="http://google.com/LinkRewriteMe"
- rel="noopener"
- target="_blank"
- >
- LinkRewriteMe
- </a>
- </h1>
- <h1 id="heading-with-config-html-htmlrewriteme">
- heading with config html:
- <div>HTMLRewritten</div>
- </h1>
- </div>
- </marked-element>
- `
- );
- });
-
- test('renders inline-code without linking or rewriting', async () => {
- element.markdown = `\`inline code\`
- \n\`inline code with plain link: google.com\`
- \n\`inline code with config link: LinkRewriteMe\`
- \n\`inline code with config html: HTMLRewriteMe\``;
- await element.updateComplete;
-
- assert.shadowDom.equal(
- element,
- /* HTML */ `
- <marked-element>
- <div slot="markdown-html">
- <p>
- <code> inline code </code>
- </p>
- <p>
- <code> inline code with plain link: google.com </code>
- </p>
- <p>
- <code> inline code with config link: LinkRewriteMe </code>
- </p>
- <p>
- <code> inline code with config html: HTMLRewriteMe </code>
- </p>
- </div>
- </marked-element>
- `
- );
- });
- test('renders multiline-code without linking or rewriting', async () => {
- element.markdown = `\`\`\`\nmultiline code\n\`\`\`
- \n\`\`\`\nmultiline code with plain link: google.com\n\`\`\`
- \n\`\`\`\nmultiline code with config link: LinkRewriteMe\n\`\`\`
- \n\`\`\`\nmultiline code with config html: HTMLRewriteMe\n\`\`\``;
- await element.updateComplete;
-
- assert.shadowDom.equal(
- element,
- /* HTML */ `
- <marked-element>
- <div slot="markdown-html">
- <pre>
- <code> multiline code </code>
- </pre>
- <pre>
- <code> multiline code with plain link: google.com </code>
- </pre>
- <pre>
- <code> multiline code with config link: LinkRewriteMe </code>
- </pre>
- <pre>
- <code> multiline code with config html: HTMLRewriteMe </code>
- </pre>
- </div>
- </marked-element>
- `
- );
- });
-
- test('does not render inline images into <img> tags', async () => {
- element.markdown = '';
- await element.updateComplete;
-
- assert.shadowDom.equal(
- element,
- /* HTML */ `
- <marked-element>
- <div slot="markdown-html">
- <p></p>
- </div>
- </marked-element>
- `
- );
- });
-
- test('renders inline links into <a> tags', async () => {
- element.markdown = '[myLink](https://www.google.com)';
- await element.updateComplete;
-
- assert.shadowDom.equal(
- element,
- /* HTML */ `
- <marked-element>
- <div slot="markdown-html">
- <p>
- <a href="https://www.google.com">myLink</a>
- </p>
- </div>
- </marked-element>
- `
- );
- });
-
- test('renders block quotes with links and rewrites', async () => {
- element.markdown = `> block quote
- \n> block quote with plain link: google.com
- \n> block quote with config link: LinkRewriteMe
- \n> block quote with config html: HTMLRewriteMe`;
- await element.updateComplete;
-
- assert.shadowDom.equal(
- element,
- /* HTML */ `
- <marked-element>
- <div slot="markdown-html">
- <blockquote>
- <p>block quote</p>
- </blockquote>
- <blockquote>
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <marked-element>
+ <div slot="markdown-html">
+ <p>text</p>
<p>
- block quote with plain link:
+ text with plain link:
<a href="http://google.com" rel="noopener" target="_blank">
google.com
</a>
</p>
- </blockquote>
- <blockquote>
<p>
- block quote with config link:
+ text with config link:
<a
href="http://google.com/LinkRewriteMe"
rel="noopener"
@@ -271,50 +148,236 @@
LinkRewriteMe
</a>
</p>
- </blockquote>
- <blockquote>
- <p>block quote with config html:</p>
+ <p>text with config html:</p>
<div>HTMLRewritten</div>
<p></p>
- </blockquote>
- </div>
- </marked-element>
- `
- );
- });
+ </div>
+ </marked-element>
+ `
+ );
+ });
- test('never renders typed html', async () => {
- element.markdown = `plain text <div>foo</div>
- \n\`inline code <div>foo</div>\`
- \n\`\`\`\nmultiline code <div>foo</div>\`\`\`
- \n> block quote <div>foo</div>
- \n[inline link <div>foo</div>](http://google.com)`;
- await element.updateComplete;
+ test('renders headings with links and rewrites', async () => {
+ element.content = `# h1-heading
+ \n## h2-heading
+ \n### h3-heading
+ \n#### h4-heading
+ \n##### h5-heading
+ \n###### h6-heading
+ \n# heading with plain link: google.com
+ \n# heading with config link: LinkRewriteMe
+ \n# heading with config html: HTMLRewriteMe`;
+ await element.updateComplete;
- const escapedDiv = '<div>foo</div>';
- assert.shadowDom.equal(
- element,
- /* HTML */ `
- <marked-element>
- <div slot="markdown-html">
- <p>plain text ${escapedDiv}</p>
- <p>
- <code> inline code ${escapedDiv} </code>
- </p>
- <pre>
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <marked-element>
+ <div slot="markdown-html">
+ <h1>h1-heading</h1>
+ <h2>h2-heading</h2>
+ <h3>h3-heading</h3>
+ <h4>h4-heading</h4>
+ <h5>h5-heading</h5>
+ <h6>h6-heading</h6>
+ <h1>
+ heading with plain link:
+ <a href="http://google.com" rel="noopener" target="_blank">
+ google.com
+ </a>
+ </h1>
+ <h1>
+ heading with config link:
+ <a
+ href="http://google.com/LinkRewriteMe"
+ rel="noopener"
+ target="_blank"
+ >
+ LinkRewriteMe
+ </a>
+ </h1>
+ <h1>
+ heading with config html:
+ <div>HTMLRewritten</div>
+ </h1>
+ </div>
+ </marked-element>
+ `
+ );
+ });
+
+ test('renders inline-code without linking or rewriting', async () => {
+ element.content = `\`inline code\`
+ \n\`inline code with plain link: google.com\`
+ \n\`inline code with config link: LinkRewriteMe\`
+ \n\`inline code with config html: HTMLRewriteMe\``;
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <marked-element>
+ <div slot="markdown-html">
+ <p>
+ <code>inline code</code>
+ </p>
+ <p>
+ <code>inline code with plain link: google.com</code>
+ </p>
+ <p>
+ <code>inline code with config link: LinkRewriteMe</code>
+ </p>
+ <p>
+ <code>inline code with config html: HTMLRewriteMe</code>
+ </p>
+ </div>
+ </marked-element>
+ `
+ );
+ });
+ test('renders multiline-code without linking or rewriting', async () => {
+ element.content = `\`\`\`\nmultiline code\n\`\`\`
+ \n\`\`\`\nmultiline code with plain link: google.com\n\`\`\`
+ \n\`\`\`\nmultiline code with config link: LinkRewriteMe\n\`\`\`
+ \n\`\`\`\nmultiline code with config html: HTMLRewriteMe\n\`\`\``;
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <marked-element>
+ <div slot="markdown-html">
+ <pre>
+ <code>multiline code</code>
+ </pre>
+ <pre>
+ <code>multiline code with plain link: google.com</code>
+ </pre>
+ <pre>
+ <code>multiline code with config link: LinkRewriteMe</code>
+ </pre>
+ <pre>
+ <code>multiline code with config html: HTMLRewriteMe</code>
+ </pre>
+ </div>
+ </marked-element>
+ `
+ );
+ });
+
+ test('does not render inline images into <img> tags', async () => {
+ element.content = '';
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <marked-element>
+ <div slot="markdown-html">
+ <p></p>
+ </div>
+ </marked-element>
+ `
+ );
+ });
+
+ test('renders inline links into <a> tags', async () => {
+ element.content = '[myLink](https://www.google.com)';
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <marked-element>
+ <div slot="markdown-html">
+ <p>
+ <a href="https://www.google.com">myLink</a>
+ </p>
+ </div>
+ </marked-element>
+ `
+ );
+ });
+
+ test('renders block quotes with links and rewrites', async () => {
+ element.content = `> block quote
+ \n> block quote with plain link: google.com
+ \n> block quote with config link: LinkRewriteMe
+ \n> block quote with config html: HTMLRewriteMe`;
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <marked-element>
+ <div slot="markdown-html">
+ <blockquote>
+ <p>block quote</p>
+ </blockquote>
+ <blockquote>
+ <p>
+ block quote with plain link:
+ <a href="http://google.com" rel="noopener" target="_blank">
+ google.com
+ </a>
+ </p>
+ </blockquote>
+ <blockquote>
+ <p>
+ block quote with config link:
+ <a
+ href="http://google.com/LinkRewriteMe"
+ rel="noopener"
+ target="_blank"
+ >
+ LinkRewriteMe
+ </a>
+ </p>
+ </blockquote>
+ <blockquote>
+ <p>block quote with config html:</p>
+ <div>HTMLRewritten</div>
+ <p></p>
+ </blockquote>
+ </div>
+ </marked-element>
+ `
+ );
+ });
+
+ test('never renders typed html', async () => {
+ element.content = `plain text <div>foo</div>
+ \n\`inline code <div>foo</div>\`
+ \n\`\`\`\nmultiline code <div>foo</div>\`\`\`
+ \n> block quote <div>foo</div>
+ \n[inline link <div>foo</div>](http://google.com)`;
+ await element.updateComplete;
+
+ const escapedDiv = '<div>foo</div>';
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <marked-element>
+ <div slot="markdown-html">
+ <p>plain text ${escapedDiv}</p>
+ <p>
+ <code>inline code ${escapedDiv}</code>
+ </p>
+ <pre>
<code>
multiline code ${escapedDiv}
</code>
</pre>
- <blockquote>
- <p>block quote ${escapedDiv}</p>
- </blockquote>
- <p>
- <a href="http://google.com"> inline link ${escapedDiv} </a>
- </p>
- </div>
- </marked-element>
- `
- );
+ <blockquote>
+ <p>block quote ${escapedDiv}</p>
+ </blockquote>
+ <p>
+ <a href="http://google.com">inline link ${escapedDiv}</a>
+ </p>
+ </div>
+ </marked-element>
+ `
+ );
+ });
});
});
diff --git a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
index 3ea9c5d..7b5ce15 100644
--- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
+++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
@@ -24,12 +24,14 @@
import {PropertyValues} from 'lit';
import {classMap} from 'lit/directives/class-map.js';
import {KnownExperimentId} from '../../../services/flags/flags';
-import {NumericChangeId} from '../../../api/rest-api';
+import {NumericChangeId, ServerInfo} from '../../../api/rest-api';
import {subscribe} from '../../lit/subscription-controller';
import {resolve} from '../../../models/dependency';
import {changeModelToken} from '../../../models/change/change-model';
import {assert} from '../../../utils/common-util';
import {ShortcutController} from '../../lit/shortcut-controller';
+import {getAccountDisplayName} from '../../../utils/display-name-util';
+import {configModelToken} from '../../../models/config/config-model';
const MAX_ITEMS_DROPDOWN = 10;
@@ -118,6 +120,10 @@
private readonly restApiService = getAppContext().restApiService;
+ private readonly getConfigModel = resolve(this, configModelToken);
+
+ private serverConfig?: ServerInfo;
+
private changeNum?: NumericChangeId;
// private but used in tests
@@ -135,6 +141,13 @@
() => this.getChangeModel().changeNum$,
x => (this.changeNum = x)
);
+ subscribe(
+ this,
+ () => this.getConfigModel().serverConfig$,
+ config => {
+ this.serverConfig = config;
+ }
+ );
this.shortcuts.addLocal({key: Key.UP}, e => this.handleUpKey(e), {
preventDefault: false,
});
@@ -600,6 +613,7 @@
}
}
+ // TODO(dhruvsri): merge with getAccountSuggestions in account-util
async computeReviewerSuggestions() {
this.suggestions = (
(await this.restApiService.getSuggestedAccounts(
@@ -612,7 +626,7 @@
.filter(account => account.email)
.map(account => {
return {
- text: `${account.name ?? ''} <${account.email}>`,
+ text: `${getAccountDisplayName(this.serverConfig, account)}`,
dataValue: account.email,
};
});
diff --git a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts
index 76c5033..6837a71 100644
--- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts
@@ -137,7 +137,11 @@
test('mention selector opens when previous char is \n', async () => {
stubRestApi('getSuggestedAccounts').returns(
Promise.resolve([
- {...createAccountWithEmail('abc@google.com'), name: 'A'},
+ {
+ ...createAccountWithEmail('abc@google.com'),
+ name: 'A',
+ display_name: 'display A',
+ },
{...createAccountWithEmail('abcdef@google.com'), name: 'B'},
])
);
@@ -154,7 +158,7 @@
assert.deepEqual(element.suggestions, [
{
dataValue: 'abc@google.com',
- text: 'A <abc@google.com>',
+ text: 'display A <abc@google.com>',
},
{
dataValue: 'abcdef@google.com',
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor_test.ts
index d958ef4..1637296 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor_test.ts
@@ -637,8 +637,8 @@
// REST API.
let content = [
' <section class="summary">',
- ' <gr-linked-text content="' +
- '[[_computeCurrentRevisionMessage(change)]]"></gr-linked-text>',
+ ' <gr-markdown content="' +
+ '[[_computeCurrentRevisionMessage(change)]]"></gr-markdown>',
' </section>',
];
let highlights = [
@@ -664,7 +664,7 @@
{
contentIndex: 2,
startIndex: 0,
- endIndex: 6,
+ endIndex: 12,
},
]);
const lines = element.linesFromRows(
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection_test.ts
index cb3eed5..89a0756 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection_test.ts
@@ -22,7 +22,7 @@
<div data-side="left">
<div class="comment-thread">
<div class="gr-formatted-text message">
- <span id="output" class="gr-linked-text">This is a comment</span>
+ <span id="output" class="gr-markdown">This is a comment</span>
</div>
</div>
</div>
@@ -44,7 +44,7 @@
<div data-side="right">
<div class="comment-thread">
<div class="gr-formatted-text message">
- <span id="output" class="gr-linked-text"
+ <span id="output" class="gr-markdown"
>This is a comment on the right</span
>
</div>
@@ -60,7 +60,7 @@
<div data-side="left">
<div class="comment-thread">
<div class="gr-formatted-text message">
- <span id="output" class="gr-linked-text"
+ <span id="output" class="gr-markdown"
>This is <a>a</a> different comment 💩 unicode is fun</span
>
</div>
diff --git a/polygerrit-ui/app/models/views/diff.ts b/polygerrit-ui/app/models/views/diff.ts
index 85fa081..63df521 100644
--- a/polygerrit-ui/app/models/views/diff.ts
+++ b/polygerrit-ui/app/models/views/diff.ts
@@ -8,6 +8,7 @@
RepoName,
RevisionPatchSetNum,
BasePatchSetNum,
+ ChangeInfo,
} from '../../api/rest-api';
import {GerritView} from '../../services/router/router-model';
import {UrlEncodedCommentId} from '../../types/common';
@@ -29,7 +30,42 @@
commentLink?: boolean;
}
-export function createDiffUrl(state: Omit<DiffViewState, 'view'>): string {
+/**
+ * This is a convenience type such that you can pass a `ChangeInfo` object
+ * as the `change` property instead of having to set both the `changeNum` and
+ * `project` properties explicitly.
+ */
+export type CreateChangeUrlObject = Omit<
+ DiffViewState,
+ 'view' | 'changeNum' | 'project'
+> & {
+ change: Pick<ChangeInfo, '_number' | 'project'>;
+};
+
+export function isCreateChangeUrlObject(
+ state: CreateChangeUrlObject | Omit<DiffViewState, 'view'>
+): state is CreateChangeUrlObject {
+ return !!(state as CreateChangeUrlObject).change;
+}
+
+export function objToState(
+ obj: CreateChangeUrlObject | Omit<DiffViewState, 'view'>
+): DiffViewState {
+ if (isCreateChangeUrlObject(obj)) {
+ return {
+ ...obj,
+ view: GerritView.DIFF,
+ changeNum: obj.change._number,
+ project: obj.change.project,
+ };
+ }
+ return {...obj, view: GerritView.DIFF};
+}
+
+export function createDiffUrl(
+ obj: CreateChangeUrlObject | Omit<DiffViewState, 'view'>
+) {
+ const state: DiffViewState = objToState(obj);
let range = getPatchRangeExpression(state);
if (range.length) range = '/' + range;
diff --git a/polygerrit-ui/app/test/common-test-setup.ts b/polygerrit-ui/app/test/common-test-setup.ts
index d9d98b9..6f6fe73 100644
--- a/polygerrit-ui/app/test/common-test-setup.ts
+++ b/polygerrit-ui/app/test/common-test-setup.ts
@@ -6,7 +6,6 @@
// TODO(dmfilippov): remove bundled-polymer.js imports when the following issue
// https://github.com/Polymer/polymer-resin/issues/9 is resolved.
import '../scripts/bundled-polymer';
-import './test-router';
import {AppContext, injectAppContext} from '../services/app-context';
import {Finalizable} from '../services/registry';
import {
diff --git a/polygerrit-ui/app/test/test-router.ts b/polygerrit-ui/app/test/test-router.ts
deleted file mode 100644
index cb38f55..0000000
--- a/polygerrit-ui/app/test/test-router.ts
+++ /dev/null
@@ -1,10 +0,0 @@
-/**
- * @license
- * Copyright 2017 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import {GerritNav} from '../elements/core/gr-navigation/gr-navigation';
-
-GerritNav.setup(() => {
- /* noop */
-});
diff --git a/polygerrit-ui/app/utils/account-util.ts b/polygerrit-ui/app/utils/account-util.ts
index 0e2317f..6cc2e59 100644
--- a/polygerrit-ui/app/utils/account-util.ts
+++ b/polygerrit-ui/app/utils/account-util.ts
@@ -20,7 +20,7 @@
} from '../types/common';
import {AccountTag, ReviewerState} from '../constants/constants';
import {assertNever, hasOwnProperty} from './common-util';
-import {getDisplayName} from './display-name-util';
+import {getAccountDisplayName, getDisplayName} from './display-name-util';
import {getApprovalInfo} from './label-util';
import {RestApiService} from '../services/gr-rest-api/gr-rest-api';
import {ParsedChangeInfo} from '../types/types';
@@ -176,6 +176,7 @@
export function getAccountSuggestions(
input: string,
restApiService: RestApiService,
+ config?: ServerInfo,
canSee?: NumericChangeId,
filterActive = false
) {
@@ -185,14 +186,8 @@
if (!accounts) return [];
const accountSuggestions = [];
for (const account of accounts) {
- let nameAndEmail: string;
- if (account.email !== undefined) {
- nameAndEmail = `${account.name ?? ''} <${account.email}>`;
- } else {
- nameAndEmail = account.name ?? '';
- }
accountSuggestions.push({
- name: nameAndEmail,
+ name: getAccountDisplayName(config, account),
value: account._account_id?.toString(),
});
}
diff --git a/polygerrit-ui/app/utils/inner-html-util.ts b/polygerrit-ui/app/utils/inner-html-util.ts
index 8ac114a..ed9bfac 100644
--- a/polygerrit-ui/app/utils/inner-html-util.ts
+++ b/polygerrit-ui/app/utils/inner-html-util.ts
@@ -8,7 +8,7 @@
// Internally at Google it has different a implementation.
import {BrandType} from '../types/common';
-export {sanitizeHtml, htmlEscape} from 'safevalues';
+export {sanitizeHtml, htmlEscape, sanitizeHtmlToFragment} from 'safevalues';
export type SafeStyleSheet = BrandType<string, '_safeHtml'>;
diff --git a/polygerrit-ui/app/utils/link-util.ts b/polygerrit-ui/app/utils/link-util.ts
index 223f780..fd5965b 100644
--- a/polygerrit-ui/app/utils/link-util.ts
+++ b/polygerrit-ui/app/utils/link-util.ts
@@ -8,10 +8,20 @@
import {getBaseUrl} from './url-util';
export function linkifyNormalUrls(base: string): string {
+ // Some tools are known to look for reviewers/CCs by finding lines such as
+ // "R=foo@gmail.com, bar@gmail.com". However, "=" is technically a valid email
+ // character, so ba-linkify interprets the entire string "R=foo@gmail.com" as
+ // an email address. To fix this, we insert a zero width space character
+ // \u200B before linking that prevents ba-linkify from associating the prefix
+ // with the email. After linking we remove the zero width space.
+ const baseWithZeroWidthSpace = base.replace(/^(R=|CC=)/g, '$&\u200B');
const parts: string[] = [];
- window.linkify(base, {
- callback: (text, href) =>
- parts.push(href ? createLinkTemplate(text, href) : text),
+ window.linkify(baseWithZeroWidthSpace, {
+ callback: (text, href) => {
+ const result = href ? createLinkTemplate(text, href) : text;
+ const resultWithoutZeroWidthSpace = result.replace(/\u200B/g, '');
+ parts.push(resultWithoutZeroWidthSpace);
+ },
});
return parts.join('');
}
diff --git a/polygerrit-ui/app/utils/link-util_test.ts b/polygerrit-ui/app/utils/link-util_test.ts
index ecbcb61..c491e35 100644
--- a/polygerrit-ui/app/utils/link-util_test.ts
+++ b/polygerrit-ui/app/utils/link-util_test.ts
@@ -47,13 +47,45 @@
`${linkedNumber} ${linkedFoo}`
);
});
- test('linkifyNormalUrls', () => {
- const googleLink = link('google.com', 'http://google.com');
- const mapsLink = link('maps.google.com', 'http://maps.google.com');
- assert.equal(
- linkifyNormalUrls('google.com, maps.google.com'),
- `${googleLink}, ${mapsLink}`
- );
+ suite('linkifyNormalUrls', () => {
+ test('links urls', () => {
+ const googleLink = link('google.com', 'http://google.com');
+ const mapsLink = link('maps.google.com', 'http://maps.google.com');
+
+ assert.equal(
+ linkifyNormalUrls('google.com, maps.google.com'),
+ `${googleLink}, ${mapsLink}`
+ );
+ });
+
+ test('links emails without including R= prefix', () => {
+ const fooEmail = link('foo@gmail.com', 'mailto:foo@gmail.com');
+ const barEmail = link('bar@gmail.com', 'mailto:bar@gmail.com');
+ assert.equal(
+ linkifyNormalUrls('R=foo@gmail.com, bar@gmail.com'),
+ `R=${fooEmail}, ${barEmail}`
+ );
+ });
+
+ test('links emails without including CC= prefix', () => {
+ const fooEmail = link('foo@gmail.com', 'mailto:foo@gmail.com');
+ const barEmail = link('bar@gmail.com', 'mailto:bar@gmail.com');
+ assert.equal(
+ linkifyNormalUrls('CC=foo@gmail.com, bar@gmail.com'),
+ `CC=${fooEmail}, ${barEmail}`
+ );
+ });
+
+ test('links emails maintains R= and CC= within addresses', () => {
+ const fooBarBazEmail = link(
+ 'fooR=barCC=baz@gmail.com',
+ 'mailto:fooR=barCC=baz@gmail.com'
+ );
+ assert.equal(
+ linkifyNormalUrls('fooR=barCC=baz@gmail.com'),
+ fooBarBazEmail
+ );
+ });
});
});
