core(font-size): comment why source = Other happens #9363

connorjclark · 2019-07-13T02:03:56Z

Provide the steps to reproduce

Run LH on http://m.sands-style.com/interviews-features (mobile)

node lighthouse-cli http://www.sands-style.com/ --only-audits=font-size

Or via DevTools.

What is the current behavior?

font-size will have an Unknown (10px).

What is the expected behavior?

It should not say Unknown.

how'd I find this?

I was curious when this audit resulted in Unknown, so I queried httparchive:

CREATE TEMP FUNCTION extractSources(itemsJson STRING)
RETURNS ARRAY<STRING>
LANGUAGE js AS """
  let items = JSON.parse(itemsJson);
  if (!Array.isArray(items)) return [];
  return JSON.parse(itemsJson).map(x=>x.source);
"""; 

SELECT COUNTIF(source = 'Unknown') UNKNOWN,
       COUNTIF(source = 'User Agent Stylesheet') USER_AGENT,
       COUNTIF(source = 'dynamic') DYNAMIC,
       COUNTIF(true) TOTAL
FROM (
  SELECT source
  FROM (
    SELECT extractSources(JSON_EXTRACT(report, '$.audits.font-size.details.items')) AS sources
    FROM `httparchive.latest.lighthouse_mobile`
--     LIMIT 1000
  ),
  UNNEST(sources) as source
)

result:

[
  {
    "UNKNOWN": "8626",
    "USER_AGENT": "111618",
    "DYNAMIC": "485029",
    "TOTAL": "14162843"
  }
]

That's ~0.05% (very few).

Gettings some URLs to work with:

SELECT DISTINCT url, source
FROM (
  SELECT url, extractSources(JSON_EXTRACT(report, '$.audits.font-size.details.items')) AS sources
  FROM `httparchive.latest.lighthouse_mobile`
),
UNNEST(sources) as source
WHERE source = 'Unknown'
LIMIT 10

[
  {
    "url": "http://m.sands-style.com/",
    "source": "Unknown"
  },
  {
    "url": "https://grayes.com/",
    "source": "Unknown"
  },
  {
    "url": "https://fins.money/",
    "source": "Unknown"
  },
  {
    "url": "http://m.espresso.repubblica.it/",
    "source": "Unknown"
  },
  {
    "url": "https://www.heimgourmet.com/",
    "source": "Unknown"
  },
  {
    "url": "https://www.albirplayahotel.com/",
    "source": "Unknown"
  },
  {
    "url": "https://www.twistys.com/",
    "source": "Unknown"
  },
  {
    "url": "http://www.kopi-ki.net/",
    "source": "Unknown"
  },
  {
    "url": "https://grayhawkgolf.com/",
    "source": "Unknown"
  },
  {
    "url": "http://thquanglong.pgdbadon.edu.vn/",
    "source": "Unknown"
  }
]

I picked the first site, saw the unknown font-size was 10, and ran this:

[...document.querySelectorAll('h3')].map(node => [parseFloat(window.getComputedStyle(node).fontSize), node]).sort((a,b) => a[0]-b[0])

and looked at some h3s set to 10. The CSS that sets the font-size to 10 is from a stlesheet, and is even mentioned in other items in the font-size table! weird.

I set a breakpoint in the font-size audit for Unknown, the stylesheetDeclaration is:

{
  "type": "Regular",
  "parentRule": {
    "origin": "regular",
    "selectors": [
      {
        "text": "#footer h3"
      }
    ]
  }
}

Note lack of a stylesheet property. This is set in the font-size gatherer here:

lighthouse/lighthouse-core/gather/gatherers/seo/font-size.js

Line 340 in 7750fe4

// @ts-ignore - guaranteed to exist from the filter immediately above

Finding the failing nodes for this:

analyzedFailingNodesData.map(n => n.cssRule).filter(r => r.parentRule.selectors.some(s => s.text === '#footer h3'))

[
  {
    "fontSize": 10,
    "textLength": 13,
    "node": { ... },
    "cssRule": {
      "type": "Regular",
      "parentRule": {
        "origin": "regular",
        "selectors": [
          {
            "text": "#footer h3"
          }
        ]
      }
    }
  },
  {
    "fontSize": 10,
    "textLength": 12,
    "node": {
        ...
      }
    },
    "cssRule": {
      "type": "Regular",
      "parentRule": {
        "origin": "regular",
        "selectors": [
          {
            "text": "#footer h3"
          }
        ]
      }
    }
  }
]

Note a lack of styleSheetId / styleSheet on the cssRules. Which I trace back to the limtiting we do:

lighthouse/lighthouse-core/gather/gatherers/seo/font-size.js

Line 292 in 7750fe4

.slice(0, MAX_NODES_SOURCE_RULE_FETCHED)

So that makes sense. I'll just leave an explanatory comment.

patrickhulce

LGTM

aside, is PR lint bot broken @paulirish?

audits: isn't an approved type

lighthouse/.cz-config.js

Lines 13 to 22 in b925dbd

    
           {value: 'new_audit',  name: 'new_audit: A new audit'}, 
        
           {value: 'core',       name: 'core:      Driver, gather, (non-new) audits, LHR JSON, etc'}, 
        
           {value: 'tests',      name: 'tests:     Tests, smokehouse, etc'}, 
        
           {value: 'i18n',       name: 'i18n:      Internationalization'}, 
        
           {value: 'docs',       name: 'docs:      Documentation'}, 
        
           {value: 'deps',       name: 'deps:      Dependency bumps only'}, 
        
           {value: 'report',     name: 'report:    Report, UI, renderers'}, 
        
           {value: 'cli',        name: 'cli:       CLI stuff'}, 
        
           {value: 'clients',    name: 'clients:   Extension, DevTools, or LR stuff'}, 
        
           {value: 'misc',       name: 'misc:      Something else entirely'}

brendankenny · 2019-07-15T20:38:02Z

comment why source = Other happens

er...

return {
  selector: '',
  source: 'Unknown',
};

:)

audits(font-size): comment why source = Other happens

9872161

connorjclark requested review from brendankenny, patrickhulce and paulirish as code owners July 13, 2019 02:03

googlebot added the cla: yes label Jul 13, 2019

patrickhulce approved these changes Jul 13, 2019

View reviewed changes

patrickhulce changed the title ~~audits(font-size): comment why source = Other happens~~ core(font-size): comment why source = Other happens Jul 13, 2019

connorjclark merged commit e6b3b8c into master Jul 15, 2019

connorjclark deleted the font-size-comment branch July 15, 2019 20:34

snyk-bot mentioned this pull request Mar 21, 2020

[Snyk] Upgrade lighthouse from 5.1.0 to 5.6.0 godaddy/lighthouse4u#13

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core(font-size): comment why source = Other happens #9363

core(font-size): comment why source = Other happens #9363

	{value: 'new_audit', name: 'new_audit: A new audit'},
	{value: 'core', name: 'core: Driver, gather, (non-new) audits, LHR JSON, etc'},
	{value: 'tests', name: 'tests: Tests, smokehouse, etc'},
	{value: 'i18n', name: 'i18n: Internationalization'},
	{value: 'docs', name: 'docs: Documentation'},
	{value: 'deps', name: 'deps: Dependency bumps only'},
	{value: 'report', name: 'report: Report, UI, renderers'},
	{value: 'cli', name: 'cli: CLI stuff'},
	{value: 'clients', name: 'clients: Extension, DevTools, or LR stuff'},
	{value: 'misc', name: 'misc: Something else entirely'}

core(font-size): comment why source = Other happens #9363

core(font-size): comment why source = Other happens #9363

Conversation

Provide the steps to reproduce

What is the current behavior?

What is the expected behavior?

how'd I find this?

Choose a reason for hiding this comment