From 3a8c2e1ebd5d4e3f73e07ab963305376750b413e Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 28 Sep 2026 04:49:30 +0000 Subject: [PATCH] Keep long comment links readable Turn web addresses in review comments into safe links with compact labels so long destinations no longer overflow comment threads. Preserve the full destination for navigation and tooltips while keeping surrounding prose intact. --- .../package/spec/review/AutoLinkText-spec.js | 150 ++++++++++++++++++ .../package/spec/review/Comment-spec.js | 8 + .../package/src/review/AutoLinkText.js | 109 +++++++++++++ .../scrolled/package/src/review/Comment.js | 3 +- .../package/src/review/Comment.module.css | 1 + 5 files changed, 270 insertions(+), 1 deletion(-) create mode 100644 entry_types/scrolled/package/spec/review/AutoLinkText-spec.js create mode 100644 entry_types/scrolled/package/src/review/AutoLinkText.js diff --git a/entry_types/scrolled/package/spec/review/AutoLinkText-spec.js b/entry_types/scrolled/package/spec/review/AutoLinkText-spec.js new file mode 100644 index 0000000000..be3d04a583 --- /dev/null +++ b/entry_types/scrolled/package/spec/review/AutoLinkText-spec.js @@ -0,0 +1,150 @@ +import React from 'react'; +import {render} from '@testing-library/react'; +import '@testing-library/jest-dom/extend-expect'; + +import {AutoLinkText} from 'review/AutoLinkText'; + +describe('AutoLinkText', () => { + it('turns URLs into links with shortened text and the full URL as title', () => { + const url = 'https://www.example.com/a/very/long/path/that/keeps/going/without/' + + 'short/segments?utm_campaign=long-url-in-comment-thread'; + + const {getByRole} = render(); + + const link = getByRole('link'); + expect(link).toHaveAttribute('href', url); + expect(link).toHaveAttribute('title', url); + expect(link).toHaveAttribute('target', '_blank'); + expect(link).toHaveAttribute('rel', 'noopener noreferrer'); + expect(link).toHaveTextContent( + 'www.example.com/a/very/long/path/that/keeps/…-comment-thread' + ); + }); + + it('shortens display text only when it exceeds 60 characters', () => { + const sixtyCharacterUrl = `https://${'a'.repeat(56)}.com`; + const sixtyOneCharacterUrl = `https://${'a'.repeat(57)}.com`; + const {getByRole, rerender} = render(); + + expect(getByRole('link')).toHaveTextContent(sixtyCharacterUrl.slice(8)); + + rerender(); + + expect(getByRole('link').textContent).toHaveLength(60); + expect(getByRole('link')).toHaveTextContent('…'); + }); + + it('does not split Unicode characters when shortening display text', () => { + const url = `https://example.com/${'a'.repeat(31)}😀${'b'.repeat(20)}`; + const expected = `example.com/${'a'.repeat(31)}😀…${'b'.repeat(15)}`; + + const {getByRole} = render(); + + expect(getByRole('link')).toHaveTextContent(expected); + }); + + it('leaves sentence punctuation outside the link', () => { + const {getByRole} = render( + + ); + + const link = getByRole('link'); + expect(link).toHaveAttribute('href', 'https://example.com/docs'); + expect(link.nextSibling).toHaveTextContent('). Next.'); + }); + + it.each([ + ['(https://example.com/docs.)', 'https://example.com/docs', '.)'], + ['[(https://example.com/docs)]', 'https://example.com/docs', ')]'], + [ + '(https://example.com/docs/(section)).', + 'https://example.com/docs/(section)', + ').' + ] + ])('removes surrounding punctuation from %s', (text, url, trailingText) => { + const {getByRole} = render(); + + const link = getByRole('link'); + expect(link).toHaveAttribute('href', url); + expect(link.nextSibling).toHaveTextContent(trailingText); + }); + + it('keeps balanced parentheses that are part of a URL', () => { + const url = 'https://example.com/docs/(section)'; + + const {getByRole} = render(); + + expect(getByRole('link')).toHaveAttribute('href', url); + }); + + it('keeps apostrophes that are part of a URL', () => { + const url = "https://en.wikipedia.org/wiki/O'Reilly_Media"; + + const {getByRole} = render(); + + expect(getByRole('link')).toHaveAttribute('href', url); + expect(getByRole('link')).toHaveAttribute('title', url); + }); + + it('leaves enclosing single quotes outside the link', () => { + const url = 'https://example.com/docs'; + + const {getByRole} = render(); + + const link = getByRole('link'); + expect(link).toHaveAttribute('href', url); + expect(link.previousSibling).toHaveTextContent("See '"); + expect(link.nextSibling).toHaveTextContent("'"); + }); + + it.each([ + ["He wrote 'see https://example.com/docs'.", "'."], + ["'(https://example.com/docs)'", ")'"], + ['“https://example.com/docs”', '”'] + ])('leaves closing sentence quotes outside the link in %s', (text, trailingText) => { + const {getByRole} = render(); + + const link = getByRole('link'); + expect(link).toHaveAttribute('href', 'https://example.com/docs'); + expect(link.nextSibling).toHaveTextContent(trailingText); + }); + + it('links multiple URLs while preserving intervening text', () => { + const {getAllByRole, container} = render( + + ); + + expect(getAllByRole('link').map(link => link.getAttribute('href'))).toEqual([ + 'https://one.example.com', + 'http://two.example.com' + ]); + expect(container.textContent).toBe('First one.example.com\nthen two.example.com.'); + }); + + it('does not link unsupported URL schemes', () => { + const text = 'Do not open javascript:alert(1), mailto:test@example.com, ' + + 'git+https://example.com/repo or blob:https://example.com/id'; + const {queryByRole, getByText} = render(); + + expect(queryByRole('link')).toBeNull(); + expect(getByText(text)).toBeInTheDocument(); + }); + + it('only links standalone URLs following nested unsupported schemes', () => { + const text = 'view-source:blob:https://example.com/id then https://standalone.example.com'; + const {getByRole, container} = render(); + + expect(getByRole('link')).toHaveAttribute('href', 'https://standalone.example.com'); + expect(container.textContent) + .toBe('view-source:blob:https://example.com/id then standalone.example.com'); + }); + + it('links a valid URL following a malformed candidate', () => { + const {getByRole, container} = render( + + ); + + expect(getByRole('link')).toHaveAttribute('href', 'https://valid.example.com'); + expect(container.textContent).toBe('Broken https://? then valid.example.com'); + }); +}); diff --git a/entry_types/scrolled/package/spec/review/Comment-spec.js b/entry_types/scrolled/package/spec/review/Comment-spec.js index d154f60612..b92806d9e7 100644 --- a/entry_types/scrolled/package/spec/review/Comment-spec.js +++ b/entry_types/scrolled/package/spec/review/Comment-spec.js @@ -62,6 +62,14 @@ describe('Comment', () => { expect(getByText(/^Mar \d+$/)).toHaveAttribute('datetime', '2026-03-15T14:30:00Z'); }); + it('renders URLs in the comment body as links', () => { + const {getByRole} = renderWithReviewState( + + ); + + expect(getByRole('link')).toHaveAttribute('href', 'https://example.com/docs'); + }); + describe('edited hint', () => { useFakeTranslations({ 'en.pageflow_scrolled.review.edited': 'Edited %{date}', diff --git a/entry_types/scrolled/package/src/review/AutoLinkText.js b/entry_types/scrolled/package/src/review/AutoLinkText.js new file mode 100644 index 0000000000..6666174d9c --- /dev/null +++ b/entry_types/scrolled/package/src/review/AutoLinkText.js @@ -0,0 +1,109 @@ +import React from 'react'; + +const URL_PATTERN = /\bhttps?:\/\/[^\s<>"]+/gi; +const MAX_LINK_TEXT_LENGTH = 60; +const TRAILING_PUNCTUATION = new Set(['.', ',', '!', '?', ';', ':', "'", '’', '”']); +const BRACKETS = { + '(': ')', + '[': ']', + '{': '}' +}; +const OPENING_BRACKET = Object.fromEntries( + Object.entries(BRACKETS).map(([opening, closing]) => [closing, opening]) +); + +export function AutoLinkText({text}) { + const parts = []; + let start = 0; + let match; + + URL_PATTERN.lastIndex = 0; + + while ((match = URL_PATTERN.exec(text))) { + if (startsInsideUnsupportedScheme(text, match.index)) continue; + + const candidate = match[0]; + const url = removeTrailingPunctuation(candidate); + + if (!isValidUrl(url)) continue; + + parts.push(text.slice(start, match.index)); + parts.push( + + {shortenUrl(url)} + + ); + + start = match.index + url.length; + URL_PATTERN.lastIndex = start; + } + + parts.push(text.slice(start)); + return parts; +} + +function isValidUrl(value) { + try { + return Boolean(new URL(value).hostname); + } + catch (e) { + return false; + } +} + +function startsInsideUnsupportedScheme(text, index) { + let start = index - 1; + + while (start >= 0 && /[a-z0-9+.:-]/i.test(text[start])) start--; + + return /^[a-z][a-z0-9+.:-]*[:+.-]$/i.test(text.slice(start + 1, index)); +} + +function removeTrailingPunctuation(value) { + const excessClosingBrackets = {')': 0, ']': 0, '}': 0}; + + for (const character of value) { + if (BRACKETS[character]) { + excessClosingBrackets[BRACKETS[character]]--; + } + else if (OPENING_BRACKET[character]) { + excessClosingBrackets[character]++; + } + } + + let end = value.length; + + while (end > 0) { + const character = value[end - 1]; + + if (TRAILING_PUNCTUATION.has(character)) { + end--; + } + else if (excessClosingBrackets[character] > 0) { + excessClosingBrackets[character]--; + end--; + } + else { + break; + } + } + + return value.slice(0, end); +} + +function shortenUrl(value) { + const displayValue = value.replace(/^https?:\/\//i, ''); + const characters = Array.from(displayValue); + + if (characters.length <= MAX_LINK_TEXT_LENGTH) return displayValue; + + const endLength = 15; + const startLength = MAX_LINK_TEXT_LENGTH - endLength - 1; + + return `${characters.slice(0, startLength).join('')}…` + + characters.slice(-endLength).join(''); +} diff --git a/entry_types/scrolled/package/src/review/Comment.js b/entry_types/scrolled/package/src/review/Comment.js index 5b54586308..ff1549bfe0 100644 --- a/entry_types/scrolled/package/src/review/Comment.js +++ b/entry_types/scrolled/package/src/review/Comment.js @@ -7,6 +7,7 @@ import {useCurrentUser, useUpdateComment} from './ReviewStateProvider'; import {autoGrow, autoResize} from './autoGrow'; import {formatDate, formatDateTime} from './formatDate'; import {isSubmitShortcut} from './submitShortcut'; +import {AutoLinkText} from './AutoLinkText'; import EditIcon from './images/edit.svg'; import styles from './Comment.module.css'; @@ -49,7 +50,7 @@ export function Comment({ {editing ? : <> -

{comment.body}

+

{comment.editedAt &&

{t('pageflow_scrolled.review.edited', diff --git a/entry_types/scrolled/package/src/review/Comment.module.css b/entry_types/scrolled/package/src/review/Comment.module.css index ed90c07eee..ac1afac428 100644 --- a/entry_types/scrolled/package/src/review/Comment.module.css +++ b/entry_types/scrolled/package/src/review/Comment.module.css @@ -45,6 +45,7 @@ line-height: 1.4; color: var(--ui-on-surface-color); white-space: pre-wrap; + overflow-wrap: anywhere; } .editedHint {