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.

Sign up for GitHub

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

Jump to bottom

.matches is not a function causes many audits to error #5934

Closed
lethevimlet opened this issue Aug 29, 2018 · 2 comments
Closed

.matches is not a function causes many audits to error #5934

lethevimlet opened this issue Aug 29, 2018 · 2 comments
Assignees
@connorjclark
Labels
bug P1.5

Comments

@lethevimlet
Copy link

lethevimlet commented Aug 29, 2018 •
edited
Loading

Hi, I was using object HTML tag to exploit element resize detection. Unfortunately, I found through lighthouse that this is detected as using "plugins" a.k.a embedded flash/silverlight/etc even when actually not embedding anything. I strongly suggest lighthouse/google to check for the data attribute of object tag before flagging it as "plugin" usage since object tag is one of the most efficient/cross-browser compatible ways of detecting element resize events, therefore only count as a use of "plugins" if it actually has a data attribute defined.

Anyway, after removing all object tags to my surprise I found I was still getting nagged about the use of "plugins". Digging through the code, and actually checking the source of Lighthouse plugin test, I managed to find the problem, apparently if you define for an element a matches property, this will trigger a false positive for "use of plugins", and maybe other tests too.

For example

el.matches = "don't hate me :(";

will cause "use of plugins" audit to fail, it will even fail when initialized to null.

My wild guess is that you are using matches internally to flag elements and doing something along the lines of

el.hasOwnProperty("matches") {
 warnUseOfPlugins();
}

If this is the case and you are injecting properties to parse the DOM, you should be using a safe scope such as

el._lighthouse_matches

I hope this helps to nail down the issue and hope this behavior does not reflect to the actual SEO algorithm.

BTW good job with Lighthouse tool, it's getting more and more awesome each day!

Here is the smallest example I can think of, to prove the concept.

<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8" />
  <meta http-equiv="X-UA-Compatible" content="IE=edge">
  <meta name="viewport" content="width=device-width, initial-scale=1">
  <title>Page Title</title>
</head>
<body>
  I don't use plugins, I swear!
  <div id="test"></div>
  <script>
    document.querySelector("#test").matches = "don't hate me :(";
  </script>
</body>
</html>

image
@patrickhulce
Copy link
Collaborator

Thanks for filing the very detailed explanation @lethevimlet!

We would love your input here because what we're trying to communicate with Error! is not that you've failed the audit (communicated with a red triangle and exclamation point), but that Lighthouse has encountered an error while trying to evaluate the audit e.g. a bug :)

If you hover over the "Error!" you should see a tooltip that reads

Audit error: Required EmbeddedContent gatherer encountered an error: el.matches is not a function

image

Either way we should try to find a way to fix. Thanks for the minified repro to help us too!

@patrickhulce patrickhulce changed the title Object tag, and matches properties causes to fail plugin audit. .matches is not a function causes many audits to error Aug 29, 2018
@lethevimlet
Copy link
Author

lethevimlet commented Aug 30, 2018 •
edited
Loading

@patrickhulce Nice to hear this comes handy, regarding the Error! communication intention, how about the use of Warning! instead, IMO error feels like something went wrong that might be my fault, whereas Warning! explicitly draws my attention as a developer, saying there's something here you want to read, therefore more people would see the hover tooltip that I totally missed. Other more verbose possibilities includes "Not Applicable", "Irresolvable" or "Ups! Something Went Wrong 😅" if the last one is preferred, please note the emoji is a must!

connorjclark added a commit that referenced this issue Oct 15, 2018
@connorjclark
core: save native HTMLElement.prototype.matches function to avoid con…
f984ab1 
…flict (#5934)_
connorjclark added a commit that referenced this issue Oct 15, 2018
@connorjclark
core: save native HTMLElement.prototype.matches function to avoid con…
5b2a8d6 
…flict (#5934)
@connorjclark connorjclark mentioned this issue Oct 15, 2018
Merged
connorjclark added a commit that referenced this issue Oct 15, 2018
@connorjclark
core: save native HTMLElement.prototype.matches function to avoid con…
b878b6a 
…flict (#5934)
connorjclark added a commit that referenced this issue Oct 15, 2018
@connorjclark
core: save native HTMLElement.prototype.matches function to avoid con…
03617b3 
…flict (#5934)
connorjclark added a commit that referenced this issue Oct 16, 2018
@connorjclark
core: save native HTMLElement.prototype.matches function to avoid con…
d336580 
…flict (#5934)
connorjclark added a commit that referenced this issue Oct 16, 2018
@connorjclark
core: save native HTMLElement.prototype.matches function to avoid con…
5319678 
…flict (#5934)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@connorjclark connorjclark

Labels
bug P1.5
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@patrickhulce @lethevimlet @connorjclark