From 40cae71df88156036fecc36008c86ee4639c3d9e Mon Sep 17 00:00:00 2001 From: John Kleinschmidt Date: Thu, 26 Sep 2024 08:53:27 -0400 Subject: [PATCH] test: re-enable tests that were disabled in chromium rolls (#43968) * test: fix should support base url for data urls test Caused by https://chromium-review.googlesource.com/c/chromium/src/+/5802682 * test: fixup extensions can cancel http requests * chore: document custom protocol handling on Windows change due to Non-Special Scheme URLs shipping https://chromium-review.googlesource.com/c/chromium/src/+/5802682 --- docs/breaking-changes.md | 27 +++++++++++++++++++++++++++ spec/api-browser-window-spec.ts | 14 ++++++++------ spec/extensions-spec.ts | 18 ++++++++++++------ 3 files changed, 47 insertions(+), 12 deletions(-) mode change 100644 => 100755 spec/api-browser-window-spec.ts diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index 5e14d39513d3..b6d36b2e70a4 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -14,6 +14,33 @@ This document uses the following convention to categorize breaking changes: ## Planned Breaking API Changes (33.0) +### Behavior Changed: custom protocol URL handling on Windows + +Due to changes made in Chromium to support [Non-Special Scheme URLs](http://bit.ly/url-non-special), custom protocol URLs that use Windows file paths will no longer work correctly with the deprecated `protocol.registerFileProtocol` and the `baseURLForDataURL` property on `BrowserWindow.loadURL`, `WebContents.loadURL`, and `.loadURL`. `protocol.handle` will also not work with these types of URLs but this is not a change since it has always worked that way. + +```js +// No longer works +protocol.registerFileProtocol('other', () => { + callback({ filePath: '/path/to/my/file' }) +}) + +const mainWindow = new BrowserWindow() +mainWindow.loadURL('data:text/html,', { baseURLForDataURL: 'other://C:\\myapp' }) +mainWindow.loadURL('other://C:\\myapp\\index.html') + +// Replace with +const path = require('node:path') +const nodeUrl = require('node:url') +protocol.handle(other, (req) => { + const srcPath = 'C:\\myapp\\' + const reqURL = new URL(req.url) + return net.fetch(nodeUrl.pathToFileURL(path.join(srcPath, reqURL.pathname)).toString()) +}) + +mainWindow.loadURL('data:text/html,', { baseURLForDataURL: 'other://' }) +mainWindow.loadURL('other://index.html') +``` + ### Behavior Changed: menu bar will be hidden during fullscreen on Windows This brings the behavior to parity with Linux. Prior behavior: Menu bar is still visible during fullscreen on Windows. New behavior: Menu bar is hidden during fullscreen on Windows. diff --git a/spec/api-browser-window-spec.ts b/spec/api-browser-window-spec.ts old mode 100644 new mode 100755 index 158e90ef6e7f..06b63046fb59 --- a/spec/api-browser-window-spec.ts +++ b/spec/api-browser-window-spec.ts @@ -4,9 +4,10 @@ import * as path from 'node:path'; import * as fs from 'node:fs'; import * as qs from 'node:querystring'; import * as http from 'node:http'; +import * as nodeUrl from 'node:url'; import * as os from 'node:os'; import { AddressInfo } from 'node:net'; -import { app, BrowserWindow, BrowserView, dialog, ipcMain, OnBeforeSendHeadersListenerDetails, protocol, screen, webContents, webFrameMain, session, WebContents, WebFrameMain } from 'electron/main'; +import { app, BrowserWindow, BrowserView, dialog, ipcMain, OnBeforeSendHeadersListenerDetails, net, protocol, screen, webContents, webFrameMain, session, WebContents, WebFrameMain } from 'electron/main'; import { emittedUntil, emittedNTimes } from './lib/events-helpers'; import { ifit, ifdescribe, defer, listen } from './lib/spec-helpers'; @@ -296,15 +297,16 @@ describe('BrowserWindow module', () => { describe('BrowserWindow.loadURL(url)', () => { let w: BrowserWindow; const scheme = 'other'; - const srcPath = path.join(fixtures, 'api', 'loaded-from-dataurl.js'); + const srcPath = path.join(fixtures, 'api'); before(() => { - protocol.registerFileProtocol(scheme, (request, callback) => { - callback(srcPath); + protocol.handle(scheme, (req) => { + const reqURL = new URL(req.url); + return net.fetch(nodeUrl.pathToFileURL(path.join(srcPath, reqURL.pathname)).toString()); }); }); after(() => { - protocol.unregisterProtocol(scheme); + protocol.unhandle(scheme); }); beforeEach(() => { @@ -496,7 +498,7 @@ describe('BrowserWindow module', () => { // FIXME(#43730): fix underlying bug and re-enable asap it.skip('should support base url for data urls', async () => { await w - .loadURL('data:text/html,', { baseURLForDataURL: `other://${path.join(fixtures, 'api')}${path.sep}` }) + .loadURL('data:text/html,', { baseURLForDataURL: 'other://' }) .catch((e) => console.log(e)); expect(await w.webContents.executeJavaScript('window.ping')).to.equal('pong'); }); diff --git a/spec/extensions-spec.ts b/spec/extensions-spec.ts index e0e48945cdb4..fdde8341c38b 100644 --- a/spec/extensions-spec.ts +++ b/spec/extensions-spec.ts @@ -6,7 +6,7 @@ import * as path from 'node:path'; import * as fs from 'node:fs/promises'; import * as WebSocket from 'ws'; import { emittedNTimes, emittedUntil } from './lib/events-helpers'; -import { ifit, listen } from './lib/spec-helpers'; +import { ifit, listen, waitUntil } from './lib/spec-helpers'; import { once } from 'node:events'; const uuid = require('uuid'); @@ -355,14 +355,20 @@ describe('chrome extensions', () => { w = new BrowserWindow({ show: false, webPreferences: { session: customSession, sandbox: true, contextIsolation: true } }); }); - // FIXME: these tests do not work as intended. the extension is loaded in the browser, but - // the extension's background page has not yet loaded by the time we check behavior, causing - // race conditions in CI vs local. - describe.skip('onBeforeRequest', () => { + describe('onBeforeRequest', () => { + async function haveRejectedFetch () { + try { + await fetch(w.webContents, url); + } catch (ex: any) { + return ex.message === 'Failed to fetch'; + } + return false; + } + it('can cancel http requests', async () => { await w.loadURL(url); await customSession.loadExtension(path.join(fixtures, 'extensions', 'chrome-webRequest')); - await expect(fetch(w.webContents, url)).to.eventually.be.rejectedWith('Failed to fetch'); + await expect(waitUntil(haveRejectedFetch)).to.eventually.be.fulfilled(); }); it('does not cancel http requests when no extension loaded', async () => {