diff --git a/lighthouse-core/report/html/renderer/dom.js b/lighthouse-core/report/html/renderer/dom.js index dee3fa981a93..28de47bb246f 100644 --- a/lighthouse-core/report/html/renderer/dom.js +++ b/lighthouse-core/report/html/renderer/dom.js @@ -16,7 +16,7 @@ */ 'use strict'; -/* globals URL self */ +/* globals URL self Util */ /** @typedef {HTMLElementTagNameMap & {[id: string]: HTMLElement}} HTMLElementByTagName */ @@ -117,52 +117,47 @@ class DOM { convertMarkdownLinkSnippets(text) { const element = this.createElement('span'); - // Split on markdown links (e.g. [some link](https://...)). - const parts = text.split(/\[([^\]]*?)\]\((https?:\/\/.*?)\)/g); - - while (parts.length) { - // Pop off the same number of elements as there are capture groups. - const [preambleText, linkText, linkHref] = parts.splice(0, 3); - element.appendChild(this._document.createTextNode(preambleText)); - - // Append link if there are any. - if (linkText && linkHref) { - const url = new URL(linkHref); - - const DEVELOPERS_GOOGLE_ORIGIN = 'https://developers.google.com'; - if (url.origin === DEVELOPERS_GOOGLE_ORIGIN) { - url.searchParams.set('utm_source', 'lighthouse'); - url.searchParams.set('utm_medium', this._lighthouseChannel); - } - - const a = this.createElement('a'); - a.rel = 'noopener'; - a.target = '_blank'; - a.textContent = linkText; - a.href = url.href; - element.appendChild(a); + for (const segment of Util.splitMarkdownLink(text)) { + if (!segment.isLink) { + // Plain text segment. + element.appendChild(this._document.createTextNode(segment.text)); + continue; } + + // Otherwise, append any links found. + const url = new URL(segment.linkHref); + + const DEVELOPERS_GOOGLE_ORIGIN = 'https://developers.google.com'; + if (url.origin === DEVELOPERS_GOOGLE_ORIGIN) { + url.searchParams.set('utm_source', 'lighthouse'); + url.searchParams.set('utm_medium', this._lighthouseChannel); + } + + const a = this.createElement('a'); + a.rel = 'noopener'; + a.target = '_blank'; + a.textContent = segment.text; + a.href = url.href; + element.appendChild(a); } return element; } /** - * @param {string} text + * @param {string} markdownText * @return {Element} */ - convertMarkdownCodeSnippets(text) { + convertMarkdownCodeSnippets(markdownText) { const element = this.createElement('span'); - const parts = text.split(/`(.*?)`/g); // Split on markdown code slashes - while (parts.length) { - // Pop off the same number of elements as there are capture groups. - const [preambleText, codeText] = parts.splice(0, 2); - element.appendChild(this._document.createTextNode(preambleText)); - if (codeText) { + for (const segment of Util.splitMarkdownCodeSpans(markdownText)) { + if (segment.isCode) { const pre = this.createElement('code'); - pre.textContent = codeText; + pre.textContent = segment.text; element.appendChild(pre); + } else { + element.appendChild(this._document.createTextNode(segment.text)); } } diff --git a/lighthouse-core/report/html/renderer/util.js b/lighthouse-core/report/html/renderer/util.js index b47602a57759..902cfc4cb68b 100644 --- a/lighthouse-core/report/html/renderer/util.js +++ b/lighthouse-core/report/html/renderer/util.js @@ -280,6 +280,73 @@ class Util { return parts.join(' '); } + /** + * Split a string by markdown code spans (enclosed in `backticks`), splitting + * into segments that were enclosed in backticks (marked as `isCode === true`) + * and those that outside the backticks (`isCode === false`). + * @param {string} text + * @return {Array<{isCode: true, text: string}|{isCode: false, text: string}>} + */ + static splitMarkdownCodeSpans(text) { + /** @type {Array<{isCode: true, text: string}|{isCode: false, text: string}>} */ + const segments = []; + + // Split on backticked code spans. + const parts = text.split(/`(.*?)`/g); + for (let i = 0; i < parts.length; i ++) { + const text = parts[i]; + + // Empty strings are an artifact of splitting, not meaningful. + if (!text) continue; + + // Alternates between plain text and code segments. + const isCode = i % 2 !== 0; + segments.push({ + isCode, + text, + }); + } + + return segments; + } + + /** + * Split a string on markdown links (e.g. [some link](https://...)) into + * segments of plain text that weren't part of a link (marked as + * `isLink === false`), and segments with text content and a URL that did make + * up a link (marked as `isLink === true`). + * @param {string} text + * @return {Array<{isLink: true, text: string, linkHref: string}|{isLink: false, text: string}>} + */ + static splitMarkdownLink(text) { + /** @type {Array<{isLink: true, text: string, linkHref: string}|{isLink: false, text: string}>} */ + const segments = []; + + const parts = text.split(/\[([^\]]+?)\]\((https?:\/\/.*?)\)/g); + while (parts.length) { + // Shift off the same number of elements as the pre-split and capture groups. + const [preambleText, linkText, linkHref] = parts.splice(0, 3); + + if (preambleText) { // Skip empty text as it's an artifact of splitting, not meaningful. + segments.push({ + isLink: false, + text: preambleText, + }); + } + + // Append link if there are any. + if (linkText && linkHref) { + segments.push({ + isLink: true, + text: linkText, + linkHref, + }); + } + } + + return segments; + } + /** * @param {URL} parsedUrl * @param {{numPathParts?: number, preserveQuery?: boolean, preserveHost?: boolean}=} options diff --git a/lighthouse-core/scripts/i18n/collect-strings.js b/lighthouse-core/scripts/i18n/collect-strings.js index 583dc407f878..6cff23080d7f 100644 --- a/lighthouse-core/scripts/i18n/collect-strings.js +++ b/lighthouse-core/scripts/i18n/collect-strings.js @@ -14,6 +14,7 @@ const path = require('path'); const assert = require('assert'); const tsc = require('typescript'); const collectAndBakeCtcStrings = require('./bake-ctc-to-lhl.js'); +const Util = require('../../report/html/renderer/util.js'); const LH_ROOT = path.join(__dirname, '../../../'); const UISTRINGS_REGEX = /UIStrings = .*?\};\n/s; @@ -157,28 +158,27 @@ function convertMessageToCtc(message, examples = {}) { * @param {IncrementalCtc} icu */ function _processPlaceholderMarkdownCode(icu) { + const message = icu.message; + // Check that number of backticks is even. - const match = icu.message.match(/`/g); + const match = message.match(/`/g); if (match && match.length % 2 !== 0) { - throw Error(`Open backtick in message "${icu.message}"`); + throw Error(`Open backtick in message "${message}"`); } - // Split on backticked code spans - const parts = icu.message.split(/`(.*?)`/g); icu.message = ''; let idx = 0; - while (parts.length) { - // Pop off the same number of elements as there are capture groups. - const [preambleText, codeText] = parts.splice(0, 2); - icu.message += preambleText; - if (codeText) { + for (const segment of Util.splitMarkdownCodeSpans(message)) { + if (segment.isCode) { const placeholderName = `MARKDOWN_SNIPPET_${idx++}`; // Backtick replacement looks unreadable here, so .join() instead. icu.message += '$' + placeholderName + '$'; icu.placeholders[placeholderName] = { - content: '`' + codeText + '`', - example: codeText, + content: '`' + segment.text + '`', + example: segment.text, }; + } else { + icu.message += segment.text; } } } @@ -189,35 +189,39 @@ function _processPlaceholderMarkdownCode(icu) { * @param {IncrementalCtc} icu */ function _processPlaceholderMarkdownLink(icu) { + const message = icu.message; + // Check for markdown link common errors, ex: // * [extra] (space between brackets and parens) - if (icu.message.match(/\[.*\] \(.*\)/)) { - throw Error(`Bad Link syntax in message "${icu.message}"`); + if (message.match(/\[.*\] \(.*\)/)) { + throw Error(`Bad Link spacing in message "${message}"`); + } + // * [](empty link text) + if (message.match(/\[\]\(.*\)/)) { + throw Error(`markdown link text missing in message "${message}"`); } - // Split on markdown links (e.g. [some link](https://...)). - const parts = icu.message.split(/\[([^\]]*?)\]\((https?:\/\/.*?)\)/g); icu.message = ''; let idx = 0; - while (parts.length) { - // Pop off the same number of elements as there are capture groups. - const [preambleText, linkText, linkHref] = parts.splice(0, 3); - icu.message += preambleText; - - // Append link if there are any. - if (linkText && linkHref) { - const startPlaceholder = `LINK_START_${idx}`; - const endPlaceholder = `LINK_END_${idx}`; - icu.message += '$' + startPlaceholder + '$' + linkText + '$' + endPlaceholder + '$'; - idx++; - icu.placeholders[startPlaceholder] = { - content: '[', - }; - icu.placeholders[endPlaceholder] = { - content: `](${linkHref})`, - }; + for (const segment of Util.splitMarkdownLink(message)) { + if (!segment.isLink) { + // Plain text segment. + icu.message += segment.text; + continue; } + + // Otherwise, append any links found. + const startPlaceholder = `LINK_START_${idx}`; + const endPlaceholder = `LINK_END_${idx}`; + icu.message += '$' + startPlaceholder + '$' + segment.text + '$' + endPlaceholder + '$'; + idx++; + icu.placeholders[startPlaceholder] = { + content: '[', + }; + icu.placeholders[endPlaceholder] = { + content: `](${segment.linkHref})`, + }; } } diff --git a/lighthouse-core/test/report/html/renderer/dom-test.js b/lighthouse-core/test/report/html/renderer/dom-test.js index 0a9e9e6f72e8..39a35b9603de 100644 --- a/lighthouse-core/test/report/html/renderer/dom-test.js +++ b/lighthouse-core/test/report/html/renderer/dom-test.js @@ -10,6 +10,7 @@ const fs = require('fs'); const jsdom = require('jsdom'); const URL = require('../../../../lib/url-shim.js'); const DOM = require('../../../../report/html/renderer/dom.js'); +const Util = require('../../../../report/html/renderer/util.js'); const TEMPLATE_FILE = fs.readFileSync(__dirname + '/../../../../report/html/templates.html', 'utf8'); @@ -21,6 +22,7 @@ describe('DOM', () => { beforeAll(() => { global.URL = URL; + global.Util = Util; const {document} = new jsdom.JSDOM(TEMPLATE_FILE).window; dom = new DOM(document); dom.setLighthouseChannel('someChannel'); @@ -28,6 +30,7 @@ describe('DOM', () => { afterAll(() => { global.URL = undefined; + global.Util = undefined; }); describe('createElement', () => { diff --git a/lighthouse-core/test/report/html/renderer/util-test.js b/lighthouse-core/test/report/html/renderer/util-test.js index 543d5b539886..774b64795b97 100644 --- a/lighthouse-core/test/report/html/renderer/util-test.js +++ b/lighthouse-core/test/report/html/renderer/util-test.js @@ -12,23 +12,9 @@ const sampleResult = require('../../../results/sample_v2.json'); const NBSP = '\xa0'; /* eslint-env jest */ -/* eslint-disable no-console */ /* global URL */ describe('util helpers', () => { - let origConsoleWarn; - let consoleWarnCalls; - - beforeEach(() => { - origConsoleWarn = console.warn; - consoleWarnCalls = []; - console.warn = msg => consoleWarnCalls.push(msg); - }); - - afterEach(() => { - console.warn = origConsoleWarn; - }); - it('formats a number', () => { assert.strictEqual(Util.formatNumber(10), '10'); assert.strictEqual(Util.formatNumber(100.01), '100'); @@ -279,4 +265,172 @@ describe('util helpers', () => { assert.equal(Util.getRootDomain(new URL('http://localhost:8080')), 'localhost'); }); }); + + describe('#splitMarkdownCodeSpans', () => { + it('handles strings with no backticks in them', () => { + expect(Util.splitMarkdownCodeSpans('regular text')).toEqual([ + {isCode: false, text: 'regular text'}, + ]); + }); + + it('does not split on a single backtick', () => { + expect(Util.splitMarkdownCodeSpans('regular `text')).toEqual([ + {isCode: false, text: 'regular `text'}, + ]); + }); + + it('splits on backticked code', () => { + expect(Util.splitMarkdownCodeSpans('regular `code` text')).toEqual([ + {isCode: false, text: 'regular '}, + {isCode: true, text: 'code'}, + {isCode: false, text: ' text'}, + ]); + }); + + it('splits on backticked code at the beginning of the string', () => { + expect(Util.splitMarkdownCodeSpans('`start code` regular text')).toEqual([ + {isCode: true, text: 'start code'}, + {isCode: false, text: ' regular text'}, + ]); + }); + + it('splits on backticked code at the end of the string', () => { + expect(Util.splitMarkdownCodeSpans('regular text `end code`')).toEqual([ + {isCode: false, text: 'regular text '}, + {isCode: true, text: 'end code'}, + ]); + }); + + it('does not split on a single backtick after split out backticked code', () => { + expect(Util.splitMarkdownCodeSpans('regular text `code` and more `text')).toEqual([ + {isCode: false, text: 'regular text '}, + {isCode: true, text: 'code'}, + {isCode: false, text: ' and more `text'}, + ]); + }); + + it('splits on two instances of backticked code', () => { + expect(Util.splitMarkdownCodeSpans('regular text `code` more text `and more code`')).toEqual([ + {isCode: false, text: 'regular text '}, + {isCode: true, text: 'code'}, + {isCode: false, text: ' more text '}, + {isCode: true, text: 'and more code'}, + ]); + }); + + it('splits on two directly adjacent instances of backticked code', () => { + // eslint-disable-next-line max-len + expect(Util.splitMarkdownCodeSpans('regular text `first code``second code` end text')).toEqual([ + {isCode: false, text: 'regular text '}, + {isCode: true, text: 'first code'}, + {isCode: true, text: 'second code'}, + {isCode: false, text: ' end text'}, + ]); + }); + + it('handles text only within backticks', () => { + expect(Util.splitMarkdownCodeSpans('`first code``second code`')).toEqual([ + {isCode: true, text: 'first code'}, + {isCode: true, text: 'second code'}, + ]); + }); + + it('splits on two instances of backticked code separated by only a space', () => { + // eslint-disable-next-line max-len + expect(Util.splitMarkdownCodeSpans('`first code` `second code`')).toEqual([ + {isCode: true, text: 'first code'}, + {isCode: false, text: ' '}, + {isCode: true, text: 'second code'}, + ]); + }); + }); + + describe('#splitMarkdownLink', () => { + it('handles strings with no links in them', () => { + expect(Util.splitMarkdownLink('some text')).toEqual([ + {isLink: false, text: 'some text'}, + ]); + }); + + it('does not split on an incomplete markdown link', () => { + expect(Util.splitMarkdownLink('some [not link text](text')).toEqual([ + {isLink: false, text: 'some [not link text](text'}, + ]); + }); + + it('splits on a markdown link', () => { + expect(Util.splitMarkdownLink('some [link text](https://example.com) text')).toEqual([ + {isLink: false, text: 'some '}, + {isLink: true, text: 'link text', linkHref: 'https://example.com'}, + {isLink: false, text: ' text'}, + ]); + }); + + it('splits on an http markdown link', () => { + expect(Util.splitMarkdownLink('you should [totally click here](http://never-mitm.com) now')).toEqual([ + {isLink: false, text: 'you should '}, + {isLink: true, text: 'totally click here', linkHref: 'http://never-mitm.com'}, + {isLink: false, text: ' now'}, + ]); + }); + + it('does not split on a non-http/https link', () => { + expect(Util.splitMarkdownLink('some [link text](ftp://example.com) text')).toEqual([ + {isLink: false, text: 'some [link text](ftp://example.com) text'}, + ]); + }); + + it('does not split on a malformed markdown link', () => { + expect(Util.splitMarkdownLink('some [link ]text](https://example.com')).toEqual([ + {isLink: false, text: 'some [link ]text](https://example.com'}, + ]); + + expect(Util.splitMarkdownLink('some [link text] (https://example.com')).toEqual([ + {isLink: false, text: 'some [link text] (https://example.com'}, + ]); + }); + + it('does not split on empty link text', () => { + expect(Util.splitMarkdownLink('some [](https://example.com) empty link')).toEqual([ + {isLink: false, text: 'some [](https://example.com) empty link'}, + ]); + }); + + it('splits on a markdown link at the beginning of a string', () => { + expect(Util.splitMarkdownLink('[link text](https://example.com) end text')).toEqual([ + {isLink: true, text: 'link text', linkHref: 'https://example.com'}, + {isLink: false, text: ' end text'}, + ]); + }); + + it('splits on a markdown link at the end of a string', () => { + expect(Util.splitMarkdownLink('start text [link text](https://example.com)')).toEqual([ + {isLink: false, text: 'start text '}, + {isLink: true, text: 'link text', linkHref: 'https://example.com'}, + ]); + }); + + it('handles a string consisting only of a markdown link', () => { + expect(Util.splitMarkdownLink(`[I'm only a link](https://example.com)`)).toEqual([ + {isLink: true, text: `I'm only a link`, linkHref: 'https://example.com'}, + ]); + }); + + it('handles a string starting and ending with a markdown link', () => { + expect(Util.splitMarkdownLink('[first link](https://first.com) other text [second link](https://second.com)')).toEqual([ + {isLink: true, text: 'first link', linkHref: 'https://first.com'}, + {isLink: false, text: ' other text '}, + {isLink: true, text: 'second link', linkHref: 'https://second.com'}, + ]); + }); + + it('handles a string with adjacent markdown links', () => { + expect(Util.splitMarkdownLink('start text [first link](https://first.com)[second link](https://second.com) and scene')).toEqual([ + {isLink: false, text: 'start text '}, + {isLink: true, text: 'first link', linkHref: 'https://first.com'}, + {isLink: true, text: 'second link', linkHref: 'https://second.com'}, + {isLink: false, text: ' and scene'}, + ]); + }); + }); }); diff --git a/lighthouse-core/test/scripts/i18n/collect-strings-test.js b/lighthouse-core/test/scripts/i18n/collect-strings-test.js index 41929d091a42..50c4895450f1 100644 --- a/lighthouse-core/test/scripts/i18n/collect-strings-test.js +++ b/lighthouse-core/test/scripts/i18n/collect-strings-test.js @@ -418,10 +418,18 @@ describe('Convert Message to Placeholder', () => { }); }); - it('catches common link markdown mistakes', () => { - const message = 'Hello [World] (https://google.com/).'; - expect(() => collect.convertMessageToCtc(message)) - .toThrow(/Bad Link syntax in message "Hello \[World\] \(https:\/\/google\.com\/\)\."/); + describe('catches common link markdown mistakes', () => { + it('throws on spaces between link text and href blocks', () => { + const message = 'Hello [World] (https://google.com/).'; + expect(() => collect.convertMessageToCtc(message)) + .toThrow(/Bad Link spacing in message "Hello \[World\] \(https:\/\/google\.com\/\)\."/); + }); + + it('throws on empty link text', () => { + const message = '[](https://example.com/).'; + expect(() => collect.convertMessageToCtc(message)) + .toThrow(/markdown link text missing in message "\[\]\(https:\/\/example\.com\/\)\."/); + }); }); it('converts custom-formatted ICU to placholders', () => {