Lighthouse: DevTools: node revealing is flaky

[paulirish]

First a little primer on how this system works:

On the Lighthouse side:

• we collect a selector path, an outerHTML snippet, we generate the proprietary node path.
• we set all that on data attributes on the element for safekeeping.

On the devtools side:

1. we let Lighthouse make that renderNode element
2. lighthouse is currently reloading the page to clean things up so we need to wait for that to finish.
3. we wait for SDK.ResourceTreeModel.Events.MainFrameNavigated to fire once before proceeding
4. once that's done we use the domModel.pushNodeByPathToFrontend method to push our fancy node path to the backend and have it return a current nodeID.
5. sometimes it doesnt return any results. sometimes it does return a node, but the nodeForId lookup fails because the domModel isn't populated yet. one of these can happen if the node hasn't been inserted into the DOM yet. (something rather lazy).
6. if we have a working nodeID we give it to linkifyNodeReference which hooks up the blue highlight and click

notes

1. I've noticed linkifyNodeReference being passed nodeIds that appear to be slightly "deprecated" nodeIds for a given element. As in.. the click-to-reveal will work, but the hover will not give a blue highlight. And if you get the current nodeid of the element, it's soemthing like 243 rather than 78.
2. there seems to be some other reasons the transition from nodePath to nodeId fail. A) race conditions with reload B) race conditions with the DOM updating and dom model being build/rebuilt. C) something about the DOM structure trips up this resolving phase.

@robdodson wrestled with this a bit tonight which has unearthed some things..

On that last point (2/C), rob noticed that on his test page, having "Any script at the end of body" will make it fail. If there are no scripts, the whole thing works.
Also if the script is in the head and async, things work.

Why a script at the end could affect this nodePath doesn't really add up.

But that's where things are at the moment!

[kaycebasques]

Can repro on theverge.com

[carmacleod]

Same problem here: http://www.carbondesignsystem.com/
Yes, there are scripts at end of body.

[carmacleod]

1. lighthouse is currently reloading the page to clean things up so we need to wait for that to finish.

This is problematic for an accessibility audit, because you want to be able to drop down menus, open dialogs, etc and then rerun the audit to see if there are errors in those components.

[carmacleod]

Any chance element linking will be in Chrome 65? I am planning to demo the new devtools Accessibility Tree and Color Picker to about 50 people on March 14. A natural segue would be to demo a lighthouse accessibility audit, but if element linking is still broken, then I will leave that part of the demo out. (because without element linking it feels like a step backwards from the previous accessibility devtools, which I have demoed before).

[paulirish]

Hi Carolyn, thanks for the detail on this issue. We'll work on this bug soon.

It won't be fixed in Chrome 65 or 66. I'm sorry for the delay.

[carmacleod]

Ok. Thanks for the time frame info. Much appreciated.

[robdodson]

I think/hope I was able to land a fix for this here https://chromium-review.googlesource.com/c/chromium/src/+/1006116

[carmacleod]

Woo-hoo, @robdodson ! When can I try it? :)

[robdodson]

@paulirish can correct me but I think it usually takes about a day for something to make it into Canary.

[paulirish]

yup. should be in tomorrow's Chrome canary.​

[carmacleod]

Very sorry, but this still doesn't work for me.
I can't reopen this issue - please let me know if you want me to open a new one.

Version 68.0.3397.0 (Official Build) canary (64-bit)

• Tried it in my own product first, but I couldn't reveal the failing element:

• So then I tried it on a google search page, and it works ok, i.e. I _can_ click on failing elements to reveal them in the elements panel (can't focus failed elements, but click is a start).

• So I tried it on the Wikipedia main page, and it doesn't work there either, i.e. I cannot reveal a failed element by clicking, right-clicking, or attempting to focus.

The only difference that I can see is the number of attributes (if there's only id, it works), and the color (clickable ones are purple. ;)

[robdodson]

I think this does work but there's a quirk which I discussed with the team before.

Lighthouse has a dropdown that lets you switch between auditing the page with mobile emulation on or off.

If you're viewing a desktop site, and you set this dropdown to 'desktop', then I think it should work.

Similarly, if you are viewing the site with mobile emulation turned on, and you set the dropdown to 'mobile', I think it will also work.

But, by default, Lighthouse is set to audit on 'mobile' while you are viewing the site in desktop mode. This _may_ mean (depending on how the site is built) that the structure of the DOM is different between those two states. As a result, the node path doesn't match what is currently in the devtools.

Personally I would prefer if Lighthouse did not have this dropdown, and instead audited the page based on whatever mode I was currently viewing it in. I.e. if I have mobile emulation enabled, it does a mobile audit. And if I am in desktop, it does a desktop audit. I realize the reason it doesn't work this way is because we really want to encourage folks to think about their slow-connection mobile web experience—but the side effect is the issue we're encountering here.

[robdodson]

Sorry I forgot to mention, I was able to confirm this bug (and the fix) using Canary and wikipedia. If the audit is on 'mobile' but I'm viewing desktop, I get a non-selectable node path. But if both the audit and my view match, then the path seems to work.

[patrickhulce]

We might want to explore a different visual treatment of elements we can't find. I'm sure we'll never be able to solve all the cases of finding the proper element to highlight, especially given the emulation situation @robdodson described. Grayed out or some other treatment to reflect that it doesn't seem to be there anymore might be nice.

[robdodson]

@patrickhulce Since the current default setup in Lighthouse almost guarantees that these paths will be broken I'd like to include an instruction of some kind to explain that to the user.

[carmacleod]

OOOOOHHHHH. Got it. Sorry for the noob-swirl, but @robdodson is correct - this is not intuitive.

For my part, I thought it was odd that Lighthouse seemed to be doing a mobile audit for my desktop app, but I figured it was probably to make sure it caught performance issues. I unchecked all but accessibility audit, and found it even odder that it was still focusing on mobile when I wasn't even doing a performance audit, but I figured "whatever", and moved on. Didn't guess that it meant that the node paths would be broken.

Suggestion:

• Put the Lighthouse emulation switch with the "Audits to perform" checkboxes.
  This gives you a one-line place to say something like: "Does this page do well on mobile (recommended)".
• Detect whether there's an emulation mode mismatch, and warn the user that node paths (i.e. reveal in elements panel) may only work in matching mode.

FWIW, having the audit settings in 2 places is potentially confusing, resulting in questions (these are just examples; I don't need answers) like: Why are the emulation, throttling and storage settings special? Do they apply to every type of audit? Does Lighthouse throttle an Accessibility audit? For accessibility, I don't want storage cleared, and I don't even want the page refreshed - can I make the UI stay in the state I put it in before the audit?

Perhaps if the settings were all together in one place, they could be more intuitively arranged. For example, emulation and storage settings, followed by audits to perform, with the throttling setting under the performance audit checkbox.

[patrickhulce]

cc @paulirish on setting placement ideas :)

[carmacleod]

Having re-read @robdodson's comment, above, I realize that I vastly prefer his solution, which is:

if I have mobile emulation enabled, it does a mobile audit. And if I am in desktop, it does a desktop audit.

i.e. remove the Lighthouse emulation switch entirely.

If you want to encourage folks to test and improve their slow-connection mobile web experience, add a "Note" below the Lighthouse image, something like (disclaimer: I'm not a designer...):

That way, there's no magic. Everything is out in the open.

[carmacleod]

Really, really sorry, but the original issue is still a problem for me, even in Desktop mode with Lighthouse Desktop emulation.

The Wikipedia page shows the issue fairly well because it has 2 similar, but not identical, failures for invalid lang:

1. The span element in the page footer works (reveal, hover both work as expected):

<a class="external text" href="https://simple.wikipedia.org/wiki/">
   <span class="autonym" title="Simple English (simple:)" lang="simple" xml:lang="simple">
         English (simple form)
   </span>
</a>

1. The anchor element in the page sidebar does not work (no reveal, no hover, nothing):

<a href="https://simple.wikipedia.org/wiki/" title="Simple English" lang="simple" hreflang="simple"
   class="interlanguage-link-target">
      Simple English
</a>

In case it's helpful, the element in my code that is failing is:

<div class="split splitLayout" style="left: 327.25px; position: absolute; right: auto; visibility: visible;"
   role="separator" aria-valuenow="25" aria-orientation="vertical">
   ...
</div>

(which shouldn't be failing, because aria-valuenow and aria-orientation are both valid attributes for separator role in ARIA 1.1... but that's a different issue :)

[robdodson]

I'll have to look more at the Wikipedia issue. It may be that they are populating that list after the browser fires its load event. If so, we may need to wait a bit longer before trying to build the node path.

Regarding your site, it looks like aria-valuemin and aria-valuemax are also required attributes on role=separator. What's the error that Lighthouse is reporting for you?

[carmacleod]

It may be that they are populating that list after the browser fires its load event. If so, we may need to wait a bit longer before trying to build the node path.

Maybe. I know our stuff can take a horribly long time. If that's the problem, then it would be best for our case if Lighthouse had a "don't refresh" option, so I could press "Start" whenever I'm ready. :)

What's the error that Lighthouse is reporting for you?

[aria-*] attributes do not match their roles.
(the full text is in the first screen snap in this comment, above)

it looks like aria-valuemin and aria-valuemax are also required attributes on role=separator.

I think (?) they should be ok, because their values are 0 and 100, and the ARIA 1.1 spec for separator says:

Authors SHOULD also provide the value of aria-valuemin if it is not 0 and the value of aria-valuemax if it is not 100.

[robdodson]

@carmacleod do you have the axe extension installed? Under the hood that's what we're using to come up with these errors.

[robdodson]

sorry, i meant to add, can you test with axe to see if it also gives you that error?

[carmacleod]

Good idea. So, the aXe extension has a lot more info (and a working reveal ;) plus it found a lot more violations (probably because our page is only a skeleton on refresh until it populates. Lighthouse does that unwanted initial refresh, but aXe just runs on the current page content).

Here's the Chrome A11y Audit:

Here's the aXe audit:

Since aXe has a more detailed error message, I was able to check the aXe issues and see that the reason they are flagging it is because there's no screen reader support yet.

[bethwhitmer]

I'm still having this issue in Lighthouse 3.0.0-beta.0 in Chrome Version 68.0.3440.106 (Official Build) (64-bit). I've tested several public pages over the last few days in preparation for some training I'll be doing and haven't found any Failing Elements that contain a selectable node path. :( That was so nice in prior versions of Lighthouse! I know that I could tell my colleagues to download axe for the "Inspect Node" feature but the goal of the training was to show "what accessibility testing you can do in Chrome without downloading a single thing."

Some of the sites I tested include:
www.worldmarket.com
www.ikea.com
www.potterybarn.com

I ran all of my tests on a desktop machine with Device set to "Desktop".

[robdodson]

Hm, yeah it seems to have cropped up again...

@paulirish any idea on a longer duration event we could wait on?

[mattzeunert]

DevTools is passing its own category renderer into the ReportRenderer, but that constructor parameter was removed. I stepped through the Chrome Stable audit panel rendering code and and the DevTools-specific rendering logic never runs.

[paulirish]

youch. nice find @mattzeunert.

@hoten let's get you set up to fix this one (specifically the egregious bug Matt points out) and roll to DevTools.

[connorjclark]

Pushed some changes, now In Chrome Canary. If a node has the _same_ node path (of the form 1,HTML,1,BODY,10,DIV,0,IFRAME, "index,node name" pairs), then the node link should work.

On the examples in this thread and on cnn.com, all nodes without links that I examined were because of a different DOM shape between loads (expected for complex sites).

Besides changing how we reload the page (or rather, instead of _not_ reloading the page), we could have a secondary identifier that we use only in the case that the node path fails. For example, we could find the first parent node with a unique id, and then build the node path out from that. That would resolve cases (most? but not all) where an additional element was placed somewhere in the path, but it would obviously do nothing for the cases where the element simply doesn't exist on the new page load.

[mattzeunert]

Should this work in Canary now? For me the code after await resourceTreeModel.once(SDK.ResourceTreeModel.Events.Load); never runs.

[connorjclark]

@mattzeunert Can you confirm if any node links work in the report for cnn.com?

At the very least, I see a functioning link for a#logo.nav__logo on cnn.com every time (although i notice it takes a second to convert into a link). Others are random due to the nature of cnn.com and my previous comment.

It is possible that the load event doesn't always fire _after_ the report is generated. i can't seem to replicate it though.

[mattzeunert]

Thanks for checking. I think it was just bad luck with what I was looking at, works fine now.

[robdodson]

Hey @Hoten @paulirish given how flaky this feature is, do y'all think we should maybe consider disabling it? For instance, if the shape of the DOM is just different in mobile emulation, vs. desktop then we kind of know this feature will always break to some extent. Is it worth it to have things that are only clickable some of the time? cc @yuinchien

[connorjclark]

I think this feature is too useful to some people to remove. At the very least, I'd like to explore https://github.com/GoogleChrome/lighthouse/issues/2289#issuecomment-446606864 first, before giving up on this.

[carmacleod]

@Hoten Does node linking work if you don't reload the page?
There is an outstanding issue for making page refresh optional.

[connorjclark]

I think if you limit LH to one pass (example, just accessibility/seo) AND disabled the refresh at the end, all node links would work. currently the only node links that work are for pages where the node path does not change.

currently I find things work inconsistently on cnn.com - for some reason the load event doesn't always fire when expected https://github.com/GoogleChrome/lighthouse/issues/2289#issuecomment-451727498

[connorjclark]

@carmacleod I can test the URL you have myself to see if it ever works for me. Just now I tried cnn.com, it didn't work the first few times, but now it always works..