From d022d8e3ff77dd940e89bff4b48830caab7329ba Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Fri, 7 Jun 2019 16:09:01 -0700 Subject: [PATCH 1/2] report: ensure unique SVG element ids --- .../html/renderer/pwa-category-renderer.js | 47 +++++++++++++++++++ lighthouse-core/report/html/templates.html | 8 ++-- .../renderer/pwa-category-renderer-test.js | 34 +++++++++++++- 3 files changed, 84 insertions(+), 5 deletions(-) diff --git a/lighthouse-core/report/html/renderer/pwa-category-renderer.js b/lighthouse-core/report/html/renderer/pwa-category-renderer.js index 0c96c60159b7..222b32dafee9 100644 --- a/lighthouse-core/report/html/renderer/pwa-category-renderer.js +++ b/lighthouse-core/report/html/renderer/pwa-category-renderer.js @@ -18,6 +18,16 @@ /* globals self, Util, CategoryRenderer */ +/** + * An always-increasing counter for making unique SVG ID suffixes. + */ +const svgSuffixCounter = (() => { + let svgSuffix = 0; + return function() { + return svgSuffix++; + }; +})(); + class PwaCategoryRenderer extends CategoryRenderer { /** * @param {LH.ReportResult.Category} category @@ -62,6 +72,11 @@ class PwaCategoryRenderer extends CategoryRenderer { tmpl)); wrapper.href = `#${category.id}`; + // Correct IDs in case multiple instances end up in the page. + const svgRoot = tmpl.querySelector('svg'); + if (!svgRoot) throw new Error('no SVG element found in PWA score gauge template'); + PwaCategoryRenderer._makeSvgReferencesUnique(svgRoot); + const allGroups = this._getGroupIds(category.auditRefs); const passingGroupIds = this._getPassingGroupIds(category.auditRefs); @@ -147,6 +162,38 @@ class PwaCategoryRenderer extends CategoryRenderer { return auditsElem; } + + /** + * Alters SVG id references so multiple instances of an SVG element can coexist + * in a single page. If `svgRoot` has a `` block, gives all elements defined + * in it unique ids, then updates id references (``, + * `fill="url(#...)"`) to the altered ids in all descendents of `svgRoot`. + * @param {SVGElement} svgRoot + */ + static _makeSvgReferencesUnique(svgRoot) { + const defsEl = svgRoot.querySelector('defs'); + if (!defsEl) return; + + const idSuffix = svgSuffixCounter(); + const elsToUpdate = defsEl.querySelectorAll('[id]'); + for (const el of elsToUpdate) { + const oldId = el.id; + const newId = `${oldId}-${idSuffix}`; + el.id = newId; + + // Update all s. + const useEls = svgRoot.querySelectorAll(`use[href="#${oldId}"]`); + for (const useEl of useEls) { + useEl.setAttribute('href', `#${newId}`); + } + + // Update all fill="url(#...)"s. + const fillEls = svgRoot.querySelectorAll(`[fill="url(#${oldId})"]`); + for (const fillEl of fillEls) { + fillEl.setAttribute('fill', `url(#${newId})`); + } + } + } } if (typeof module !== 'undefined' && module.exports) { diff --git a/lighthouse-core/report/html/templates.html b/lighthouse-core/report/html/templates.html index ee4b388c69af..fde5dfd30dac 100644 --- a/lighthouse-core/report/html/templates.html +++ b/lighthouse-core/report/html/templates.html @@ -669,24 +669,24 @@ - + - + - + - + diff --git a/lighthouse-core/test/report/html/renderer/pwa-category-renderer-test.js b/lighthouse-core/test/report/html/renderer/pwa-category-renderer-test.js index 8bbe83b1d6a6..e71a81f5d1ad 100644 --- a/lighthouse-core/test/report/html/renderer/pwa-category-renderer-test.js +++ b/lighthouse-core/test/report/html/renderer/pwa-category-renderer-test.js @@ -242,7 +242,7 @@ describe('PwaCategoryRenderer', () => { describe('#renderScoreGauge', () => { it('renders an error score gauge in case of category error', () => { category.score = null; - const badgeGauge = pwaRenderer.renderScoreGauge(category); + const badgeGauge = pwaRenderer.renderScoreGauge(category, sampleResults.categoryGroups); // Not a PWA gauge. assert.strictEqual(badgeGauge.querySelector('.lh-gauge--pwa__wrapper'), null); @@ -251,5 +251,37 @@ describe('PwaCategoryRenderer', () => { assert.strictEqual(percentageElem.textContent, '?'); assert.strictEqual(percentageElem.title, Util.UIStrings.errorLabel); }); + + it('renders score gauges with unique ids for items in ', () => { + const gauge1 = pwaRenderer.renderScoreGauge(category, sampleResults.categoryGroups); + const gauge1Ids = [...gauge1.querySelectorAll('defs [id]')].map(el => el.id); + assert.ok(gauge1Ids.length > 3); + + const gauge2 = pwaRenderer.renderScoreGauge(category, sampleResults.categoryGroups); + const gauge2Ids = [...gauge2.querySelectorAll('defs [id]')].map(el => el.id); + assert.ok(gauge2Ids.length === gauge1Ids.length); + + /** Returns whether id is used by at least one or fill under `rootEl`. */ + function idInUseElOrFillAttr(rootEl, id) { + const isUse = rootEl.querySelector(`use[href="#${id}"]`); + const isFill = rootEl.querySelector(`[fill="url(#${id})"]`); + + return !!(isUse || isFill); + } + + // Check that each gauge1 ID is actually used in gauge1 and isn't used in gauge2. + for (const gauge1Id of gauge1Ids) { + assert.equal(idInUseElOrFillAttr(gauge1, gauge1Id), true); + assert.ok(!gauge2Ids.includes(gauge1Id)); + assert.equal(idInUseElOrFillAttr(gauge2, gauge1Id), false); + } + + // And that the reverse is true for gauge2 IDs. + for (const gauge2Id of gauge2Ids) { + assert.equal(idInUseElOrFillAttr(gauge1, gauge2Id), false); + assert.equal(idInUseElOrFillAttr(gauge2, gauge2Id), true); + assert.ok(!gauge1Ids.includes(gauge2Id)); + } + }); }); }); From 691ba17c6de53c6df73ff90fe4ca42c6b8bdd54e Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Mon, 10 Jun 2019 13:57:26 -0700 Subject: [PATCH 2/2] names naming names --- .../report/html/renderer/pwa-category-renderer.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lighthouse-core/report/html/renderer/pwa-category-renderer.js b/lighthouse-core/report/html/renderer/pwa-category-renderer.js index 222b32dafee9..1b49cff5c0ce 100644 --- a/lighthouse-core/report/html/renderer/pwa-category-renderer.js +++ b/lighthouse-core/report/html/renderer/pwa-category-renderer.js @@ -21,7 +21,7 @@ /** * An always-increasing counter for making unique SVG ID suffixes. */ -const svgSuffixCounter = (() => { +const getUniqueSuffix = (() => { let svgSuffix = 0; return function() { return svgSuffix++; @@ -174,9 +174,9 @@ class PwaCategoryRenderer extends CategoryRenderer { const defsEl = svgRoot.querySelector('defs'); if (!defsEl) return; - const idSuffix = svgSuffixCounter(); - const elsToUpdate = defsEl.querySelectorAll('[id]'); - for (const el of elsToUpdate) { + const idSuffix = getUniqueSuffix(); + const elementsToUpdate = defsEl.querySelectorAll('[id]'); + for (const el of elementsToUpdate) { const oldId = el.id; const newId = `${oldId}-${idSuffix}`; el.id = newId;