Repository: electron/electron (language: cpp)

Issue:
[Upgrades Follow-Up]: Re-enable base url + data url test
In #43454 we've ended up with a single test that was failing on Windows CI: `BrowserWindow module BrowserWindow.loadURL(url) should support base url for data urls`.

This roll has been open and paused for a couple of weeks. In the interest of keeping upgrades rolling, we're opting to disable the test in the short-term. We want to come back and re-enable this test ASAP, but we figure that can be done async to the rest of the upgrade roll process.


Discussion / hints:

Fix patch:
diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md
index 5e14d39513d3c..b6d36b2e70a42 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 `<webview>.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,<script src="loaded-from-dataurl.js"></script>', { 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,<script src="loaded-from-dataurl.js"></script>', { 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 6d3ba73d8fa43..d34d8c6c7c041
--- 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,<script src="loaded-from-dataurl.js"></script>', { baseURLForDataURL: `other://${path.join(fixtures, 'api')}${path.sep}` })
+        .loadURL('data:text/html,<script src="loaded-from-dataurl.js"></script>', { 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 8b846aa0b6283..bfd5a1f4c1159 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 () => {


Test patch: