diff --git a/.changeset/breezy-hairs-greet.md b/.changeset/breezy-hairs-greet.md new file mode 100644 index 000000000000..73d7130d7ec0 --- /dev/null +++ b/.changeset/breezy-hairs-greet.md @@ -0,0 +1,5 @@ +--- +"@sveltejs/kit": patch +--- + +reset selection in setTimeout after navigating, to ensure correct behaviour in Firefox diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e609cdeb081a..65f07dda6887 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,9 +10,17 @@ env: TURBO_TOKEN: ${{ secrets.TURBO_TOKEN }} TURBO_TEAM: svelte +# cancel in-progress runs on new commits to same PR (gitub.event.number) +concurrency: + group: ${{ github.workflow }}-${{ github.event.number || github.sha }} + cancel-in-progress: true + jobs: Lint: runs-on: ubuntu-latest + env: + # not needed for linting + PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: "1" steps: - uses: actions/checkout@v3 - uses: pnpm/action-setup@v2.2.2 @@ -30,12 +38,23 @@ jobs: fail-fast: false matrix: node-version: [16] - os: [ubuntu-latest, macOS-latest, windows-2019] + os: [ubuntu-latest , windows-2019] + e2e-browser: ['chromium'] include: + - node-version: 16 + os: ubuntu-latest + e2e-browser: 'firefox' + - node-version: 16 + os: macOS-latest + e2e-browser: 'safari' - node-version: 18 os: ubuntu-latest + e2e-browser: 'chromium' env: TURBO_CACHE_KEY: ${{ matrix.os }}-${{ matrix.node-version }} + # Install playwright's binray under node_modules so it will be cached together + PLAYWRIGHT_BROWSERS_PATH: "0" + KIT_E2E_BROWSER: ${{matrix.e2e-browser}} steps: - run: git config --global core.autocrlf false - uses: actions/checkout@v3 @@ -45,15 +64,16 @@ jobs: node-version: ${{ matrix.node-version }} cache: pnpm - run: pnpm install --frozen-lockfile + - run: pnpm playwright install - run: pnpm test - - name: archive test results + - name: Archive test results if: failure() shell: bash - run: find packages -type d -name test-results -not -empty | tar -czf test-results.tar.gz --exclude='*-retry*' --files-from=- - - name: Upload failed tests screenshots + run: find packages -type d -name test-results -not -empty | tar -czf test-results.tar.gz --files-from=- + - name: Upload test results if: failure() uses: actions/upload-artifact@v3 with: retention-days: 3 - name: test-failure-${{ github.run_id }}-${{ matrix.os }}-${{ matrix.node-version }} + name: test-failure-${{ github.run_id }}-${{ matrix.os }}-${{ matrix.node-version }}-${{ matrix.e2e-browser }} path: test-results.tar.gz diff --git a/package.json b/package.json index 6f3820c32e75..455b60197bd9 100644 --- a/package.json +++ b/package.json @@ -35,6 +35,7 @@ "eslint-plugin-import": "^2.26.0", "eslint-plugin-node": "^11.1.0", "eslint-plugin-svelte3": "^4.0.0", + "playwright": "^1.22.2", "prettier": "^2.6.2", "rollup": "^2.75.3", "svelte": "^3.48.0", diff --git a/packages/adapter-static/test/utils.js b/packages/adapter-static/test/utils.js index ec85731dd40f..f46805e30707 100644 --- a/packages/adapter-static/test/utils.js +++ b/packages/adapter-static/test/utils.js @@ -50,11 +50,7 @@ export function run(app, callback) { context.server = await create_server(context.port, handler); context.base = `http://localhost:${context.port}`; - context.browser = await chromium.launch({ - // use stable chrome from host OS instead of downloading one - // see https://playwright.dev/docs/browsers#google-chrome--microsoft-edge - channel: 'chrome' - }); + context.browser = await chromium.launch(); context.page = await context.browser.newPage(); } catch (e) { // TODO remove unnecessary try-catch https://github.com/lukeed/uvu/pull/61 diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index b844f637f464..5ca0509438d6 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -304,10 +304,13 @@ export function create_client({ target, session, base, trailing_slash }) { const root = document.body; const tabindex = root.getAttribute('tabindex'); - getSelection()?.removeAllRanges(); root.tabIndex = -1; root.focus({ preventScroll: true }); + setTimeout(() => { + getSelection()?.removeAllRanges(); + }); + // restore `tabindex` as to prevent `root` from stealing input from elements if (tabindex !== null) { root.setAttribute('tabindex', tabindex); diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 8d1b4baaccd4..655aee1ec231 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -74,7 +74,7 @@ test.describe('a11y', () => { selection.addRange(range); return selection.rangeCount; } - return 0; + return -1; }) ).toBe(1); @@ -85,7 +85,7 @@ test.describe('a11y', () => { if (selection) { return selection.rangeCount; } - return 1; + return -1; }) ).toBe(0); }); @@ -139,7 +139,7 @@ test.describe('beforeNavigate', () => { test('prevents unload', async ({ page }) => { await page.goto('/before-navigate/prevent-navigation'); - + await page.click('h1'); // The browsers block attempts to prevent navigation on a frame that's never had a user gesture. const type = new Promise((fulfil) => { page.on('dialog', async (dialog) => { fulfil(dialog.type()); @@ -205,17 +205,12 @@ test.describe('Scrolling', () => { expect(await page.evaluate(() => scrollY === 0)).toBeTruthy(); }); - test('scroll is restored after hitting the back button', async ({ - back, - baseURL, - clicknav, - page - }) => { + test('scroll is restored after hitting the back button', async ({ baseURL, clicknav, page }) => { await page.goto('/anchor'); await page.click('#scroll-anchor'); const originalScrollY = /** @type {number} */ (await page.evaluate(() => scrollY)); await clicknav('#routing-page'); - await back(); + await page.goBack(); expect(page.url()).toBe(baseURL + '/anchor#last-anchor-2'); expect(await page.evaluate(() => scrollY)).toEqual(originalScrollY); @@ -226,13 +221,15 @@ test.describe('Scrolling', () => { test('scroll is restored after hitting the back button for an in-app cross-document navigation', async ({ page, - clicknav, - back + clicknav }) => { await page.goto('/scroll/cross-document/a'); - await page.locator('[href="/scroll/cross-document/b"]').scrollIntoViewIfNeeded(); - const y1 = await page.evaluate(() => scrollY); + const rect = await page.locator('[href="/scroll/cross-document/b"]').boundingBox(); + const height = await page.evaluate(() => innerHeight); + + const target_scroll_y = rect.y + rect.height - height; + await page.evaluate((y) => scrollTo(0, y), target_scroll_y); await page.click('[href="/scroll/cross-document/b"]'); expect(await page.textContent('h1')).toBe('b'); @@ -241,16 +238,16 @@ test.describe('Scrolling', () => { await clicknav('[href="/scroll/cross-document/c"]'); expect(await page.textContent('h1')).toBe('c'); - await back(); // client-side back + await page.goBack(); // client-side back await page.goBack(); // native back expect(await page.textContent('h1')).toBe('a'); await page.waitForSelector('body.started'); await page.waitForTimeout(250); // needed for the test to fail reliably without the fix - const y2 = await page.evaluate(() => scrollY); + const scroll_y = await page.evaluate(() => scrollY); - expect(Math.abs(y2 - y1)).toBeLessThan(10); // we need a few pixels wiggle room, because browsers + expect(Math.abs(scroll_y - target_scroll_y)).toBeLessThan(50); // we need a few pixels wiggle room, because browsers }); test('url-supplied anchor is ignored with onMount() scrolling on direct page load', async ({ @@ -302,7 +299,7 @@ test.describe('Scrolling', () => { await expect(page.locator('input')).toBeFocused(); }); - test('scroll positions are recovered on reloading the page', async ({ page, back, app }) => { + test('scroll positions are recovered on reloading the page', async ({ page, app }) => { await page.goto('/anchor'); await page.evaluate(() => window.scrollTo(0, 1000)); await app.goto('/anchor/anchor'); @@ -311,7 +308,7 @@ test.describe('Scrolling', () => { await page.reload(); expect(await page.evaluate(() => window.scrollY)).toBe(1000); - await back(); + await page.goBack(); expect(await page.evaluate(() => window.scrollY)).toBe(1000); }); @@ -1910,7 +1907,7 @@ test.describe('searchParams', () => { }); test.describe('Redirects', () => { - test('redirect', async ({ baseURL, page, clicknav, back }) => { + test('redirect', async ({ baseURL, page, clicknav }) => { await page.goto('/redirect'); await clicknav('[href="/redirect/a"]'); @@ -1919,7 +1916,7 @@ test.describe('Redirects', () => { expect(await page.textContent('h1')).toBe('c'); expect(page.url()).toBe(`${baseURL}/redirect/c`); - await back(); + await page.goBack(); expect(page.url()).toBe(`${baseURL}/redirect`); }); @@ -1937,10 +1934,10 @@ test.describe('Redirects', () => { ); } else { // there's not a lot we can do to handle server-side redirect loops - if (browserName === 'webkit') { - expect(page.url()).toBe(`${baseURL}/redirect`); - } else { + if (browserName === 'chromium') { expect(page.url()).toBe('chrome-error://chromewebdata/'); + } else { + expect(page.url()).toBe(`${baseURL}/redirect`); } } }); @@ -2260,17 +2257,16 @@ test.describe('Routing', () => { expect(await page.textContent('h1')).toBe('y/1'); }); - test('back button returns to initial route', async ({ page, clicknav, back }) => { + test('back button returns to initial route', async ({ page, clicknav }) => { await page.goto('/routing'); await clicknav('[href="/routing/a"]'); - await back(); + await page.goBack(); expect(await page.textContent('h1')).toBe('Great success!'); }); test('back button returns to previous route when previous route has been navigated to via hash anchor', async ({ page, - back, clicknav }) => { await page.goto('/routing/hashes/a'); @@ -2278,7 +2274,7 @@ test.describe('Routing', () => { await page.click('[href="#hash-target"]'); await clicknav('[href="/routing/hashes/b"]'); - await back(); + await page.goBack(); expect(await page.textContent('h1')).toBe('a'); }); @@ -2286,9 +2282,12 @@ test.describe('Routing', () => { await page.goto('/routing/hashes/target#p2'); await page.keyboard.press(browserName === 'webkit' ? 'Alt+Tab' : 'Tab'); - expect(await page.evaluate(() => (document.activeElement || {}).textContent)).toBe( - 'next focus element' - ); + await page.waitForTimeout(50); // give browser a bit of time to complete the native behavior of the key press + expect( + await page.evaluate( + () => document.activeElement?.textContent || 'ERROR: document.activeElement not set' + ) + ).toBe('next focus element'); }); test('focus works when navigating to a hash on the same page', async ({ page, browserName }) => { @@ -2414,7 +2413,7 @@ test.describe('Routing', () => { expect(await page.textContent('body')).toBe('xyz/abc/qwe'); }); - test('rest parameters do not swallow characters', async ({ page, clicknav, back }) => { + test('rest parameters do not swallow characters', async ({ page, clicknav }) => { await page.goto('/routing/rest/non-greedy'); await clicknav('[href="/routing/rest/non-greedy/foo/one/two"]'); @@ -2424,7 +2423,7 @@ test.describe('Routing', () => { await clicknav('[href="/routing/rest/non-greedy/food/one/two"]'); expect(await page.textContent('h1')).not.toBe('non-greedy'); - await back(); + await page.goBack(); await clicknav('[href="/routing/rest/non-greedy/one-bar/two/three"]'); expect(await page.textContent('h1')).toBe('non-greedy'); diff --git a/packages/kit/test/utils.d.ts b/packages/kit/test/utils.d.ts index 050b525bd86f..abb0004e864d 100644 --- a/packages/kit/test/utils.d.ts +++ b/packages/kit/test/utils.d.ts @@ -20,7 +20,6 @@ export const test: TestType< prefetch: (url: string) => Promise; prefetchRoutes: (urls: string[]) => Promise; }; - back: () => Promise; clicknav: (selector: string, options?: { timeout?: number }) => Promise; in_view: (selector: string) => Promise; read_errors: (href: string) => string; diff --git a/packages/kit/test/utils.js b/packages/kit/test/utils.js index 365a0f8da1a1..086d27228e1a 100644 --- a/packages/kit/test/utils.js +++ b/packages/kit/test/utils.js @@ -1,7 +1,7 @@ import fs from 'fs'; import http from 'http'; import * as ports from 'port-authority'; -import { test as base } from '@playwright/test'; +import { test as base, devices } from '@playwright/test'; export const test = base.extend({ // @ts-expect-error @@ -54,17 +54,6 @@ export const test = base.extend({ }); }, - // @ts-expect-error - back: async ({ page, javaScriptEnabled }, use) => { - use(async () => { - if (javaScriptEnabled) { - await Promise.all([page.goBack(), page.evaluate(() => window.navigated)]); - } else { - return page.goBack().then(() => void 0); - } - }); - }, - // @ts-expect-error clicknav: async ({ page, javaScriptEnabled }, use) => { /** @@ -105,19 +94,22 @@ export const test = base.extend({ }); } - const goto = page.goto; - page.goto = - /** - * @param {string} url - * @param {object} opts - */ - async function (url, opts) { - const res = await goto.call(page, url, opts); + // automatically wait for kit started event after navigation functions if js is enabled + const page_navigation_functions = ['goto', 'goBack', 'reload']; + page_navigation_functions.forEach((fn) => { + const page_fn = page[fn]; + if (!page_fn) { + throw new Error(`function does not exist on page: ${fn}`); + } + page[fn] = async function (...args) { + const res = await page_fn.call(page, ...args); if (javaScriptEnabled) { await page.waitForSelector('body.started', { timeout: 5000 }); } return res; }; + }); + await use(page); }, @@ -135,6 +127,22 @@ export const test = base.extend({ use(read_errors); } }); +const test_browser = process.env.KIT_E2E_BROWSER ?? 'chromium'; +const known_devices = { + chromium: devices['Desktop Chrome'], + firefox: devices['Desktop Firefox'], + safari: devices['Desktop Safari'] +}; + +const test_browser_device = known_devices[test_browser]; + +if (!test_browser_device) { + throw new Error( + `invalid test browser specified: KIT_E2E_BROWSER=${ + process.env.KIT_E2E_BROWSER + }. Allowed values: ${Object.keys(known_devices).join(', ')}` + ); +} /** @type {import('@playwright/test').PlaywrightTestConfig} */ export const config = { @@ -148,24 +156,22 @@ export const config = { retries: process.env.CI ? 5 : 0, projects: [ { - name: `${process.env.DEV ? 'dev' : 'build'}+js`, + name: `${test_browser}-${process.env.DEV ? 'dev' : 'build'}+js`, use: { javaScriptEnabled: true } }, { - name: `${process.env.DEV ? 'dev' : 'build'}-js`, + name: `${test_browser}-${process.env.DEV ? 'dev' : 'build'}-js`, use: { javaScriptEnabled: false } } ], use: { + ...test_browser_device, screenshot: 'only-on-failure', - trace: 'retain-on-failure', - // use stable chrome from host OS instead of downloading one - // see https://playwright.dev/docs/browsers#google-chrome--microsoft-edge - channel: 'chrome' + trace: process.env.KIT_E2E_TRACE ? 'retain-on-failure' : 'on-first-retry' }, workers: process.env.CI ? 2 : undefined }; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1dd3fc1d92bb..19c5daf47f44 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -16,6 +16,7 @@ importers: eslint-plugin-import: ^2.26.0 eslint-plugin-node: ^11.1.0 eslint-plugin-svelte3: ^4.0.0 + playwright: ^1.22.2 prettier: ^2.6.2 rollup: ^2.75.3 svelte: ^3.48.0 @@ -35,6 +36,7 @@ importers: eslint-plugin-import: 2.26.0_stpfporf4ezjetv6zdnl75453m eslint-plugin-node: 11.1.0_eslint@8.18.0 eslint-plugin-svelte3: 4.0.0_wy4erphnvhealet26qderqv6bu + playwright: 1.23.1 prettier: 2.7.1 rollup: 2.75.3 svelte: 3.48.0 @@ -3941,6 +3943,15 @@ packages: hasBin: true dev: true + /playwright/1.23.1: + resolution: {integrity: sha512-+lgiy1JZMNPwpBp5tsUdjTGxuJ+lXZbtSKmMDc0/f1oVsNDXXA+r7zeC9Kzd+4jSHteoJJocargxVx2Mn1o2kw==} + engines: {node: '>=14'} + hasBin: true + requiresBuild: true + dependencies: + playwright-core: 1.23.1 + dev: true + /polka/1.0.0-next.22: resolution: {integrity: sha512-a7tsZy5gFbJr0aUltZS97xCkbPglXuD67AMvTyZX7BTDBH384FWf0ZQF6rPvdutSxnO1vUlXM2zSLf5tCKk5RA==} engines: {node: '>=8'}