From 6356113abb8caa67ba34c9fb4f85cbb3faf1771e Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 6 Aug 2024 13:27:59 -0600 Subject: [PATCH 1/7] Import base64 utils directly from js-sdk See comments in code --- src/utils/tokens/pickling.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/utils/tokens/pickling.ts b/src/utils/tokens/pickling.ts index 9e096bedef4..591fcacf7ef 100644 --- a/src/utils/tokens/pickling.ts +++ b/src/utils/tokens/pickling.ts @@ -17,7 +17,12 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { encodeUnpaddedBase64 } from "matrix-js-sdk/src/matrix"; +// Note: we don't import the base64 utils from `matrix-js-sdk/src/matrix` because this file +// is used by Element Web's service worker, and importing `matrix` brings in ~1mb of stuff +// we don't need. Instead, we ignore the import restriction and only bring in what we actually +// need. +// eslint-disable-next-line no-restricted-imports +import { encodeUnpaddedBase64 } from "matrix-js-sdk/src/base64"; import { logger } from "matrix-js-sdk/src/logger"; /** From 835806d253106b36f337e6387e48d740cc8fb1f2 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 6 Aug 2024 14:14:22 -0600 Subject: [PATCH 2/7] Use the authenticated routes (because the service worker said so) --- playwright/e2e/timeline/timeline.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/playwright/e2e/timeline/timeline.spec.ts b/playwright/e2e/timeline/timeline.spec.ts index 2bd021e5736..40d3df9ab49 100644 --- a/playwright/e2e/timeline/timeline.spec.ts +++ b/playwright/e2e/timeline/timeline.spec.ts @@ -724,7 +724,7 @@ test.describe("Timeline", () => { axe.disableRules("color-contrast"); await page.route( - "**/_matrix/media/v3/thumbnail/matrix.org/2022-08-16_yaiSVSRIsNFfxDnV?*", + "**/_matrix/client/v1/media/thumbnail/matrix.org/2022-08-16_yaiSVSRIsNFfxDnV?*", async (route) => { await route.fulfill({ path: "playwright/sample-files/riot.png", @@ -750,7 +750,7 @@ test.describe("Timeline", () => { const requestPromises: Promise[] = [ page.waitForResponse("**/_matrix/media/v3/preview_url?url=https%3A%2F%2Fcall.element.io%2F&ts=*"), - page.waitForResponse("**/_matrix/media/v3/thumbnail/matrix.org/2022-08-16_yaiSVSRIsNFfxDnV?*"), + page.waitForResponse("**/_matrix/client/v1/media/thumbnail/matrix.org/2022-08-16_yaiSVSRIsNFfxDnV?*"), ]; await app.client.sendMessage(room.roomId, "https://call.element.io/"); From 8003cf8545cbe15102a4704ed7337fc82d1e0c66 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 6 Aug 2024 14:57:00 -0600 Subject: [PATCH 3/7] Revert "Use the authenticated routes (because the service worker said so)" This reverts commit 835806d253106b36f337e6387e48d740cc8fb1f2. --- playwright/e2e/timeline/timeline.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/playwright/e2e/timeline/timeline.spec.ts b/playwright/e2e/timeline/timeline.spec.ts index 40d3df9ab49..2bd021e5736 100644 --- a/playwright/e2e/timeline/timeline.spec.ts +++ b/playwright/e2e/timeline/timeline.spec.ts @@ -724,7 +724,7 @@ test.describe("Timeline", () => { axe.disableRules("color-contrast"); await page.route( - "**/_matrix/client/v1/media/thumbnail/matrix.org/2022-08-16_yaiSVSRIsNFfxDnV?*", + "**/_matrix/media/v3/thumbnail/matrix.org/2022-08-16_yaiSVSRIsNFfxDnV?*", async (route) => { await route.fulfill({ path: "playwright/sample-files/riot.png", @@ -750,7 +750,7 @@ test.describe("Timeline", () => { const requestPromises: Promise[] = [ page.waitForResponse("**/_matrix/media/v3/preview_url?url=https%3A%2F%2Fcall.element.io%2F&ts=*"), - page.waitForResponse("**/_matrix/client/v1/media/thumbnail/matrix.org/2022-08-16_yaiSVSRIsNFfxDnV?*"), + page.waitForResponse("**/_matrix/media/v3/thumbnail/matrix.org/2022-08-16_yaiSVSRIsNFfxDnV?*"), ]; await app.client.sendMessage(room.roomId, "https://call.element.io/"); From 400d7fbba30b3029db3a14ecb44bc718dcd115a3 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 6 Aug 2024 15:21:36 -0600 Subject: [PATCH 4/7] Use the authenticated routes (because the service worker said so) --- playwright/e2e/timeline/timeline.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/playwright/e2e/timeline/timeline.spec.ts b/playwright/e2e/timeline/timeline.spec.ts index 2bd021e5736..e88b3b89eec 100644 --- a/playwright/e2e/timeline/timeline.spec.ts +++ b/playwright/e2e/timeline/timeline.spec.ts @@ -724,7 +724,7 @@ test.describe("Timeline", () => { axe.disableRules("color-contrast"); await page.route( - "**/_matrix/media/v3/thumbnail/matrix.org/2022-08-16_yaiSVSRIsNFfxDnV?*", + "**/_matrix/client/v1/media/thumbnail/matrix.org/2022-08-16_yaiSVSRIsNFfxDnV?*", async (route) => { await route.fulfill({ path: "playwright/sample-files/riot.png", From d05417a29582482265f7ee5b50bff4bccd75c62d Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 6 Aug 2024 16:26:01 -0600 Subject: [PATCH 5/7] Continue fighting Playwright --- playwright/e2e/timeline/timeline.spec.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/playwright/e2e/timeline/timeline.spec.ts b/playwright/e2e/timeline/timeline.spec.ts index e88b3b89eec..d311548c6b9 100644 --- a/playwright/e2e/timeline/timeline.spec.ts +++ b/playwright/e2e/timeline/timeline.spec.ts @@ -720,10 +720,15 @@ test.describe("Timeline", () => { ).toBeVisible(); }); - test("should render url previews", async ({ page, app, room, axe, checkA11y }) => { + test("should render url previews", async ({ page, app, room, axe, checkA11y, context }) => { axe.disableRules("color-contrast"); - await page.route( + // Element Web uses a Service Worker to rewrite unauthenticated media requests to authenticated ones, but + // the page can't see this happening. We intercept the route at the BrowserContext to ensure we get it + // post-worker, but we can't waitForResponse on that, so the page context is still used there. Because + // the page doesn't see the rewrite, it waits for the unauthenticated route. This is only confusing until + // the js-sdk (and thus the app as a whole) switches to using authenticated endpoints by default, hopefully. + await context.route( "**/_matrix/client/v1/media/thumbnail/matrix.org/2022-08-16_yaiSVSRIsNFfxDnV?*", async (route) => { await route.fulfill({ @@ -750,6 +755,7 @@ test.describe("Timeline", () => { const requestPromises: Promise[] = [ page.waitForResponse("**/_matrix/media/v3/preview_url?url=https%3A%2F%2Fcall.element.io%2F&ts=*"), + // see page.route above for why we listen for the unauthenticated endpoint page.waitForResponse("**/_matrix/media/v3/thumbnail/matrix.org/2022-08-16_yaiSVSRIsNFfxDnV?*"), ]; From a2a1ae1e30c4dae3ad9e399a292f90b27cbfe34a Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 6 Aug 2024 16:26:18 -0600 Subject: [PATCH 6/7] Document who is at fault if the import breaks (it's us) --- src/utils/tokens/pickling.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/utils/tokens/pickling.ts b/src/utils/tokens/pickling.ts index 591fcacf7ef..f1739145a06 100644 --- a/src/utils/tokens/pickling.ts +++ b/src/utils/tokens/pickling.ts @@ -21,6 +21,9 @@ limitations under the License. // is used by Element Web's service worker, and importing `matrix` brings in ~1mb of stuff // we don't need. Instead, we ignore the import restriction and only bring in what we actually // need. +// Note: `base64` is not public in the js-sdk, so if it changes/breaks, that's on us. We should +// be okay with our frequent tests, locked versioning, etc though. We'll pick up problems well +// before release. // eslint-disable-next-line no-restricted-imports import { encodeUnpaddedBase64 } from "matrix-js-sdk/src/base64"; import { logger } from "matrix-js-sdk/src/logger"; From 032ed466e5a1561a06e204b9e51b8778778086c5 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 6 Aug 2024 22:19:26 -0600 Subject: [PATCH 7/7] Update playwright/e2e/timeline/timeline.spec.ts Co-authored-by: Robin --- playwright/e2e/timeline/timeline.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/playwright/e2e/timeline/timeline.spec.ts b/playwright/e2e/timeline/timeline.spec.ts index d311548c6b9..b00179c0c0a 100644 --- a/playwright/e2e/timeline/timeline.spec.ts +++ b/playwright/e2e/timeline/timeline.spec.ts @@ -755,7 +755,7 @@ test.describe("Timeline", () => { const requestPromises: Promise[] = [ page.waitForResponse("**/_matrix/media/v3/preview_url?url=https%3A%2F%2Fcall.element.io%2F&ts=*"), - // see page.route above for why we listen for the unauthenticated endpoint + // see context.route above for why we listen for the unauthenticated endpoint page.waitForResponse("**/_matrix/media/v3/thumbnail/matrix.org/2022-08-16_yaiSVSRIsNFfxDnV?*"), ];