From bb133e1c56e52460f474f0108a5ae621085baa9c Mon Sep 17 00:00:00 2001 From: Joseph Wynn Date: Mon, 23 Nov 2020 15:52:18 +1300 Subject: [PATCH 01/11] Avoid splitting node labels in the middle of unicode surrogate pairs. Fixes #11697 --- lighthouse-core/lib/page-functions.js | 2 +- lighthouse-core/test/lib/page-functions-test.js | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index 3bc0df208474..5b2ccb4c0364 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -375,7 +375,7 @@ function getNodeLabel(node) { if (str.length <= maxLength) { return str; } - return str.slice(0, maxLength - 1) + 'โ€ฆ'; + return Array.from(str).slice(0, maxLength - 1).join('') + 'โ€ฆ'; } const tagName = node.tagName.toLowerCase(); // html and body content is too broad to be useful, since they contain all page content diff --git a/lighthouse-core/test/lib/page-functions-test.js b/lighthouse-core/test/lib/page-functions-test.js index 90e8be1c2b25..24b6a3212116 100644 --- a/lighthouse-core/test/lib/page-functions-test.js +++ b/lighthouse-core/test/lib/page-functions-test.js @@ -116,6 +116,12 @@ describe('Page Functions', () => { assert.equal(pageFunctions.getNodeLabel(el).length, 80); }); + it('Truncates long text containing unicode surrogate pairs', () => { + const el = dom.createElement('div'); + el.innerText = Array(78).fill('a').join('') + '๐Ÿ’ก๐Ÿ’ก๐Ÿ’ก'; + assert.equal(pageFunctions.getNodeLabel(el), Array(78).fill('a').join('') + '๐Ÿ’กโ€ฆ'); + }); + it('Uses tag name for html tags', () => { const el = dom.createElement('html'); assert.equal(pageFunctions.getNodeLabel(el), 'html'); From 215d30a6e052c4bb9aee6eb6294eb2f0215444c9 Mon Sep 17 00:00:00 2001 From: Joseph Wynn Date: Tue, 24 Nov 2020 08:23:03 +1300 Subject: [PATCH 02/11] Apply suggestions from code review Co-authored-by: Patrick Hulce --- lighthouse-core/lib/page-functions.js | 2 ++ lighthouse-core/test/lib/page-functions-test.js | 2 ++ 2 files changed, 4 insertions(+) diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index 5b2ccb4c0364..4463f331f65c 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -375,6 +375,8 @@ function getNodeLabel(node) { if (str.length <= maxLength) { return str; } + // Take advantage of string iterator multi-byte character awareness. + // Regular `.slice` will ignore unicode character boundaries and lead to malformed text. return Array.from(str).slice(0, maxLength - 1).join('') + 'โ€ฆ'; } const tagName = node.tagName.toLowerCase(); diff --git a/lighthouse-core/test/lib/page-functions-test.js b/lighthouse-core/test/lib/page-functions-test.js index 24b6a3212116..56958940f619 100644 --- a/lighthouse-core/test/lib/page-functions-test.js +++ b/lighthouse-core/test/lib/page-functions-test.js @@ -118,6 +118,8 @@ describe('Page Functions', () => { it('Truncates long text containing unicode surrogate pairs', () => { const el = dom.createElement('div'); + // `getNodeLabel` truncates to 80 characters internally. + // We want to test a unicode character on the boundary. el.innerText = Array(78).fill('a').join('') + '๐Ÿ’ก๐Ÿ’ก๐Ÿ’ก'; assert.equal(pageFunctions.getNodeLabel(el), Array(78).fill('a').join('') + '๐Ÿ’กโ€ฆ'); }); From a17f9e14978388925e09bf0fd833725ab6fbb49a Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Tue, 24 Nov 2020 11:53:54 -0800 Subject: [PATCH 03/11] drop trailing space --- lighthouse-core/test/lib/page-functions-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/test/lib/page-functions-test.js b/lighthouse-core/test/lib/page-functions-test.js index 56958940f619..41f512dec81e 100644 --- a/lighthouse-core/test/lib/page-functions-test.js +++ b/lighthouse-core/test/lib/page-functions-test.js @@ -118,7 +118,7 @@ describe('Page Functions', () => { it('Truncates long text containing unicode surrogate pairs', () => { const el = dom.createElement('div'); - // `getNodeLabel` truncates to 80 characters internally. + // `getNodeLabel` truncates to 80 characters internally. // We want to test a unicode character on the boundary. el.innerText = Array(78).fill('a').join('') + '๐Ÿ’ก๐Ÿ’ก๐Ÿ’ก'; assert.equal(pageFunctions.getNodeLabel(el), Array(78).fill('a').join('') + '๐Ÿ’กโ€ฆ'); From 29ef1f7282c713c03374f62a729f1262e8ec58a6 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Fri, 4 Dec 2020 03:50:47 -0800 Subject: [PATCH 04/11] logging out this env var.. --- .github/workflows/ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1a81a6440870..1ab392fbd32b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,6 +17,8 @@ jobs: - name: git clone uses: actions/checkout@v2 + - run: echo "gh repo env is: $GITHUB_REPOSITORY ... $GITHUB_EVENT_PATH" + - name: Use Node.js 12.x uses: actions/setup-node@v1 with: From ec6b9d1afb4832ab9f97bad8c9fd00757d4c27ee Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Fri, 4 Dec 2020 03:59:56 -0800 Subject: [PATCH 05/11] deepen fork history (attempt to fix...) --- .github/scripts/git-get-shared-history.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/scripts/git-get-shared-history.sh b/.github/scripts/git-get-shared-history.sh index 5c57f1e52e03..151d0a87d01a 100755 --- a/.github/scripts/git-get-shared-history.sh +++ b/.github/scripts/git-get-shared-history.sh @@ -20,6 +20,11 @@ set -euxo pipefail # We can always use some more history git -c protocol.version=2 fetch --deepen=100 + +# If the PR is coming from a fork, we have to get the fork's history, too. +if [[ $GITHUB_REPOSITORY != "GoogleChrome/lighthouse" ]]; then + git -c protocol.version=2 fetch --deepen=100 "git@github.com:$GITHUB_REPOSITORY.git" +fi echo "History is deepened." if git merge-base HEAD origin/master > /dev/null; then From b3569b6a1219b956acc21cde39ca863e8403d247 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Fri, 4 Dec 2020 04:03:18 -0800 Subject: [PATCH 06/11] fixed dumb syntax err --- .github/workflows/ci.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1ab392fbd32b..4d9298e026d8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,8 +17,9 @@ jobs: - name: git clone uses: actions/checkout@v2 - - run: echo "gh repo env is: $GITHUB_REPOSITORY ... $GITHUB_EVENT_PATH" - + - run: echo "gh repo env is $GITHUB_REPOSITORY" + - run: echo "gh evt path is $GITHUB_EVENT_PATH" + - name: Use Node.js 12.x uses: actions/setup-node@v1 with: @@ -27,6 +28,7 @@ jobs: - run: yarn --frozen-lockfile - run: yarn build-all + # Run tests that require headfull Chrome. - run: sudo apt-get install xvfb - name: yarn test-clients From a7fe2e8a5cca26a2d53be666022f1573d3bc00e5 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Fri, 4 Dec 2020 04:23:10 -0800 Subject: [PATCH 07/11] remove these debugging statements. sorry everyone for the commit noise :/ --- .github/workflows/ci.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4d9298e026d8..1a81a6440870 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,9 +17,6 @@ jobs: - name: git clone uses: actions/checkout@v2 - - run: echo "gh repo env is $GITHUB_REPOSITORY" - - run: echo "gh evt path is $GITHUB_EVENT_PATH" - - name: Use Node.js 12.x uses: actions/setup-node@v1 with: @@ -28,7 +25,6 @@ jobs: - run: yarn --frozen-lockfile - run: yarn build-all - # Run tests that require headfull Chrome. - run: sudo apt-get install xvfb - name: yarn test-clients From 05b02a8397ebefab41eda92d15e3e44378b47afa Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Fri, 4 Dec 2020 04:23:34 -0800 Subject: [PATCH 08/11] hopefully fixed the git deepen magic. fingers crossed. --- .github/scripts/git-get-shared-history.sh | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/scripts/git-get-shared-history.sh b/.github/scripts/git-get-shared-history.sh index 151d0a87d01a..b39380881e34 100755 --- a/.github/scripts/git-get-shared-history.sh +++ b/.github/scripts/git-get-shared-history.sh @@ -21,9 +21,11 @@ set -euxo pipefail # We can always use some more history git -c protocol.version=2 fetch --deepen=100 -# If the PR is coming from a fork, we have to get the fork's history, too. -if [[ $GITHUB_REPOSITORY != "GoogleChrome/lighthouse" ]]; then - git -c protocol.version=2 fetch --deepen=100 "git@github.com:$GITHUB_REPOSITORY.git" +# Find out if the PR is coming from a fork +base_clone_url=$(jq '.pull_request.base.repo.clone_url' $GITHUB_EVENT_PATH) +# If it is, we need the fork's history, too. +if [[ $base_clone_url != "GoogleChrome/lighthouse.git" ]]; then + git -c protocol.version=2 fetch --deepen=100 "git@github.com:$base_clone_url" fi echo "History is deepened." From 694e471710198eb6d70fd25fb9b18ade3e3fe932 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Sat, 5 Dec 2020 10:42:27 -0800 Subject: [PATCH 09/11] Update .github/scripts/git-get-shared-history.sh Co-authored-by: Patrick Hulce --- .github/scripts/git-get-shared-history.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/scripts/git-get-shared-history.sh b/.github/scripts/git-get-shared-history.sh index b39380881e34..03b51a131bbf 100755 --- a/.github/scripts/git-get-shared-history.sh +++ b/.github/scripts/git-get-shared-history.sh @@ -22,7 +22,7 @@ set -euxo pipefail git -c protocol.version=2 fetch --deepen=100 # Find out if the PR is coming from a fork -base_clone_url=$(jq '.pull_request.base.repo.clone_url' $GITHUB_EVENT_PATH) +base_clone_url=$(jq '.pull_request.head.repo.clone_url' $GITHUB_EVENT_PATH) # If it is, we need the fork's history, too. if [[ $base_clone_url != "GoogleChrome/lighthouse.git" ]]; then git -c protocol.version=2 fetch --deepen=100 "git@github.com:$base_clone_url" From dea8e07379d2a99cd0143035269e2e957f330c74 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Sat, 5 Dec 2020 15:06:06 -0800 Subject: [PATCH 10/11] who doesn't love debugging in someone else's PR? --- .github/scripts/git-get-shared-history.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/scripts/git-get-shared-history.sh b/.github/scripts/git-get-shared-history.sh index 03b51a131bbf..78074e0c8a42 100755 --- a/.github/scripts/git-get-shared-history.sh +++ b/.github/scripts/git-get-shared-history.sh @@ -22,7 +22,7 @@ set -euxo pipefail git -c protocol.version=2 fetch --deepen=100 # Find out if the PR is coming from a fork -base_clone_url=$(jq '.pull_request.head.repo.clone_url' $GITHUB_EVENT_PATH) +base_clone_url=$(jq --raw-output '.pull_request.head.repo.clone_url' $GITHUB_EVENT_PATH) # If it is, we need the fork's history, too. if [[ $base_clone_url != "GoogleChrome/lighthouse.git" ]]; then git -c protocol.version=2 fetch --deepen=100 "git@github.com:$base_clone_url" From a298d4229b4a8e4dc005ea47f318ee3f0e2f7d32 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Sat, 5 Dec 2020 16:32:43 -0800 Subject: [PATCH 11/11] gha's syntax for git remotes is whack. --- .github/scripts/git-get-shared-history.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/scripts/git-get-shared-history.sh b/.github/scripts/git-get-shared-history.sh index 78074e0c8a42..3b508fe315cb 100755 --- a/.github/scripts/git-get-shared-history.sh +++ b/.github/scripts/git-get-shared-history.sh @@ -25,7 +25,7 @@ git -c protocol.version=2 fetch --deepen=100 base_clone_url=$(jq --raw-output '.pull_request.head.repo.clone_url' $GITHUB_EVENT_PATH) # If it is, we need the fork's history, too. if [[ $base_clone_url != "GoogleChrome/lighthouse.git" ]]; then - git -c protocol.version=2 fetch --deepen=100 "git@github.com:$base_clone_url" + git -c protocol.version=2 fetch --deepen=100 "$base_clone_url" fi echo "History is deepened."