diff --git a/.github/manual-test b/.github/manual-test index 1bf5ab7..db2dca4 100755 --- a/.github/manual-test +++ b/.github/manual-test @@ -9,9 +9,11 @@ if ! command -v node > /dev/null; then echo "Missing node"; missingDeps=true; fi if [ "$missingDeps" = true ]; then exit 1; fi function launch_app() { - printf '\n*** Running appn' + printf '\n*** Running app\n' if [ "$(uname -s)" = "Darwin" ]; then open -a "$1/$2-darwin-x64/$2.app" + elif [ "$(uname -o)" = "Msys" ]; then + "$1/$2-win32-x64/$2.exe" --verbose else "$1/$2-linux-x64/$2" fi diff --git a/app/package.json b/app/package.json index fb4a08e..cbb18a2 100644 --- a/app/package.json +++ b/app/package.json @@ -20,6 +20,6 @@ "source-map-support": "^0.5.19" }, "devDependencies": { - "electron": "^12.0.7" + "electron": "^12.0.11" } } diff --git a/app/src/helpers/windowHelpers.test.ts b/app/src/helpers/windowHelpers.test.ts index ee12665..0ec7327 100644 --- a/app/src/helpers/windowHelpers.test.ts +++ b/app/src/helpers/windowHelpers.test.ts @@ -97,6 +97,8 @@ describe('createNewTab', () => { }); describe('injectCSS', () => { + jest.setTimeout(10000); + const mockGetCSSToInject: jest.SpyInstance = getCSSToInject as jest.Mock; const mockLogError: jest.SpyInstance = error as jest.Mock; const mockWebContentsInsertCSS: jest.SpyInstance = jest.spyOn( @@ -105,12 +107,13 @@ describe('injectCSS', () => { ); const css = 'body { color: white; }'; - const responseHeaders = { 'x-header': 'value' }; + let responseHeaders; beforeEach(() => { mockGetCSSToInject.mockReset().mockReturnValue(''); mockLogError.mockReset(); mockWebContentsInsertCSS.mockReset().mockResolvedValue(undefined); + responseHeaders = { 'x-header': 'value', 'content-type': ['test/other'] }; }); afterAll(() => { @@ -181,22 +184,34 @@ describe('injectCSS', () => { ); }); - test.each(['DELETE', 'OPTIONS'])( - 'will not inject for method %s', - (method: string, done: jest.DoneCallback) => { + test.each([ + 'application/json', + 'font/woff2', + 'image/png', + ])( + 'will not inject for content-type %s', + (contentType: string, done: jest.DoneCallback) => { mockGetCSSToInject.mockReturnValue(css); const window = new BrowserWindow(); + responseHeaders['content-type'] = [contentType]; + injectCSS(window); expect(mockGetCSSToInject).toHaveBeenCalled(); - window.webContents.emit('did-navigate'); + expect(window.webContents.emit('did-navigate')).toBe(true); + mockWebContentsInsertCSS.mockReset().mockResolvedValue(undefined); // @ts-ignore this function doesn't exist in the actual electron version, but will in our mock window.webContents.session.webRequest.send( 'onHeadersReceived', - { responseHeaders, webContents: window.webContents, method }, + { + responseHeaders, + webContents: window.webContents, + url: `test-${contentType}`, + }, (result: HeadersReceivedResponse) => { + // insertCSS will still run once for the did-navigate expect(mockWebContentsInsertCSS).not.toHaveBeenCalled(); expect(result.cancel).toBe(false); expect(result.responseHeaders).toBe(responseHeaders); @@ -206,23 +221,30 @@ describe('injectCSS', () => { }, ); - test.each(['GET', 'PATCH', 'POST', 'PUT'])( - 'will inject for method %s', - (method: string, done: jest.DoneCallback) => { + test.each(['text/html'])( + 'will inject for content-type %s', + (contentType: string, done: jest.DoneCallback) => { mockGetCSSToInject.mockReturnValue(css); const window = new BrowserWindow(); + responseHeaders['content-type'] = [contentType]; + injectCSS(window); expect(mockGetCSSToInject).toHaveBeenCalled(); window.webContents.emit('did-navigate'); + mockWebContentsInsertCSS.mockReset().mockResolvedValue(undefined); // @ts-ignore this function doesn't exist in the actual electron version, but will in our mock window.webContents.session.webRequest.send( 'onHeadersReceived', - { responseHeaders, webContents: window.webContents, method }, + { + responseHeaders, + webContents: window.webContents, + url: `test-${contentType}`, + }, (result: HeadersReceivedResponse) => { - expect(mockWebContentsInsertCSS).toHaveBeenCalled(); + expect(mockWebContentsInsertCSS).toHaveBeenCalledTimes(1); expect(result.cancel).toBe(false); expect(result.responseHeaders).toBe(responseHeaders); done(); @@ -247,11 +269,18 @@ describe('injectCSS', () => { expect(mockGetCSSToInject).toHaveBeenCalled(); window.webContents.emit('did-navigate'); + mockWebContentsInsertCSS.mockReset().mockResolvedValue(undefined); // @ts-ignore this function doesn't exist in the actual electron version, but will in our mock window.webContents.session.webRequest.send( 'onHeadersReceived', - { responseHeaders, webContents: window.webContents, resourceType }, + { + responseHeaders, + webContents: window.webContents, + resourceType, + url: `test-${resourceType}`, + }, (result: HeadersReceivedResponse) => { + // insertCSS will still run once for the did-navigate expect(mockWebContentsInsertCSS).not.toHaveBeenCalled(); expect(result.cancel).toBe(false); expect(result.responseHeaders).toBe(responseHeaders); @@ -272,12 +301,18 @@ describe('injectCSS', () => { expect(mockGetCSSToInject).toHaveBeenCalled(); window.webContents.emit('did-navigate'); + mockWebContentsInsertCSS.mockReset().mockResolvedValue(undefined); // @ts-ignore this function doesn't exist in the actual electron version, but will in our mock window.webContents.session.webRequest.send( 'onHeadersReceived', - { responseHeaders, webContents: window.webContents, resourceType }, + { + responseHeaders, + webContents: window.webContents, + resourceType, + url: `test-${resourceType}`, + }, (result: HeadersReceivedResponse) => { - expect(mockWebContentsInsertCSS).toHaveBeenCalled(); + expect(mockWebContentsInsertCSS).toHaveBeenCalledTimes(1); expect(result.cancel).toBe(false); expect(result.responseHeaders).toBe(responseHeaders); done(); diff --git a/app/src/helpers/windowHelpers.ts b/app/src/helpers/windowHelpers.ts index cc42e24..b0eeefd 100644 --- a/app/src/helpers/windowHelpers.ts +++ b/app/src/helpers/windowHelpers.ts @@ -204,6 +204,13 @@ export function injectCSS(browserWindow: BrowserWindow): void { 'browserWindow.webContents.did-navigate', browserWindow.webContents.getURL(), ); + + browserWindow.webContents + .insertCSS(cssToInject) + .catch((err: unknown) => + log.error('browserWindow.webContents.insertCSS', err), + ); + // We must inject css early enough; so onHeadersReceived is a good place. // Will run multiple times, see `did-finish-load` event on the window // that unsets this handler. @@ -213,7 +220,18 @@ export function injectCSS(browserWindow: BrowserWindow): void { details: OnHeadersReceivedListenerDetails, callback: (headersReceivedResponse: HeadersReceivedResponse) => void, ) => { - injectCSSIntoResponse(details, cssToInject) + const contentType = + 'content-type' in details.responseHeaders + ? details.responseHeaders['content-type'][0] + : undefined; + + log.debug('onHeadersReceived', { + contentType, + resourceType: details.resourceType, + url: details.url, + }); + + injectCSSIntoResponse(details, contentType, cssToInject) .then((responseHeaders) => { callback({ cancel: false, @@ -234,28 +252,33 @@ export function injectCSS(browserWindow: BrowserWindow): void { async function injectCSSIntoResponse( details: OnHeadersReceivedListenerDetails, + contentType: string, cssToInject: string, ): Promise> { - // We go with a denylist rather than a whitelist (e.g. only GET/html) + // We go with a denylist rather than a whitelist (e.g. only text/html) // to avoid "whoops I didn't think this should have been CSS-injected" cases - const nonInjectableMethods = ['DELETE', 'OPTIONS']; + const nonInjectableContentTypes = [ + /application\/.*/, + /font\/.*/, + /image\/.*/, + ]; const nonInjectableResourceTypes = ['image', 'script', 'stylesheet', 'xhr']; if ( - nonInjectableMethods.includes(details.method) || + nonInjectableContentTypes.filter((x) => x.exec(contentType)?.length > 0) + ?.length > 0 || nonInjectableResourceTypes.includes(details.resourceType) || !details.webContents ) { log.debug( - `Skipping CSS injection for:\n${details.url}\nwith method ${details.method} and resourceType ${details.resourceType} and content-type ${details.responseHeaders['content-type']}`, + `Skipping CSS injection for:\n${details.url}\nwith resourceType ${details.resourceType} and content-type ${contentType}`, ); return details.responseHeaders; } - log.debug('browserWindow.webContents.session.webRequest.onHeadersReceived', { - details, - contentType: details.responseHeaders['content-type'], - }); + log.debug( + `Injecting CSS for:\n${details.url}\nwith resourceType ${details.resourceType} and content-type ${contentType}`, + ); await details.webContents.insertCSS(cssToInject); return details.responseHeaders;