Total byte audit reports full URL

[johnboxall]

Currently the total-byte-weight audit only reports getURLDisplayName for URLs.

This can make it challenging to understand what resource the audit is talking about, as sites we work with include 3rd party scripts with mystery names.

Without the complete URL, it is impossible to create detailed CI tests that fail if certain resources are bigger than a threshold. In our case, we often do not control the size of all assets on the page, so we don't want to fail on the overall threshold... just thing we have control on.

This change displays the full URL.

Before	After

I used the test page https://lighthouse-url-example.surge.sh/ with a few examples of URLs that we may encounter in the wild.

With the longer URLs, the styling could probably use some love (I could also imagine terrible ideas like having the full URL available on hover) – happy to pursue that if this is a direction you'd like to head in. Assuming this is a direction you're looking at going in, also happy to update the relevant tests.

❤️

[wardpeet]

As I do like the idea we changed it for a reason as we didn't want to show the full url as it is irrelevant in most cases.

Maybe better to work with a toggle button or tooltip..

Cc @ebidel @paulirish

[paulirish]

@johnboxall thx for the patch! appreciate it.

We've intended to have the full URL there in a tooltip. But based on the patch, it looks like it's not there. :/

[patrickhulce]

Yeah definitely would like to see this change happen. This is part of a broader undertaking to move all the getURLDisplayName calls out of the audits and into the renderer where it's just used for the span text but can view the full URL in tooltip or inspect.

@johnboxall how do you feel about taking a stab at setting the title over in the _renderURL method in lighthouse-core/report/v2/renderer/details-renderer.js?

[johnboxall]

@patrickhulce sure 👍

[johnboxall]

👨‍🎨

[wardpeet]

LGTM! Just have to fix the linting errors where travis throws on.

[johnboxall]

@wardpeet 💁

Just have to fix the linting errors where travis throws on.

So about that 😛

1️⃣ It's not clear to me how I'd resolve this type warning and the related error:

/tmp/tmpvGobNX/front_end/audits2/lighthouse/renderer/details-renderer.js:75: WARNING - actual > parameter 1 of Util.parseURL does not match formal parameter
 found   : (string|undefined)
 required: string
     const parsedURL = Util.parseURL(text.text);
                                     ^^^^^^^^^

Relaxing the type on parseURL seems like the wrong move – I could do something silly like make an intermediate method with a more forgiving type definition ... Any pointers?

2️⃣ It's looks like I'll need to re-generate sample_v2.json to resolve these errors with something like:

node lighthouse-cli/test/fixtures/static-server.js
npm run start -- --output=json --output-path=lighthouse-core/test/results/sample_v2.json http://localhost:8080/dobetterweb/dbw_tester.html

The diff is large. Can you confirm this is expected?

🙏

[patrickhulce]

we're going to have to try to be backwards compatible here now that we've released

maybe we can make parseURL do this safely, accept {string|undefined} and return null if the URL was not parsable for some reason? then in here we can check if we were able to parse and either set the tooltip with the full text or fallback to the original behavior

on the plus side this should address both the linting and the test failures :)

[wardpeet]

thanks for the effort @johnboxall.

@patrickhulce wdyt? 😄

[patrickhulce]

nice! thanks for the work here @johnboxall!

[paulirish]

Most good. Thanks @johnboxall !