summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorFrank Borden <frankborden@google.com>2022-10-04 12:02:39 +0200
committerLuca Milanesio <luca.milanesio@gmail.com>2022-10-14 17:35:06 +0100
commitb293aa420172f7b5e418c213db93f7743682dbfd (patch)
tree316f6b5cadbfcba2ff961f0bc2ad1273dbe2cbfe
parent76f5ff5c2492003cbd18be5ddae2d93b3af76f3b (diff)
Fix rewrite logic in gr-formatted-text
Previously rewrites were applied sequentially on the user text where the result of one rewrite was passed on to the next rewrite. This had issues where rewrites may trigger on each other's output. It makes the regular expressions difficult to reason about and more order-dependent. Additionally it did not match the previous behavior of link-text-parser which all hosts have already been writing their rewrites against. Release-Notes: skip Google-Bug-Id: b/249523145 b/249597570 b/249322081 Change-Id: I626bfb024af2629683dc6438032c76698c3feb9f
-rw-r--r--polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts14
-rw-r--r--polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts4
-rw-r--r--polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts28
-rw-r--r--polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts70
-rw-r--r--polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.ts178
-rw-r--r--polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.ts471
-rw-r--r--polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.ts415
-rw-r--r--polygerrit-ui/app/utils/link-util.ts226
-rw-r--r--polygerrit-ui/app/utils/link-util_test.ts233
-rw-r--r--polygerrit-ui/app/utils/string-util_test.ts2
10 files changed, 454 insertions, 1187 deletions
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 0ad4a07d38..1000925995 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-button/gr-button';
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-formatted-text/gr-formatted-text';
import '../../shared/gr-overlay/gr-overlay';
import '../../shared/gr-tooltip-content/gr-tooltip-content';
import '../gr-change-actions/gr-change-actions';
@@ -960,7 +960,7 @@ export class GrChangeView extends LitElement {
/* 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-formatted-text {
word-break: break-word;
}
#commitMessageEditor {
@@ -1461,12 +1461,10 @@ export class GrChangeView extends LitElement {
.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-formatted-text
+ .content=${this.latestCommitMessage ?? ''}
+ .markdown=${false}
+ ></gr-formatted-text>
</gr-editable-content>
</div>
<h3 class="assistive-tech-only">Comments and Checks Summary</h3>
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 ad84fb02bf..f3017f8da2 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
@@ -433,9 +433,7 @@ suite('gr-change-view tests', () => {
id="commitMessageEditor"
remove-zero-width-space=""
>
- <gr-linked-text pre="" remove-zero-width-space="">
- <span id="output" slot="insert"></span>
- </gr-linked-text>
+ <gr-formatted-text></gr-formatted-text>
</gr-editable-content>
</div>
<h3 class="assistive-tech-only">
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 6ab0ec468e..5a1db30116 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
@@ -16,11 +16,7 @@ import {resolve} from '../../../models/dependency';
import {subscribe} from '../../lit/subscription-controller';
import {configModelToken} from '../../../models/config/config-model';
import {CommentLinks, EmailAddress} from '../../../api/rest-api';
-import {
- applyHtmlRewritesFromConfig,
- applyLinkRewritesFromConfig,
- linkifyNormalUrls,
-} from '../../../utils/link-util';
+import {linkifyUrlsAndApplyRewrite} from '../../../utils/link-util';
import '../gr-account-chip/gr-account-chip';
import {KnownExperimentId} from '../../../services/flags/flags';
import {getAppContext} from '../../../services/app-context';
@@ -125,7 +121,7 @@ export class GrFormattedText extends LitElement {
}
private renderAsPlaintext() {
- const linkedText = this.rewriteText(
+ const linkedText = linkifyUrlsAndApplyRewrite(
htmlEscape(this.content).toString(),
this.repoCommentLinks
);
@@ -140,7 +136,7 @@ export class GrFormattedText extends LitElement {
// renderer so we wrap 'this.rewriteText' so that 'this' is preserved via
// closure.
const boundRewriteText = (text: string) =>
- this.rewriteText(text, this.repoCommentLinks);
+ linkifyUrlsAndApplyRewrite(text, this.repoCommentLinks);
// We are overriding some marked-element renderers for a few reasons:
// 1. Disable inline images as a design/policy choice.
@@ -201,24 +197,6 @@ export class GrFormattedText extends LitElement {
return text;
}
- private rewriteText(text: string, repoCommentLinks: CommentLinks) {
- // Turn universally identifiable URLs into links. Ex: www.google.com. The
- // markdown library inside marked-element does this too, but is more
- // conservative and misses some URLs like "google.com" without "www" prefix.
- text = linkifyNormalUrls(text);
-
- // Apply the host's config-specific regex replacements to create links. Ex:
- // link "Bug 12345" to "google.com/bug/12345"
- text = applyLinkRewritesFromConfig(text, repoCommentLinks);
-
- // Apply the host's config-specific regex replacements to write arbitrary
- // html. Most examples seen in the wild are also used for linking but with
- // finer control over the rendered text. Ex: "Bug 12345" => "#12345"
- text = applyHtmlRewritesFromConfig(text, repoCommentLinks);
-
- return text;
- }
-
override updated() {
// Look for @mentions and replace them with an account-label chip.
if (this.flagsService.isEnabled(KnownExperimentId.MENTION_USERS)) {
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 6391347896..ca49831892 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
@@ -77,6 +77,76 @@ suite('gr-formatted-text tests', () => {
await element.updateComplete;
});
+ test('does not apply rewrites within links', async () => {
+ element.content = 'google.com/LinkRewriteMe';
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <pre class="plaintext">
+ <a
+ href="http://google.com/LinkRewriteMe"
+ rel="noopener"
+ target="_blank"
+ >
+ google.com/LinkRewriteMe
+ </a>
+ </pre>
+ `
+ );
+ });
+
+ test('does not apply rewrites on rewritten text', async () => {
+ await setCommentLinks({
+ capitalizeFoo: {
+ match: 'foo',
+ html: 'FOO',
+ },
+ lowercaseFoo: {
+ match: 'FOO',
+ html: 'foo',
+ },
+ });
+ element.content = 'foo';
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <pre class="plaintext">
+ FOO
+ </pre
+ >
+ `
+ );
+ });
+
+ test('supports overlapping rewrites', async () => {
+ await setCommentLinks({
+ bracketNum: {
+ match: '(Start:) ([0-9]+)',
+ html: '$1 [$2]',
+ },
+ bracketNum2: {
+ match: '(Start: [0-9]+) ([0-9]+)',
+ html: '$1 [$2]',
+ },
+ });
+ element.content = 'Start: 123 456';
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <pre class="plaintext">
+ Start: [123] [456]
+ </pre
+ >
+ `
+ );
+ });
+
test('renders text with links and rewrites', async () => {
element.content = `text with plain link: google.com
\ntext with config link: LinkRewriteMe
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 16a60e7007..0000000000
--- 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 00e03138aa..0000000000
--- 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 73cf58bd5d..0000000000
--- 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/utils/link-util.ts b/polygerrit-ui/app/utils/link-util.ts
index 9079f4c1fb..b5b90254f5 100644
--- a/polygerrit-ui/app/utils/link-util.ts
+++ b/polygerrit-ui/app/utils/link-util.ts
@@ -4,76 +4,164 @@
* SPDX-License-Identifier: Apache-2.0
*/
import 'ba-linkify/ba-linkify';
-import {CommentLinks} from '../types/common';
+import {CommentLinkInfo, CommentLinks} from '../types/common';
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');
+/**
+ * Finds links within the base string and convert them to HTML. Config-based
+ * rewrites are only applied on text that is not linked by the default linking
+ * library.
+ */
+export function linkifyUrlsAndApplyRewrite(
+ base: string,
+ repoCommentLinks: CommentLinks
+): string {
const parts: string[] = [];
- window.linkify(baseWithZeroWidthSpace, {
+ window.linkify(insertZeroWidthSpace(base), {
callback: (text, href) => {
- const result = href ? createLinkTemplate(href, text) : text;
- const resultWithoutZeroWidthSpace = result.replace(/\u200B/g, '');
- parts.push(resultWithoutZeroWidthSpace);
+ if (href) {
+ parts.push(removeZeroWidthSpace(createLinkTemplate(href, text)));
+ } else {
+ const rewriteResults = getRewriteResultsFromConfig(
+ text,
+ repoCommentLinks
+ );
+ parts.push(removeZeroWidthSpace(applyRewrites(text, rewriteResults)));
+ }
},
});
return parts.join('');
}
-export function applyLinkRewritesFromConfig(
+/**
+ * Generates a list of rewrites that would be applied to a base string. They are
+ * not applied immediately to the base text because one rewrite may interfere or
+ * overlap with a later rewrite. Only after all rewrites are known they are
+ * carefully merged with `applyRewrites`.
+ */
+function getRewriteResultsFromConfig(
base: string,
repoCommentLinks: CommentLinks
-) {
- const linkRewritesFromConfig = Object.values(repoCommentLinks).filter(
- commentLinkInfo => commentLinkInfo.enabled !== false && commentLinkInfo.link
+): RewriteResult[] {
+ const enabledRewrites = Object.values(repoCommentLinks).filter(
+ commentLinkInfo =>
+ commentLinkInfo.enabled !== false &&
+ (commentLinkInfo.link !== undefined || commentLinkInfo.html !== undefined)
);
- const rewrites = linkRewritesFromConfig.map(rewrite => {
- const replacementHref = rewrite.link!.startsWith('/')
- ? `${getBaseUrl()}${rewrite.link!}`
- : rewrite.link!;
- return {
- match: new RegExp(rewrite.match, 'g'),
- replace: createLinkTemplate(
+ return enabledRewrites.flatMap(rewrite => {
+ const regexp = new RegExp(rewrite.match, 'g');
+ const partialResults: RewriteResult[] = [];
+ let match: RegExpExecArray | null;
+
+ while ((match = regexp.exec(base)) !== null) {
+ const fullReplacementText = getReplacementText(match[0], rewrite);
+ // The replacement may not be changing the entire matched substring so we
+ // "trim" the replacement position and text to the part that is actually
+ // different. This makes sure that unchanged portions are still eligible
+ // for other rewrites without being rejected as overlaps during
+ // `applyRewrites`. The new `replacementText` is not eligible for other
+ // rewrites since it would introduce unexpected interactions between
+ // rewrites depending on their order of definition/execution.
+ const sharedPrefixLength = getSharedPrefixLength(
+ match[0],
+ fullReplacementText
+ );
+ const sharedSuffixLength = getSharedSuffixLength(
+ match[0],
+ fullReplacementText
+ );
+ const prefixIndex = sharedPrefixLength;
+ const matchSuffixIndex = match[0].length - sharedSuffixLength;
+ const fullReplacementSuffixIndex =
+ fullReplacementText.length - sharedSuffixLength;
+ partialResults.push({
+ replacedTextStartPosition: match.index + prefixIndex,
+ replacedTextEndPosition: match.index + matchSuffixIndex,
+ replacementText: fullReplacementText.substring(
+ prefixIndex,
+ fullReplacementSuffixIndex
+ ),
+ });
+ }
+ return partialResults;
+ });
+}
+
+/**
+ * Applies all the rewrites to the given base string. To resolve cases where
+ * multiple rewrites target overlapping pieces of the base string, the rewrite
+ * that ends latest is kept and the rest are not applied and discarded.
+ */
+function applyRewrites(base: string, rewriteResults: RewriteResult[]): string {
+ const rewritesByEndPosition = [...rewriteResults].sort((a, b) => {
+ if (b.replacedTextEndPosition !== a.replacedTextEndPosition) {
+ return b.replacedTextEndPosition - a.replacedTextEndPosition;
+ }
+ return a.replacedTextStartPosition - b.replacedTextStartPosition;
+ });
+ const filteredSortedRewrites: RewriteResult[] = [];
+ let latestReplace = base.length;
+ for (const rewrite of rewritesByEndPosition) {
+ // Only accept rewrites that do not overlap with any previously accepted
+ // rewrites.
+ if (rewrite.replacedTextEndPosition <= latestReplace) {
+ filteredSortedRewrites.push(rewrite);
+ latestReplace = rewrite.replacedTextStartPosition;
+ }
+ }
+ return filteredSortedRewrites.reduce(
+ (text, rewrite) =>
+ text
+ .substring(0, rewrite.replacedTextStartPosition)
+ .concat(rewrite.replacementText)
+ .concat(text.substring(rewrite.replacedTextEndPosition)),
+ base
+ );
+}
+
+/**
+ * For a given regexp match, apply the rewrite based on the rewrite's type and
+ * return the resulting string.
+ */
+function getReplacementText(
+ matchedText: string,
+ rewrite: CommentLinkInfo
+): string {
+ if (rewrite.link !== undefined) {
+ const replacementHref = rewrite.link.startsWith('/')
+ ? `${getBaseUrl()}${rewrite.link}`
+ : rewrite.link;
+ const regexp = new RegExp(rewrite.match, 'g');
+ return matchedText.replace(
+ regexp,
+ createLinkTemplate(
replacementHref,
rewrite.text ?? '$&',
rewrite.prefix,
rewrite.suffix
- ),
- };
- });
- return applyRewrites(base, rewrites);
+ )
+ );
+ } else if (rewrite.html !== undefined) {
+ return matchedText.replace(new RegExp(rewrite.match, 'g'), rewrite.html);
+ } else {
+ throw new Error('commentLinkInfo is not a link or html rewrite');
+ }
}
-export function applyHtmlRewritesFromConfig(
- base: string,
- repoCommentLinks: CommentLinks
-) {
- const htmlRewritesFromConfig = Object.values(repoCommentLinks).filter(
- commentLinkInfo => commentLinkInfo.enabled !== false && commentLinkInfo.html
- );
- const rewrites = htmlRewritesFromConfig.map(rewrite => {
- return {
- match: new RegExp(rewrite.match, 'g'),
- replace: rewrite.html!,
- };
- });
- return applyRewrites(base, rewrites);
+/**
+ * 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.
+ */
+function insertZeroWidthSpace(base: string) {
+ return base.replace(/^(R=|CC=)/g, '$&\u200B');
}
-function applyRewrites(
- base: string,
- rewrites: {match: RegExp | string; replace: string}[]
-) {
- return rewrites.reduce(
- (text, rewrite) => text.replace(rewrite.match, rewrite.replace),
- base
- );
+function removeZeroWidthSpace(base: string) {
+ return base.replace(/\u200B/g, '');
}
function createLinkTemplate(
@@ -88,3 +176,41 @@ function createLinkTemplate(
suffix ?? ''
}`;
}
+
+/**
+ * Returns the number of characters that are identical at the start of both
+ * strings.
+ *
+ * For example, `getSharedPrefixLength('12345678', '1234zz78')` would return 4
+ */
+function getSharedPrefixLength(a: string, b: string) {
+ let i = 0;
+ for (; i < a.length && i < b.length; ++i) {
+ if (a[i] !== b[i]) {
+ return i;
+ }
+ }
+ return i;
+}
+
+/**
+ * Returns the number of characters that are identical at the end of both
+ * strings.
+ *
+ * For example, `getSharedSuffixLength('12345678', '1234zz78')` would return 2
+ */
+function getSharedSuffixLength(a: string, b: string) {
+ let i = a.length;
+ for (let j = b.length; i !== 0 && j !== 0; --i, --j) {
+ if (a[i] !== b[j]) {
+ return a.length - 1 - i;
+ }
+ }
+ return a.length - i;
+}
+
+interface RewriteResult {
+ replacedTextStartPosition: number;
+ replacedTextEndPosition: number;
+ replacementText: string;
+}
diff --git a/polygerrit-ui/app/utils/link-util_test.ts b/polygerrit-ui/app/utils/link-util_test.ts
index a1ec2fa246..61d6bff3e8 100644
--- a/polygerrit-ui/app/utils/link-util_test.ts
+++ b/polygerrit-ui/app/utils/link-util_test.ts
@@ -3,11 +3,7 @@
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {
- applyHtmlRewritesFromConfig,
- applyLinkRewritesFromConfig,
- linkifyNormalUrls,
-} from './link-util';
+import {linkifyUrlsAndApplyRewrite} from './link-util';
import {assert} from '@open-wc/testing';
suite('link-util tests', () => {
@@ -15,55 +11,220 @@ suite('link-util tests', () => {
return `<a href="${href}" rel="noopener" target="_blank">${text}</a>`;
}
- test('applyHtmlRewritesFromConfig', () => {
+ suite('link rewrites', () => {
+ test('without text', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('foo', {
+ fooLinkWithoutText: {
+ match: 'foo',
+ link: 'foo.gov',
+ },
+ }),
+ link('foo', 'foo.gov')
+ );
+ });
+
+ test('with text', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('foo', {
+ fooLinkWithText: {
+ match: 'foo',
+ link: 'foo.gov',
+ text: 'foo site',
+ },
+ }),
+ link('foo site', 'foo.gov')
+ );
+ });
+
+ test('with prefix and suffix', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('there are 12 foos here', {
+ fooLinkWithText: {
+ match: '(.*)(bug|foo)s(.*)',
+ link: '$2.gov',
+ text: '$2 list',
+ prefix: '$1on the ',
+ suffix: '$3',
+ },
+ }),
+ `there are 12 on the ${link('foo list', 'foo.gov')} here`
+ );
+ });
+
+ test('multiple matches', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('foo foo', {
+ foo: {
+ match: 'foo',
+ link: 'foo.gov',
+ },
+ }),
+ `${link('foo', 'foo.gov')} ${link('foo', 'foo.gov')}`
+ );
+ });
+
+ test('does not apply within normal links', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('google.com', {
+ ogle: {
+ match: 'ogle',
+ link: 'gerritcodereview.com',
+ },
+ }),
+ link('google.com', 'http://google.com')
+ );
+ });
+ });
+ suite('html rewrites', () => {
+ test('basic case', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('foo', {
+ foo: {
+ match: '(foo)',
+ html: '<div>$1</div>',
+ },
+ }),
+ '<div>foo</div>'
+ );
+ });
+
+ test('only inserts', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('foo', {
+ foo: {
+ match: 'foo',
+ html: 'foo bar',
+ },
+ }),
+ 'foo bar'
+ );
+ });
+
+ test('only deletes', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('foo bar baz', {
+ bar: {
+ match: 'bar',
+ html: '',
+ },
+ }),
+ 'foo baz'
+ );
+ });
+
+ test('multiple matches', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('foo foo', {
+ foo: {
+ match: '(foo)',
+ html: '<div>$1</div>',
+ },
+ }),
+ '<div>foo</div> <div>foo</div>'
+ );
+ });
+
+ test('does not apply within normal links', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('google.com', {
+ ogle: {
+ match: 'ogle',
+ html: '<div>gerritcodereview.com<div>',
+ },
+ }),
+ link('google.com', 'http://google.com')
+ );
+ });
+ });
+
+ test('for overlapping rewrites prefer the latest ending', () => {
assert.equal(
- applyHtmlRewritesFromConfig('#12345 foo', {
- 'number-emphasizer': {
- match: '#(\\d+)',
- html: '<h1>Change $1 is the best change</h1>',
- },
- 'foo-capitalizer': {
+ linkifyUrlsAndApplyRewrite('foobarbaz', {
+ foo: {
match: 'foo',
- html: '<div>FOO</div>',
+ link: 'foo.gov',
+ },
+ foobarbaz: {
+ match: 'foobarbaz',
+ html: '<div>foobarbaz.gov</div>',
+ },
+ foobar: {
+ match: 'foobar',
+ link: 'foobar.gov',
},
}),
- '<h1>Change 12345 is the best change</h1> <div>FOO</div>'
+ '<div>foobarbaz.gov</div>'
);
});
- test('applyLinkRewritesFromConfig', () => {
- const linkedNumber = link('#12345', 'google.com/12345');
- const linkedFoo = link('foo', 'foo.gov');
- const linkedBar = link('Bar Page: 300', 'bar.com/page?id=300');
+ test('overlapping rewrites with same ending prefers earliest start', () => {
assert.equal(
- applyLinkRewritesFromConfig('#12345 foo crowbar:12 bar:300', {
- 'number-linker': {
- match: '#(\\d+)',
- link: 'google.com/$1',
+ linkifyUrlsAndApplyRewrite('foobarbaz', {
+ foo: {
+ match: 'baz',
+ link: 'Baz.gov',
+ },
+ foobarbaz: {
+ match: 'foobarbaz',
+ html: '<div>FooBarBaz.gov</div>',
+ },
+ foobar: {
+ match: 'barbaz',
+ link: 'BarBaz.gov',
},
- 'foo-linker': {
+ }),
+ '<div>FooBarBaz.gov</div>'
+ );
+ });
+
+ test('removed overlapping rewrites do not prevent other rewrites', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('foobarbaz', {
+ foo: {
match: 'foo',
- link: 'foo.gov',
+ html: 'FOO',
+ },
+ oobarba: {
+ match: 'oobarba',
+ html: 'OOBARBA',
+ },
+ baz: {
+ match: 'baz',
+ html: 'BAZ',
+ },
+ }),
+ 'FOObarBAZ'
+ );
+ });
+
+ test('rewrites do not interfere with each other matching', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('bugs: 123 234 345', {
+ bug1: {
+ match: '(bugs:) (\\d+)',
+ html: '$1 <div>bug/$2</div>',
+ },
+ bug2: {
+ match: '(bugs:) (\\d+) (\\d+)',
+ html: '$1 $2 <div>bug/$3</div>',
},
- 'advanced-link': {
- match: '(^|\\s)bar:(\\d+)($|\\s)',
- link: 'bar.com/page?id=$2',
- text: 'Bar Page: $2',
- prefix: '$1',
- suffix: '$3',
+ bug3: {
+ match: '(bugs:) (\\d+) (\\d+) (\\d+)',
+ html: '$1 $2 $3 <div>bug/$4</div>',
},
}),
- `${linkedNumber} ${linkedFoo} crowbar:12 ${linkedBar}`
+ 'bugs: <div>bug/123</div> <div>bug/234</div> <div>bug/345</div>'
);
});
- suite('linkifyNormalUrls', () => {
+ suite('normal links', () => {
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'),
+ linkifyUrlsAndApplyRewrite('google.com, maps.google.com', {}),
`${googleLink}, ${mapsLink}`
);
});
@@ -72,7 +233,7 @@ suite('link-util tests', () => {
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'),
+ linkifyUrlsAndApplyRewrite('R=foo@gmail.com, bar@gmail.com', {}),
`R=${fooEmail}, ${barEmail}`
);
});
@@ -81,7 +242,7 @@ suite('link-util tests', () => {
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'),
+ linkifyUrlsAndApplyRewrite('CC=foo@gmail.com, bar@gmail.com', {}),
`CC=${fooEmail}, ${barEmail}`
);
});
@@ -92,7 +253,7 @@ suite('link-util tests', () => {
'mailto:fooR=barCC=baz@gmail.com'
);
assert.equal(
- linkifyNormalUrls('fooR=barCC=baz@gmail.com'),
+ linkifyUrlsAndApplyRewrite('fooR=barCC=baz@gmail.com', {}),
fooBarBazEmail
);
});
diff --git a/polygerrit-ui/app/utils/string-util_test.ts b/polygerrit-ui/app/utils/string-util_test.ts
index 0a7ee27295..c6c65b13bf 100644
--- a/polygerrit-ui/app/utils/string-util_test.ts
+++ b/polygerrit-ui/app/utils/string-util_test.ts
@@ -12,7 +12,7 @@ import {
diffFilePaths,
} from './string-util';
-suite('formatter util tests', () => {
+suite('string-util tests', () => {
test('pluralize', () => {
const noun = 'comment';
assert.equal(pluralize(0, noun), '');