core(anchor-elements): don't consider ancestorListeners #16731
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ancestor listener logic
#16292 made crawlable-anchors audit more lenient by allowing the 'placeholder anchors' which are called out in the HTML spec. previously LH would say "hey you are misusing anchors!" but.. now we started allowing them.
The logic was:
<a>and and also not using target/download/ping/rel/type/hreflang/referrerpolicy.The event listeners wasn't just the DOM node's event listeners but also its ancestors too. Also ALL types of listeners were considered. Which kinda sucks because any listener installed on
<body>counts.. like idleness detection scripts or basically everything if a site uses event delegation.Anyway. given that, the ancestor listener check yields results like 95% of the time. even tho they probably dont have much to do with the specific anchor's use.
perf
Separate from all this, the perf of this gatherer got a lot slower from this change. See #16704. And the ancestor checks are to blame. I did make it significantly faster but.. in the end, its not worth it to use this low-quality signal to help flag anchors that don't point to crawlable destinations.
Removing ancestor listeners sped things up quite a bit. on cnn.com homepage.. before we had 8,762 CDP calls for 'DOM.pushNodeByPathToFrontend', 'DOM.resolveNode', and 'DOMDebugger.getEventListeners' (each). Now, that number is just 551.
Getting that data took 4400ms, now it takes 160ms.
driveby: generate-timing-trace on html reports
the generate timing trace script can now handle a HTML report too.
fix #16704
partial revert of #16292