report: ensure SVG elements in <defs> have unique ids (#9151)

GoogleChrome · Jun 11, 2019 · 233f197 · 233f197
1 parent f8971e2
commit 233f197
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 5 deletions.
diff --git 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 getUniqueSuffix = (() => {
+  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 `<defs>` block, gives all elements defined
+   * in it unique ids, then updates id references (`<use xlink:href="...">`,
+   * `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 = getUniqueSuffix();
+    const elementsToUpdate = defsEl.querySelectorAll('[id]');
+    for (const el of elementsToUpdate) {
+      const oldId = el.id;
+      const newId = `${oldId}-${idSuffix}`;
+      el.id = newId;
+
+      // Update all <use>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
@@ -675,24 +675,24 @@
         <!-- Just fast and reliable. -->
         <g class="lh-gauge--pwa__component lh-gauge--pwa__fast-reliable-badge" transform="translate(20, 29)">
           <path fill="url(#lh-gauge--pwa__fast-reliable__shadow-gradient)" d="M33.63 19.49A30 30 0 0 1 16.2 30.36L3 17.14 17.14 3l16.49 16.49z"/>
-          <use xlink:href="#lh-gauge--pwa__fast-reliable-badge" />
+          <use href="#lh-gauge--pwa__fast-reliable-badge" />
         </g>
 
         <!-- Just installable. -->
         <g class="lh-gauge--pwa__component lh-gauge--pwa__installable-badge" transform="translate(20, 29)">
           <path fill="url(#lh-gauge--pwa__installable__shadow-gradient)" d="M33.629 19.487c-4.272 5.453-10.391 9.39-17.415 10.869L3 17.142 17.142 3 33.63 19.487z"/>
-          <use xlink:href="#lh-gauge--pwa__installable-badge" />
+          <use href="#lh-gauge--pwa__installable-badge" />
         </g>
 
         <!-- Fast and reliable and installable. -->
         <g class="lh-gauge--pwa__component lh-gauge--pwa__fast-reliable-installable-badges">
           <g transform="translate(8, 29)"> <!-- fast and reliable -->
             <path fill="url(#lh-gauge--pwa__fast-reliable__shadow-gradient)" d="M16.321 30.463L3 17.143 17.142 3l22.365 22.365A29.864 29.864 0 0 1 22 31c-1.942 0-3.84-.184-5.679-.537z"/>
-            <use xlink:href="#lh-gauge--pwa__fast-reliable-badge" />
+            <use href="#lh-gauge--pwa__fast-reliable-badge" />
           </g>
           <g transform="translate(32, 29)"> <!-- installable -->
             <path fill="url(#lh-gauge--pwa__installable__shadow-gradient)" d="M25.982 11.84a30.107 30.107 0 0 1-13.08 15.203L3 17.143 17.142 3l8.84 8.84z"/>
-            <use xlink:href="#lh-gauge--pwa__installable-badge" />
+            <use href="#lh-gauge--pwa__installable-badge" />
           </g>
         </g>
 

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
@@ -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 <defs>', () => {
+      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 <use> 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));
+      }
+    });
   });
 });