Skip to content

Commit 1548938

Browse files
shakyShaneshakyShane
authored
add favicon feature (#1561)
* add favicon feature * linting * send the first set immediately * sample instead of debouncing * send full href * only in top * full href value * ignore messages from favicons feature * comments/logs * support new additions * don't run on iOS * remove pause * break from all loops * add missing test for when the feature is disabled as a whole
1 parent 27b7e0f commit 1548938

File tree

18 files changed

+456
-5
lines changed

18 files changed

+456
-5
lines changed

‎injected/docs/favicon.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
---
2+
title: Favicon Monitor
3+
---
4+
5+
# Favicon Monitor
6+
7+
Reports the presence of favicons on page-load, and optionally when they change.
8+
9+
## Notifications
10+
11+
### `faviconFound`
12+
- {@link "Favicon Messages".FaviconFoundNotification}
13+
- Sent on page load, sends {@link "Favicon Messages".FaviconFound}
14+
15+
**Example**
16+
17+
```json
18+
{
19+
"favicons": [
20+
{
21+
"href": "favicon.png",
22+
"rel": "stylesheet"
23+
}
24+
],
25+
"documentUrl": "https://example.com"
26+
}
27+
```
28+
29+
## Remote Config
30+
31+
## Enabled (default)
32+
{@includeCode ../integration-test/test-pages/favicon/config/favicon-enabled.json}
33+
34+
### Disable the monitor only.
35+
36+
To only receive the initial payload and nothing more (to mimic the old behavior),
37+
you can set `monitor: false` in the remote config, and it will not install the Mutation Observer.
38+
39+
{@includeCode ../integration-test/test-pages/favicon/config/favicon-monitor-disabled.json}

‎injected/integration-test/favicon.spec.js

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
import { test, expect } from '@playwright/test';
2+
import { ResultsCollector } from './page-objects/results-collector.js';
3+
4+
const HTML = '/favicon/index.html';
5+
const CONFIG = './integration-test/test-pages/favicon/config/favicon-enabled.json';
6+
7+
test('favicon feature absent', async ({ page, baseURL }, testInfo) => {
8+
const CONFIG = './integration-test/test-pages/favicon/config/favicon-absent.json';
9+
const favicon = ResultsCollector.create(page, testInfo.project.use);
10+
await favicon.load(HTML, CONFIG);
11+
12+
// ensure first favicon item was sent
13+
const messages = await favicon.waitForMessage('faviconFound', 1);
14+
const url = new URL('/favicon/favicon.png', baseURL);
15+
16+
expect(messages[0].payload.params).toStrictEqual({
17+
favicons: [{ href: url.href, rel: 'shortcut icon' }],
18+
documentUrl: 'http://localhost:3220/favicon/index.html',
19+
});
20+
});
21+
22+
test('favicon + monitor', async ({ page, baseURL }, testInfo) => {
23+
const favicon = ResultsCollector.create(page, testInfo.project.use);
24+
await favicon.load(HTML, CONFIG);
25+
26+
// ensure first favicon item was sent
27+
await favicon.waitForMessage('faviconFound', 1);
28+
29+
// now update it
30+
await page.getByRole('button', { name: 'Set override' }).click();
31+
32+
// wait for the second message
33+
const messages = await favicon.waitForMessage('faviconFound', 2);
34+
35+
const url1 = new URL('/favicon/favicon.png', baseURL);
36+
const url2 = new URL('/favicon/new_favicon.png', baseURL);
37+
38+
expect(messages[0].payload.params).toStrictEqual({
39+
favicons: [{ href: url1.href, rel: 'shortcut icon' }],
40+
documentUrl: 'http://localhost:3220/favicon/index.html',
41+
});
42+
43+
expect(messages[1].payload.params).toStrictEqual({
44+
favicons: [{ href: url2.href, rel: 'shortcut icon' }],
45+
documentUrl: 'http://localhost:3220/favicon/index.html',
46+
});
47+
});
48+
49+
test('favicon + monitor + newly added links', async ({ page, baseURL }, testInfo) => {
50+
const favicon = ResultsCollector.create(page, testInfo.project.use);
51+
await favicon.load(HTML, CONFIG);
52+
53+
// ensure first favicon item was sent
54+
await favicon.waitForMessage('faviconFound', 1);
55+
56+
// now cause a new item to be added
57+
await page.getByRole('button', { name: 'Add new' }).click();
58+
59+
// wait for the second message
60+
const messages = await favicon.waitForMessage('faviconFound', 2);
61+
62+
const url1 = new URL('/favicon/favicon.png', baseURL);
63+
const url2 = new URL('/favicon/new_favicon.png', baseURL);
64+
65+
expect(messages[0].payload.params).toStrictEqual({
66+
favicons: [{ href: url1.href, rel: 'shortcut icon' }],
67+
documentUrl: 'http://localhost:3220/favicon/index.html',
68+
});
69+
70+
expect(messages[1].payload.params).toStrictEqual({
71+
favicons: [
72+
{ href: url1.href, rel: 'shortcut icon' },
73+
{ href: url2.href, rel: 'shortcut icon' },
74+
],
75+
documentUrl: 'http://localhost:3220/favicon/index.html',
76+
});
77+
});
78+
79+
test('favicon + monitor (many updates)', async ({ page, baseURL }, testInfo) => {
80+
const favicon = ResultsCollector.create(page, testInfo.project.use);
81+
await page.clock.install();
82+
await favicon.load(HTML, CONFIG);
83+
84+
// ensure first favicon item was sent
85+
await favicon.waitForMessage('faviconFound', 1);
86+
87+
// now update it
88+
await page.getByRole('button', { name: 'Set many overrides' }).click();
89+
await page.clock.fastForward(20);
90+
91+
const messages = await favicon.outgoingMessages();
92+
expect(messages).toHaveLength(1);
93+
94+
await page.clock.fastForward(60);
95+
await page.clock.fastForward(100);
96+
97+
{
98+
const messages = await favicon.outgoingMessages();
99+
expect(messages).toHaveLength(3);
100+
}
101+
102+
{
103+
const url1 = new URL('/favicon/favicon.png', baseURL);
104+
const url2 = new URL('/favicon/new_favicon.png?count=0', baseURL);
105+
const url3 = new URL('/favicon/new_favicon.png?count=1', baseURL);
106+
107+
const messages = await favicon.outgoingMessages();
108+
expect(messages.map((x) => /** @type {{params: any}} */ (x.payload).params)).toStrictEqual([
109+
{
110+
favicons: [{ href: url1.href, rel: 'shortcut icon' }],
111+
documentUrl: 'http://localhost:3220/favicon/index.html',
112+
},
113+
{
114+
favicons: [{ href: url2.href, rel: 'shortcut icon' }],
115+
documentUrl: 'http://localhost:3220/favicon/index.html',
116+
},
117+
{
118+
favicons: [{ href: url3.href, rel: 'shortcut icon' }],
119+
documentUrl: 'http://localhost:3220/favicon/index.html',
120+
},
121+
]);
122+
}
123+
});
124+
125+
test('favicon + monitor disabled', async ({ page }, testInfo) => {
126+
const CONFIG = './integration-test/test-pages/favicon/config/favicon-monitor-disabled.json';
127+
const favicon = ResultsCollector.create(page, testInfo.project.use);
128+
129+
await page.clock.install();
130+
131+
await favicon.load(HTML, CONFIG);
132+
133+
// ensure first favicon item was sent
134+
await favicon.waitForMessage('faviconFound', 1);
135+
136+
// now update it
137+
await page.getByRole('button', { name: 'Set override' }).click();
138+
139+
await expect(page.locator('link')).toHaveAttribute('href', './new_favicon.png');
140+
141+
// account for the debounce
142+
await page.clock.fastForward(200);
143+
144+
// ensure only 1 message was still sent (ie: the monitor is disabled)
145+
const messages = await favicon.outgoingMessages();
146+
expect(messages).toHaveLength(1);
147+
});
148+
149+
test('favicon feature disabled completely', async ({ page }, testInfo) => {
150+
const CONFIG = './integration-test/test-pages/favicon/config/favicon-disabled.json';
151+
const favicon = ResultsCollector.create(page, testInfo.project.use);
152+
153+
await favicon.load(HTML, CONFIG);
154+
155+
// this is here purely to guard against a false positive in this test.
156+
// without this manual `wait`, it might be possible for the following assertion to
157+
// pass, but just because it was too quick (eg: the first message wasn't sent yet)
158+
await page.waitForTimeout(100);
159+
160+
const messages = await favicon.outgoingMessages();
161+
expect(messages).toHaveLength(0);
162+
});

‎injected/integration-test/page-objects/results-collector.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -268,14 +268,15 @@ export class ResultsCollector {
268268

269269
/**
270270
* @param {string} method
271-
* @return {Promise<object>}
271+
* @param {number} [count=1]
272+
* @return {Promise<Record<string, any>[]>}
272273
*/
273-
async waitForMessage(method) {
274+
async waitForMessage(method, count = 1) {
274275
await this.page.waitForFunction(
275276
waitForCallCount,
276277
{
277278
method,
278-
count: 1,
279+
count,
279280
},
280281
{ timeout: 5000, polling: 100 },
281282
);

‎injected/integration-test/test-pages/favicon/config/favicon-absent.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"features": {
3+
4+
},
5+
"unprotectedTemporary": []
6+
}

‎injected/integration-test/test-pages/favicon/config/favicon-disabled.json

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"features": {
3+
"favicon": {
4+
"state": "disabled",
5+
"exceptions": []
6+
}
7+
},
8+
"unprotectedTemporary": []
9+
}

‎injected/integration-test/test-pages/favicon/config/favicon-enabled.json

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"features": {
3+
"favicon": {
4+
"state": "enabled",
5+
"exceptions": [],
6+
"settings": {
7+
"monitor": true
8+
}
9+
}
10+
},
11+
"unprotectedTemporary": []
12+
}

‎injected/integration-test/test-pages/favicon/config/favicon-monitor-disabled.json

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"features": {
3+
"favicon": {
4+
"state": "enabled",
5+
"exceptions": [],
6+
"settings": {
7+
"monitor": false
8+
}
9+
}
10+
},
11+
"unprotectedTemporary": []
12+
}

‎injected/integration-test/test-pages/favicon/favicon.png

7.71 KB
Loading

‎injected/integration-test/test-pages/favicon/index.html

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<!doctype html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="UTF-8">
5+
<meta name="viewport" content="width=device-width, initial-scale=1">
6+
<title>Document</title>
7+
<link rel="shortcut icon" href="./favicon.png">
8+
</head>
9+
<body>
10+
<button onclick="setOverride()">Set override</button>
11+
<button onclick="setManyOverrides()">Set many overrides</button>
12+
<button onclick="addNew()">Add new</button>
13+
<script>
14+
function setOverride() {
15+
document.querySelector("link[rel='shortcut icon']").href = "./new_favicon.png";
16+
}
17+
async function setManyOverrides() {
18+
const elem = document.querySelector("link[rel='shortcut icon']");
19+
for (let i = 0; i < 100; i++) {
20+
await new Promise(resolve => {
21+
setTimeout(resolve, 40);
22+
})
23+
const path = `./new_favicon.png?count=${i}`
24+
elem.href = path
25+
}
26+
}
27+
function addNew() {
28+
const next = document.createElement('link');
29+
next.rel = 'shortcut icon'
30+
next.href = './new_favicon.png'
31+
document.head.appendChild(next)
32+
}
33+
</script>
34+
</body>
35+
</html>

‎injected/integration-test/test-pages/favicon/new_favicon.png

33 KB
Loading

‎injected/integration-test/test-pages/message-bridge/config/message-bridge-disabled.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
{
22
"unprotectedTemporary": [],
33
"features": {
4+
"favicon": {
5+
"state": "disabled",
6+
"exceptions": []
7+
},
48
"navigatorInterface": {
59
"state": "enabled",
610
"exceptions": []

‎injected/integration-test/test-pages/message-bridge/config/message-bridge-enabled.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55
"state": "enabled",
66
"exceptions": []
77
},
8+
"favicon": {
9+
"state": "disabled",
10+
"exceptions": []
11+
},
812
"messageBridge": {
913
"exceptions": [],
1014
"state": "enabled",

‎injected/playwright.config.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ export default defineConfig({
2121
'integration-test/duckplayer.spec.js',
2222
'integration-test/duckplayer-remote-config.spec.js',
2323
'integration-test/broker-protection-tests/**/*.spec.js',
24+
'integration-test/favicon.spec.js',
2425
],
2526
use: { injectName: 'apple-isolated', platform: 'macos' },
2627
},

‎injected/src/features.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,14 @@ const otherFeatures = /** @type {const} */ ([
2626
'performanceMetrics',
2727
'breakageReporting',
2828
'autofillPasswordImport',
29+
'favicon',
2930
]);
3031

3132
/** @typedef {baseFeatures[number]|otherFeatures[number]} FeatureName */
3233
/** @type {Record<string, FeatureName[]>} */
3334
export const platformSupport = {
3435
apple: ['webCompat', ...baseFeatures],
35-
'apple-isolated': ['duckPlayer', 'brokerProtection', 'performanceMetrics', 'clickToLoad', 'messageBridge'],
36+
'apple-isolated': ['duckPlayer', 'brokerProtection', 'performanceMetrics', 'clickToLoad', 'messageBridge', 'favicon'],
3637
android: [...baseFeatures, 'webCompat', 'breakageReporting', 'duckPlayer', 'messageBridge'],
3738
'android-broker-protection': ['brokerProtection'],
3839
'android-autofill-password-import': ['autofillPasswordImport'],

0 commit comments

Comments
 (0)