core(seo): properly handle anchors in SVG (#6483)

GoogleChrome · Nov 6, 2018 · abf7c4d · abf7c4d
1 parent 2f43eb9
commit abf7c4d
Show file tree

Hide file tree

Showing 6 changed files with 49 additions and 8 deletions.
diff --git a/lighthouse-cli/test/fixtures/seo/seo-failure-cases.html b/lighthouse-cli/test/fixtures/seo/seo-failure-cases.html
@@ -27,6 +27,12 @@ <h2>Anchor text</h2>
   <a href='https://example.com'>click this</a>
   <a href='/test.html'> click this </a>
   <a href='/test.html'>CLICK THIS</a>
+  <svg>
+    <a href='https://example.com/svg-link'>
+      <circle cx="50" cy="50" r="50"/>
+      <text x="0" y="0" >click this</text>
+    </a>
+  </svg>
 
   <!-- FAIL(plugins): java applet on page -->
   <applet code=HelloWorld.class width="200" height="200" id='applet'>

diff --git a/lighthouse-cli/test/smokehouse/seo/expectations.js b/lighthouse-cli/test/smokehouse/seo/expectations.js
@@ -99,10 +99,10 @@ module.exports = [
       },
       'link-text': {
         score: 0,
-        displayValue: '3 links found',
+        displayValue: '4 links found',
         details: {
           items: {
-            length: 3,
+            length: 4,
           },
         },
       },

diff --git a/lighthouse-core/audits/seo/link-text.js b/lighthouse-core/audits/seo/link-text.js
@@ -41,14 +41,22 @@ class LinkText extends Audit {
   static audit(artifacts) {
     const failingLinks = artifacts.CrawlableLinks
       .filter(link => {
+        const href = link.href.toLowerCase();
         if (
-          link.href.toLowerCase().startsWith('javascript:') ||
+          href.startsWith('javascript:') ||
+          href.startsWith('mailto:') ||
           URL.equalWithExcludedFragments(link.href, artifacts.URL.finalUrl)
         ) {
           return false;
         }
 
         return BLOCKLIST.has(link.text.trim().toLowerCase());
+      })
+      .map(link => {
+        return {
+          href: link.href,
+          text: link.text.trim(),
+        };
       });
 
     const headings = [

diff --git a/lighthouse-core/gather/gatherers/seo/crawlable-links.js b/lighthouse-core/gather/gatherers/seo/crawlable-links.js
@@ -13,19 +13,30 @@ class CrawlableLinks extends Gatherer {
    * @param {LH.Gatherer.PassContext} passContext
    * @return {Promise<LH.Artifacts['CrawlableLinks']>}
    */
-  afterPass(passContext) {
+  async afterPass(passContext) {
     const expression = `(function() {
       ${pageFunctions.getElementsInDocumentString}; // define function on page
+      const resolveURLOrNull = url => {
+        try { return new URL(url, window.location.href).href; }
+        catch (_) { return null; }
+      };
+
       const selector = 'a[href]:not([rel~="nofollow"])';
       const elements = getElementsInDocument(selector);
       return elements
         .map(node => ({
-          href: node.href,
-          text: node.innerText
+          href: node.href instanceof SVGAnimatedString ?
+            resolveURLOrNull(node.href.baseVal) :
+            node.href,
+          text: node.href instanceof SVGAnimatedString ?
+            node.textContent :
+            node.innerText,
         }));
     })()`;
 
-    return passContext.driver.evaluateAsync(expression);
+    /** @type {LH.Artifacts['CrawlableLinks']} */
+    const links = await passContext.driver.evaluateAsync(expression, {useIsolation: true});
+    return links.filter(link => typeof link.href === 'string' && link.href);
   }
 }
 

diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js
@@ -84,13 +84,14 @@ function checkTimeSinceLastLongTask() {
  */
 /* istanbul ignore next */
 function getElementsInDocument(selector) {
+  const realMatchesFn = window.__ElementMatches || window.Element.prototype.matches;
   /** @type {Array<Element>} */
   const results = [];
 
   /** @param {NodeListOf<Element>} nodes */
   const _findAllElements = nodes => {
     for (let i = 0, el; el = nodes[i]; ++i) {
-      if (!selector || window.__ElementMatches.call(el, selector)) {
+      if (!selector || realMatchesFn.call(el, selector)) {
         results.push(el);
       }
       // If the element has a shadow root, dig deeper.

diff --git a/...-core/test/audits/seo/anchor-text-test.js → ...se-core/test/audits/seo/link-text-test.js b/...-core/test/audits/seo/anchor-text-test.js → ...se-core/test/audits/seo/link-text-test.js
@@ -64,6 +64,21 @@ describe('SEO: link text audit', () => {
     assert.equal(auditResult.rawValue, true);
   });
 
+  it('ignores mailto: links', () => {
+    const artifacts = {
+      URL: {
+        finalUrl: 'https://example.com/page.html',
+      },
+      CrawlableLinks: [
+        {href: 'mailto:info@example.com', text: 'click here'},
+        {href: 'mailto:mailmaster@localhost', text: 'click here'},
+      ],
+    };
+
+    const auditResult = LinkTextAudit.audit(artifacts);
+    assert.equal(auditResult.rawValue, true);
+  });
+
   it('passes when all links have descriptive texts', () => {
     const artifacts = {
       URL: {