report(third party): filter out third party urls #6351

wardpeet · 2018-10-21T17:56:35Z

Summary

Hides 3th party urls from our tables when the checkbox is marked. This do not changes scores, just hides it in the UI.

A request is marked as third party when the TLD + 1 can not be found in the url.
pageTLDPlusOne = new URL(this.json.finalUrl).origin.split('.').slice(-2).join('.');

Demo:
http://wardpeet-filestorage.surge.sh/lh-store/pr-6351/report.html

Related Issues/PRs

#4516

need upgraded jsdom:
#6411

wardpeet · 2018-10-21T18:00:12Z

I still need to add a global checkbox to select/unselect all checkboxes. We might also want to show a label on marked elements like @hwikyounglee suggested.

patrickhulce

nice!! slick solution to the even background colors too :)

Two sticking points I still see I think we'll need to wrestle with:

Skipping this in certain audits (like preconnect to 3rd party origins, it doesn't make any sense to hide the list of 100% third party origins, image aspect ratios audit also doesn't have any connection to the origin of the resource)
Super obviously marking that there's information in the table that's missing when filtered. I still kinda like the idea of inserting a summary placeholder row to further indicate that there's information in the table that's being hidden, but open to other ideas of course if @hwikyounglee has some awesome tricks hidden up her sleeve for us :)

lighthouse-core/report/html/renderer/report-ui-features.js

hwikyounglee · 2018-10-24T00:50:45Z

Taking all things out of my sleeve, hmm, not much hidden! But made up a few things now. Open for suggestions.

Probably okay without background color
Probably we could see how the locally scoped checkboxes are used, and reassess a need of a global checkbox.
Just to make the checkboxes look noticeable, I'm wondering if we might try the checkbox 'checked' and the label it as 'Show' instead of 'Filter-out/hide'.

wardpeet · 2018-10-24T19:19:15Z

I think it would be nice to have local scoped checkboxes and a global one? Or maybe the global one is over rated.
We could check it by default and rephrase into Show 3th party resources. I don't really have an opinion on this.

lighthouse-core/report/html/templates.html

wardpeet · 2018-10-31T16:56:02Z

I'm having an issue with the tests because the initFeatures is ran on the report template which does not contain the strings. I could combine the files with a string replace but other errors occur so not sure what to do here? I'm thinking of moving the report-ui-feature-test.js to end to end tests with puppeteer WDYT?

brendankenny · 2018-10-31T17:24:07Z

there's definitely value in having it be a unit test.

the report template which does not contain the strings

Which strings do you mean? The travis failure doesn't give much of a clue

wardpeet · 2018-10-31T18:15:49Z

https://github.com/GoogleChrome/lighthouse/blob/master/lighthouse-core/report/html/reporttemplate.html is used in the report features test which means there are no tags to clone.
Templates are stored in https://github.com/GoogleChrome/lighthouse/blob/master/lighthouse-core/report/html/templates.html. It would make sense to replace the %%LIGHTHOUSE_TEMPLATES%% with the templates but somehow it didn't work. I could figure out why that didn't work

brendankenny · 2018-10-31T18:52:51Z

It would make sense to replace the %%LIGHTHOUSE_TEMPLATES%% with the templates but somehow it didn't work.

ah, I see. I thought maybe you were talking about the i18n strings and was worried :)

build-viewer.js does literally just htmlSrc = htmlSrc.replace(/%%LIGHTHOUSE_TEMPLATES%%/, htmlReportAssets.REPORT_TEMPLATES);, so it should work in theory...

wardpeet · 2018-10-31T21:40:18Z

@brendankenny fixed the issue by changing const container = reportUIFeatures._dom._document.body into const container = reportUIFeatures._dom._document.querySelector('main'); as it's getting cleared by the renderReport function.

Please let me know what is left to move forward.

brendankenny · 2019-04-27T01:57:58Z

No rendering tests, though!

:)

(sorry)

connorjclark · 2019-04-29T16:27:30Z

highlighted is 3P, lol. I think there is nothing that can be done about this?

EDIT: oh, btw I audited "www.nyt.com" which redirects to "www.nytimes.com". I guess this is WAI.

connorjclark · 2019-04-29T17:30:50Z

No rendering tests, though!

:)

(sorry)

done. caught a bug :)

connorjclark · 2019-04-29T18:15:02Z

lighthouse-core/report/html/renderer/report-ui-features.js

-        continue;
-      }
+      const datasetUrl = urlItem.dataset.url || '';
+      const isThirdParty = Util.getRootDomain(datasetUrl) !== rootDomain;


@wardpeet this ok?

brendankenny

LGTM!

over to you, @paulirish

paulirish

brendankenny · 2019-04-30T00:27:43Z

🎊 🎊 🎊

paulirish · 2019-04-30T04:10:27Z

Woo hoooooooooooo!!!!!!!

wardpeet · 2019-04-30T16:49:15Z

@hoten thanks for getting this out the door! 🙏

wardpeet requested a review from paulirish as a code owner October 21, 2018 17:56

wardpeet changed the title ~~report(third party): filter out third party urls~~ report(third party): filter out third party urls (WIP) Oct 21, 2018

patrickhulce reviewed Oct 22, 2018

View reviewed changes

lighthouse-core/report/html/renderer/report-ui-features.js Outdated Show resolved Hide resolved

connorjclark reviewed Oct 22, 2018

View reviewed changes

lighthouse-core/report/html/renderer/report-ui-features.js Outdated Show resolved Hide resolved

connorjclark mentioned this pull request Oct 23, 2018

Third-party Google Assets don't follow their own audit rules, causing report errors #6140

Closed

connorjclark reviewed Oct 26, 2018

View reviewed changes

lighthouse-core/report/html/templates.html Outdated Show resolved Hide resolved

wardpeet added 2 commits October 27, 2018 00:43

core(jsdom): upgrade jsdom to support closest

28cc931

remove lh extension

d99057e

wardpeet force-pushed the feat/report-3p branch 2 times, most recently from 22ef255 to c4d9b64 Compare October 26, 2018 22:58

wardpeet requested review from brendankenny and ebidel as code owners October 26, 2018 22:58

wardpeet added 5 commits October 27, 2018 12:40

fix eslint

9885704

remove jsdom from lighthouse-viewer

0c7d01b

add filter 3p logic to report

9dd3047

Show 3p table

bc9b131

fix eslint

7e23d46

wardpeet force-pushed the feat/report-3p branch from c4d9b64 to 7e23d46 Compare October 27, 2018 12:02

wardpeet changed the title ~~report(third party): filter out third party urls (WIP)~~ report(third party): filter out third party urls Oct 29, 2018

wardpeet added 2 commits October 31, 2018 16:45

Merge branch 'master' into feat/report-3p

1dac41b

update 3rd party

52253b3

fix unit test

350bf5d

connorjclark removed the land-when-ci-is-green label Apr 27, 2019

add render test

341bd5d

vercel bot deployed to staging April 29, 2019 17:18 View deployment

fix edge case

99b0b13

vercel bot deployed to staging April 29, 2019 17:25 View deployment

use dom.find

c9a1b72

vercel bot deployed to staging April 29, 2019 17:29 View deployment

connorjclark reviewed Apr 29, 2019

View reviewed changes

empty commit to trigger CI

2b90b66

vercel bot temporarily deployed to staging April 29, 2019 19:27 Inactive

Merge remote-tracking branch 'origin/master' into feat/report-3p

a29cd61

vercel bot deployed to staging April 29, 2019 19:28 View deployment

updates

a99422a

vercel bot deployed to staging April 29, 2019 20:30 View deployment

brendankenny approved these changes Apr 29, 2019

View reviewed changes

brendankenny assigned paulirish Apr 29, 2019

paulirish approved these changes Apr 29, 2019

View reviewed changes

strings

56e5551

vercel bot deployed to staging April 29, 2019 21:31 View deployment

paulirish added the land-when-ci-is-green label Apr 29, 2019

brendankenny merged commit 94800e1 into master Apr 30, 2019

brendankenny deleted the feat/report-3p branch April 30, 2019 00:26

brendankenny mentioned this pull request Apr 30, 2019

Idea: add toggle to report to filter all flagged resources to same-origin only / 3rd party / 3P #4516

Closed

paulirish mentioned this pull request Sep 15, 2021

report(3p-filter): drop for/id as elements are already nested #13067

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

report(third party): filter out third party urls #6351

report(third party): filter out third party urls #6351

wardpeet commented Oct 21, 2018 •

edited

Loading

wardpeet commented Oct 21, 2018

patrickhulce left a comment

hwikyounglee commented Oct 24, 2018 •

edited

Loading

wardpeet commented Oct 24, 2018

wardpeet commented Oct 31, 2018

brendankenny commented Oct 31, 2018

wardpeet commented Oct 31, 2018 •

edited

Loading

brendankenny commented Oct 31, 2018 •

edited

Loading

wardpeet commented Oct 31, 2018

brendankenny commented Apr 27, 2019 •

edited

Loading

connorjclark commented Apr 29, 2019 •

edited

Loading

connorjclark commented Apr 29, 2019

connorjclark Apr 29, 2019

brendankenny left a comment

paulirish left a comment

brendankenny commented Apr 30, 2019

paulirish commented Apr 30, 2019 via email

wardpeet commented Apr 30, 2019

report(third party): filter out third party urls #6351

report(third party): filter out third party urls #6351

Conversation

wardpeet commented Oct 21, 2018 • edited Loading

wardpeet commented Oct 21, 2018

patrickhulce left a comment

Choose a reason for hiding this comment

hwikyounglee commented Oct 24, 2018 • edited Loading

wardpeet commented Oct 24, 2018

wardpeet commented Oct 31, 2018

brendankenny commented Oct 31, 2018

wardpeet commented Oct 31, 2018 • edited Loading

brendankenny commented Oct 31, 2018 • edited Loading

wardpeet commented Oct 31, 2018

brendankenny commented Apr 27, 2019 • edited Loading

connorjclark commented Apr 29, 2019 • edited Loading

connorjclark commented Apr 29, 2019

connorjclark Apr 29, 2019

Choose a reason for hiding this comment

brendankenny left a comment

Choose a reason for hiding this comment

paulirish left a comment

Choose a reason for hiding this comment

brendankenny commented Apr 30, 2019

paulirish commented Apr 30, 2019 via email

wardpeet commented Apr 30, 2019

wardpeet commented Oct 21, 2018 •

edited

Loading

hwikyounglee commented Oct 24, 2018 •

edited

Loading

wardpeet commented Oct 31, 2018 •

edited

Loading

brendankenny commented Oct 31, 2018 •

edited

Loading

brendankenny commented Apr 27, 2019 •

edited

Loading

connorjclark commented Apr 29, 2019 •

edited

Loading