Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot read property 'trim' of undefined #11606

Closed
tkindy opened this issue Oct 28, 2020 · 3 comments · Fixed by #11934
Closed

Cannot read property 'trim' of undefined #11606

tkindy opened this issue Oct 28, 2020 · 3 comments · Fixed by #11934

Comments

@tkindy
Copy link
Contributor

tkindy commented Oct 28, 2020

Provide the steps to reproduce

  1. Run LH on https://www.kctrust.co.uk for just the font-size audit

What is the current behavior?

Lighthouse errors out, saying:

font-size:warn Caught exception: Cannot read property 'trim' of undefined

What is the expected behavior?

Lighthouse doesn't error and reports the font size audit results.

Environment Information

  • Affected Channels: CLI, Node
  • Lighthouse version: 6.3.0
  • Chrome version: 86.0.4240.111
  • Node.js version: v14.13.0
  • Operating System: macOS Mojave 10.14.6

Related issues
I used git bisect to find the PR that introduced this bug, #11248, first released in Lighthouse 6.3.0. I didn't dig any further than that, though. Let me know if I can help with anything else!

@patrickhulce
Copy link
Collaborator

Thanks very much for filing and for the bisect @tkindy! :)

Seems like after #11248 all attributes aren't guaranteed to be defined anymore

We'll need to be more careful about this when using them (and probably can't rely on the += 2 pattern anymore either if style shows up as a single item without a value).

for (let i = 0; i < attributes.length; i += 2) {
const name = attributes[i].toLowerCase();
const value = attributes[i + 1].trim();
if (value) {
map.set(name, value);
}
}

@tkindy
Copy link
Contributor Author

tkindy commented Jan 9, 2021

I dug into this issue further. As @patrickhulce noted, this problem occurs when DOM nodes have attributes without values such as <div class=""> or even just <div class>. Specifically, the problem is with text that has inline styles applied to it where the parent of that text's DOM element has a valueless attribute, like in this example:

<div class>
  <p style="font-size: 10px">Hello, world!</p>
</div>

In these cases, the font-size audit tries to create a selector for the parent element to display in the results table. This first builds a map of the element's attributes using the code @patrickhulce pointed to above, traversing the flattened attribute array which looks like this for that div:

['class', undefined]

Hence the error Cannot read property 'trim' of undefined.

It felt a little weird to me that the selector it creates is for the parent of the text's element instead of the text's element directly. However, assuming that's the intended implementation, I have a fix for this bug that I'll open a PR for soon.

@connorjclark
Copy link
Collaborator

connorjclark commented Jan 11, 2021

It felt a little weird to me that the selector it creates is for the parent of the text's element

I don't have a good explanation for this. I tracked it down to the original PR for this audit. I think it's related to this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants