Skip to content

Commit 8bfa45a

Browse files
bershanskiybershanskiy
authored
Fix: ignore color-scheme messages from subframes (darkreader#11254)
 - In Firefox, subframe media queries may be affected by style color-scheme attribute on iframe - Dark Reader will ignore color-scheme style on iframes (will make them dark, maintaining prior behavior)
1 parent 27246a0 commit 8bfa45a

6 files changed

Lines changed: 144 additions & 11 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
## 4.9.64 (TBD)
22

33
- Use passive event listeners (#11047)
4+
- Fix System Automation on Firefox (#10237)
45

56
## 4.9.63 (Apr 10, 2023)
67

‎src/background/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,7 @@ if (__TEST__) {
213213
break;
214214
}
215215
case 'firefox-emulateColorScheme': {
216+
ASSERT('Firefox-specific function', isFirefox);
216217
emulateColorScheme(message.data);
217218
respond();
218219
break;

‎src/background/tab-manager.ts‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import type {FetchRequestParameters} from './utils/network';
44
import type {Message} from '../definitions';
55
import {isFirefox} from '../utils/platform';
66
import {MessageType} from '../utils/message';
7-
import {logInfo, logWarn} from './utils/log';
7+
import {ASSERT, logInfo, logWarn} from './utils/log';
88
import {StateManager} from '../utils/state-manager';
99
import {getURLHostOrProtocol} from '../utils/url';
1010
import {isPanel} from './utils/tab';
@@ -58,13 +58,15 @@ export default class TabManager {
5858
private static tabs: {[tabId: tabId]: {[frameId: frameId]: DocumentInfo}};
5959
private static stateManager: StateManager<TabManagerState>;
6060
private static fileLoader: {get: (params: FetchRequestParameters) => Promise<string | null>} | null = null;
61-
private static getTabMessage: (tabURL: string, url: string, isTopFrame: boolean) => Message;
61+
private static onColorSchemeChange: TabManagerOptions['onColorSchemeChange'];
62+
private static getTabMessage: TabManagerOptions['getTabMessage'];
6263
private static timestamp = 0;
6364
private static readonly LOCAL_STORAGE_KEY = 'TabManager-state';
6465

6566
public static init({getConnectionMessage, onColorSchemeChange, getTabMessage}: TabManagerOptions): void {
6667
TabManager.stateManager = new StateManager<TabManagerState>(TabManager.LOCAL_STORAGE_KEY, this, {tabs: {}, timestamp: 0}, logWarn);
6768
TabManager.tabs = {};
69+
TabManager.onColorSchemeChange = onColorSchemeChange;
6870
TabManager.getTabMessage = getTabMessage;
6971

7072
chrome.runtime.onMessage.addListener(async (message: Message, sender, sendResponse) => {
@@ -73,7 +75,7 @@ export default class TabManager {
7375
}
7476
switch (message.type) {
7577
case MessageType.CS_FRAME_CONNECT: {
76-
onColorSchemeChange(message.data.isDark);
78+
TabManager.onColorSchemeMessage(message, sender);
7779
await TabManager.stateManager.loadState();
7880
const reply = (tabURL: string, url: string, isTopFrame: boolean) => {
7981
getConnectionMessage(tabURL, url, isTopFrame).then((message) => {
@@ -132,7 +134,7 @@ export default class TabManager {
132134
}
133135

134136
case MessageType.CS_FRAME_RESUME: {
135-
onColorSchemeChange(message.data.isDark);
137+
TabManager.onColorSchemeMessage(message, sender);
136138
await TabManager.stateManager.loadState();
137139
const tabId = sender.tab!.id!;
138140
const tabURL = sender.tab!.url!;
@@ -191,7 +193,7 @@ export default class TabManager {
191193
case MessageType.UI_COLOR_SCHEME_CHANGE:
192194
// fallthrough
193195
case MessageType.CS_COLOR_SCHEME_CHANGE:
194-
onColorSchemeChange(message.data.isDark);
196+
TabManager.onColorSchemeMessage(message, sender);
195197
break;
196198

197199
case MessageType.UI_SAVE_FILE: {
@@ -220,6 +222,18 @@ export default class TabManager {
220222
chrome.tabs.onRemoved.addListener(async (tabId) => TabManager.removeFrame(tabId, 0));
221223
}
222224

225+
private static onColorSchemeMessage(message: Message, sender: chrome.runtime.MessageSender) {
226+
ASSERT('TabManager.onColorSchemeMessage is set', () => Boolean(TabManager.onColorSchemeChange));
227+
228+
// We honor only messages which come from tab's top frame
229+
// because sub-frames color scheme can be overridden by style with prefers-color-scheme
230+
// TODO(MV3): instead of dropping these messages, consider making a query to an authoritative source
231+
// like offscreen document
232+
if (sender && sender.frameId === 0) {
233+
TabManager.onColorSchemeChange(message.data.isDark);
234+
}
235+
}
236+
223237
private static async queryTabs(query: chrome.tabs.QueryInfo = {}) {
224238
return new Promise<chrome.tabs.Tab[]>((resolve) =>
225239
chrome.tabs.query(query, resolve)

‎src/inject/index.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,10 @@ if (__TEST__) {
297297
checkPageStylesNow();
298298
break;
299299
}
300+
case 'firefox-getColorScheme': {
301+
respond(isSystemDarkModeEnabled() ? 'dark' : 'light');
302+
break;
303+
}
300304
case 'firefox-emulateColorScheme': {
301305
emulateColorScheme(data);
302306
respond(undefined);

‎tests/browser/e2e/toggle.tests.ts‎

Lines changed: 108 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ async function expectStyles(styles: StyleExpectations) {
55
await expectPageStyles(expect, styles);
66
}
77

8-
async function loadBasicPage(header = 'E2E test page') {
8+
async function loadBasicPage(header: string) {
99
await loadTestPage({
1010
'/': multiline(
1111
'<!DOCTYPE html>',
@@ -29,6 +29,9 @@ describe('Toggling the extension', () => {
2929
// TODO: remove flakes and remove this line
3030
jest.retryTimes(10, {logErrorsBeforeRetry: true});
3131

32+
const automationMenuSelector = '.header__more-settings-button';
33+
const automationSystemSelector = '.header__more-settings__system-dark-mode__checkbox .checkbox__input';
34+
3235
it('should turn On/Off', async () => {
3336
await loadBasicPage('Toggle on/off');
3437

@@ -69,11 +72,8 @@ describe('Toggling the extension', () => {
6972
it('should follow system color scheme', async () => {
7073
await loadBasicPage('Automation (color scheme)');
7174

72-
const automationMenuSelector = '.header__more-settings-button';
73-
const automationSystemSelector = '.header__more-settings__system-dark-mode__checkbox .checkbox__input';
7475

7576
await emulateColorScheme('light');
76-
await expect(getColorScheme()).resolves.toBe('light');
7777

7878
await expectStyles([
7979
['document', 'background-color', 'rgb(24, 26, 27)'],
@@ -97,7 +97,6 @@ describe('Toggling the extension', () => {
9797
]);
9898

9999
await emulateColorScheme('dark');
100-
await expect(getColorScheme()).resolves.toBe('dark');
101100

102101
await expectStyles([
103102
['document', 'background-color', 'rgb(24, 26, 27)'],
@@ -109,7 +108,6 @@ describe('Toggling the extension', () => {
109108
]);
110109

111110
await emulateColorScheme('light');
112-
await expect(getColorScheme()).resolves.toBe('light');
113111

114112
await expectStyles([
115113
['document', 'background-color', 'rgba(0, 0, 0, 0)'],
@@ -134,6 +132,110 @@ describe('Toggling the extension', () => {
134132
await emulateColorScheme('dark');
135133
});
136134

135+
// Note: this test is relevant only to Firefox and Thunderbird
136+
it('should ignore color watcher messages from subframes', async () => {
137+
const darkPageExpectations: StyleExpectations = [
138+
['document', 'background-color', 'rgb(24, 26, 27)'],
139+
['document', 'color', 'rgb(232, 230, 227)'],
140+
['body', 'background-color', 'rgb(24, 26, 27)'],
141+
['body', 'color', 'rgb(232, 230, 227)'],
142+
['h1', 'color', 'rgb(255, 26, 26)'],
143+
['a', 'color', 'rgb(51, 145, 255)'],
144+
];
145+
146+
const lightPageExpectations: StyleExpectations = [
147+
['document', 'background-color', 'rgba(0, 0, 0, 0)'],
148+
['document', 'color', 'rgb(0, 0, 0)'],
149+
['body', 'background-color', 'rgba(0, 0, 0, 0)'],
150+
['body', 'color', 'rgb(0, 0, 0)'],
151+
['h1', 'color', 'rgb(255, 0, 0)'],
152+
['a', 'color', 'rgb(0, 0, 238)'],
153+
];
154+
155+
const darkSubframePageExpectations: StyleExpectations = [
156+
[['iframe', 'h1'], 'color', 'rgb(255, 26, 26)'],
157+
[['iframe', 'a'], 'color', 'rgb(51, 145, 255)'],
158+
];
159+
160+
const lightSubframePageExpectations: StyleExpectations = [
161+
[['iframe', 'h1'], 'color', 'rgb(255, 0, 0)'],
162+
[['iframe', 'a'], 'color', 'rgb(0, 0, 238)'],
163+
];
164+
165+
let loadSubframe: () => void = null;
166+
await loadTestPage({
167+
'/': multiline(
168+
'<!DOCTYPE html>',
169+
'<html>',
170+
'<head>',
171+
' <style>',
172+
' h1 { color: red; }',
173+
' </style>',
174+
'</head>',
175+
'<body>',
176+
` <h1>Color scheme detector</h1>`,
177+
' <p>Text</p>',
178+
' <a href="#">Link</a>',
179+
' <iframe src="/subframe.html" style="color-scheme: light"></iframe>',
180+
'</body>',
181+
'</html>',
182+
),
183+
'/subframe.html': async (_, res) => {
184+
res.statusCode = 200;
185+
res.setHeader('Content-Type', 'text/html');
186+
187+
await new Promise<void>((resolve) => loadSubframe = resolve);
188+
189+
res.end(
190+
multiline(
191+
'<!DOCTYPE html>',
192+
'<html>',
193+
'<head>',
194+
' <style>',
195+
' h1 { color: red; }',
196+
' </style>',
197+
'</head>',
198+
'<body>',
199+
' <h1>Header</h1>',
200+
' <p>Text</p>',
201+
' <a href="#">Link</a>',
202+
'</body>',
203+
'</html>',
204+
),
205+
'utf8'
206+
);
207+
},
208+
}, {
209+
waitUntil: 'domcontentloaded',
210+
});
211+
212+
await emulateColorScheme('dark');
213+
await expectStyles(darkPageExpectations);
214+
215+
await popupUtils.click(automationMenuSelector);
216+
await popupUtils.click(automationSystemSelector);
217+
218+
await expectStyles(darkPageExpectations);
219+
220+
loadSubframe();
221+
222+
// Ensure that the subframe received its styles
223+
await expectStyles(darkSubframePageExpectations);
224+
// Ensure that the parent frame retained its styles
225+
await expectStyles(darkPageExpectations);
226+
227+
await emulateColorScheme('light');
228+
229+
await expectStyles(lightPageExpectations);
230+
await expectStyles(lightSubframePageExpectations);
231+
232+
await popupUtils.click(automationSystemSelector);
233+
234+
await expectStyles(darkPageExpectations);
235+
236+
await emulateColorScheme('dark');
237+
});
238+
137239
it('should have new design button on desktop', async () => {
138240
expect(await devtoolsUtils.exists('.preview-design-button'));
139241
});

‎tests/browser/environment.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,11 @@ export default class CustomJestEnvironment extends TestEnvironment {
297297
if (global.product === 'firefox') {
298298
await global.pageUtils.emulateColorScheme(colorScheme);
299299
await global.backgroundUtils.emulateColorScheme(colorScheme);
300+
const newPageColorScheme = await global.backgroundUtils.getColorScheme();
301+
const newBGColorScheme = await global.pageUtils.getColorScheme();
302+
if (newPageColorScheme !== colorScheme || newBGColorScheme !== colorScheme) {
303+
throw new Error('Failed to apply new color scheme');
304+
}
300305
return;
301306
}
302307
await page.emulateMediaFeatures([{name: 'prefers-color-scheme', value: colorScheme}]);
@@ -450,6 +455,12 @@ export default class CustomJestEnvironment extends TestEnvironment {
450455
evaluate: async (script) => await sendToPage('firefox-eval', script),
451456
expectPageStyles: async (expectations) => await sendToPage('firefox-expectPageStyles', expectations),
452457
emulateColorScheme: async (colorScheme) => await sendToPage('firefox-emulateColorScheme', colorScheme),
458+
getColorScheme: async () => {
459+
if (this.global.product !== 'firefox') {
460+
throw new Error('Not supported');
461+
}
462+
return await sendToPage('firefox-getColorScheme');
463+
},
453464
};
454465

455466
this.global.awaitForEvent = awaitForEvent;

0 commit comments

Comments
 (0)