Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New C-S-S webcompat intervention: modifyLocalStorage #1026

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
2 changes: 1 addition & 1 deletion integration-test/playwright/webcompat.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ test('web compat', async ({ page }, testInfo) => {
})

export class WebcompatSpec {
htmlPage = '/webcompat/index.html'
htmlPage = '/webcompat/pages/message-handlers.html'
config = './integration-test/test-pages/webcompat/config/message-handlers.json'

/**
Expand Down
7 changes: 7 additions & 0 deletions integration-test/test-pages.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,11 @@ describe('Test integration pages', () => {
`${process.cwd()}/integration-test/test-pages/webcompat/config/shims.json`
)
})

it('Properly modifies localStorage entries', async () => {
await testPage(
'webcompat/pages/modify-localstorage.html',
`${process.cwd()}/integration-test/test-pages/webcompat/config/modify-localstorage.json`
)
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
{
"unprotectedTemporary": [],
"features": {
"webCompat": {
"exceptions": [],
"state": "enabled",
"settings": {
"windowSizing": "enabled",
"navigatorCredentials": "enabled",
"safariObject": "enabled",
"modifyLocalStorage": {
"state": "enabled",
"changes": []
},
"domains": [
{
"domain": ["localhost", "privacy-test-pages.site"],
"patchSettings": [
{
"op": "add",
"path": "/modifyLocalStorage/changes/-",
"value": {
"key": "keyToBeDeleted",
"action": "delete"
}
}
]
}
]
}
}
}
}
59 changes: 7 additions & 52 deletions integration-test/test-pages/webcompat/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,60 +3,15 @@
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width">
<title>Webcompat</title>
<title>Webcompat Interventions</title>
<link rel="stylesheet" href="../shared/style.css">
</head>
<body>
<script src="../shared/utils.js"></script>

<p>Webcompat, Message Handlers</p>
<script>
test('webkit.messageHandlers - polyfill prevents throw', async () => {
let notThrown = true;
try {
window.webkit.messageHandlers.anythingatall.postMessage({})
} catch (e) {
notThrown = false
}
return [
{ name: 'Error not thrown polyfil', result: notThrown, expected: true },
]
})
test('webkit.messageHandlers - undefined should throw', async () => {
let thrown = false;
try {
window.webkit.messageHandlers.jsHandler.postMessage({})
} catch (e) {
thrown = true
}
return [
{ name: 'undefined handler should throw', result: thrown, expected: true },
]
})
test('webkit.messageHandlers - reflected message', async () => {
window.webkit.messageHandlers.printHandler = {
postMessage() {
return { test: "test" }
}
}

const value = window.webkit.messageHandlers.printHandler.postMessage({});

return [
{ name: 'reflected message should pass through', result: value.test, expected: 'test' },
]
})
renderResults();

async function captureError(fn) {
try {
// ensure Promise.reject is captured
return fn().catch(e => e)
} catch (e) {
return e
}
}

</script>
<p><a href="../../index.html">[Home]</a></p>
<ul>
<li><a href="./pages/message-handlers.html">Message Handlers</a> - <a href="./config/message-handlers.json">Config</a></li>
<li><a href="./pages/shims.html">Shims</a> - <a href="./config/shims.json">Config</a></li>
<li><a href="./pages/modify-localstorage.html">Modify localStorage</a> - <a href="./config/modify-localstorage.json">Config</a></li>
</ul>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width">
<title>Webcompat</title>
<link rel="stylesheet" href="../../shared/style.css">
</head>
<body>
<script src="../../shared/utils.js"></script>

<p>Webcompat, Message Handlers</p>
<script>
test('webkit.messageHandlers - polyfill prevents throw', async () => {
let notThrown = true;
try {
window.webkit.messageHandlers.anythingatall.postMessage({})
} catch (e) {
notThrown = false
}
return [
{ name: 'Error not thrown polyfil', result: notThrown, expected: true },
]
})
test('webkit.messageHandlers - undefined should throw', async () => {
let thrown = false;
try {
window.webkit.messageHandlers.jsHandler.postMessage({})
} catch (e) {
thrown = true
}
return [
{ name: 'undefined handler should throw', result: thrown, expected: true },
]
})
test('webkit.messageHandlers - reflected message', async () => {
window.webkit.messageHandlers.printHandler = {
postMessage() {
return { test: "test" }
}
}

const value = window.webkit.messageHandlers.printHandler.postMessage({});

return [
{ name: 'reflected message should pass through', result: value.test, expected: 'test' },
]
})
renderResults();

async function captureError(fn) {
try {
// ensure Promise.reject is captured
return fn().catch(e => e)
} catch (e) {
return e
}
}

</script>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width">
<title>Modify localStorage</title>
<link rel="stylesheet" href="../../shared/style.css">
<script>
window.localStorage.setItem('keyToBeDeleted', 'valueToBeDeleted')
window.localStorage.setItem('otherKey', 'valueToRemain')
</script>
</head>
<body>
<script src="../../shared/utils.js"></script>
<p><a href="../index.html">[WebCompat]</a></p>

<p>This page verifies that localStorage modifications work properly given the <a href="../config/modify-localstorage.json">config</a>. At this time, only deletion is supported.</p>

<script>
// eslint-disable-next-line no-undef
test('Only specified localStorage entry should be removed', async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a test for a key that is also isn't present but the code deletes?

Copy link
Contributor Author

@dharb dharb Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const specifiedKey = window.localStorage.getItem('keyToBeDeleted')
const otherKey = window.localStorage.getItem('otherKey')

return [
{ name: 'specified localStorage entry deleted', result: specifiedKey, expected: null },
{ name: 'other localStorage entry untouched', result: otherKey, expected: 'valueToRemain' }
];
});

// eslint-disable-next-line no-undef
renderResults();
</script>
</body>
</html>
10 changes: 4 additions & 6 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
"@types/chrome": "^0.0.248",
"@types/jasmine": "^5.1.4",
"@typescript-eslint/eslint-plugin": "^6.9.1",
"config-builder": "github:duckduckgo/privacy-configuration#main",
"config-builder": "github:duckduckgo/privacy-configuration#dharb/modify-localstorage-config",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll flip this back once the config PR is merged.

"esbuild": "^0.19.5",
"eslint": "^8.57.1",
"eslint-config-standard": "^17.1.0",
Expand Down
22 changes: 22 additions & 0 deletions src/features/web-compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ export class WebCompat extends ContentFeature {
if (this.getFeatureSettingEnabled('screenLock')) {
this.screenLockFix()
}

if (this.getFeatureSettingEnabled('modifyLocalStorage')) {
this.modifyLocalStorage()
}
}

/** Shim Web Share API in Android WebView */
Expand Down Expand Up @@ -531,6 +535,24 @@ export class WebCompat extends ContentFeature {
}
}

/**
* Support for modifying localStorage entries
*/
modifyLocalStorage () {
/** @type {import('../types//webcompat-settings').WebCompatSettings['modifyLocalStorage']} */
dharb marked this conversation as resolved.
Show resolved Hide resolved
const settings = this.getFeatureSetting('modifyLocalStorage')

if (!settings || !settings.changes) return

settings.changes.forEach((change) => {
// @ts-expect-error Property 'action' does not exist on type '{}'
dharb marked this conversation as resolved.
Show resolved Hide resolved
if (change.action === 'delete') {
// @ts-expect-error Property 'key' does not exist on type '{}'
localStorage.removeItem(change.key)
}
})
}

/**
* Support for proxying `window.webkit.messageHandlers`
*/
Expand Down
25 changes: 24 additions & 1 deletion src/types/webcompat-settings.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,30 @@ export interface WebCompatSettings {
undefined: string[];
};
};
modifyLocalStorage?: {
state: State;
changes: {}[];
dharb marked this conversation as resolved.
Show resolved Hide resolved
};
notification?: {
state: State;
};
permissions?: {
state: State;
supportedPermissions: {};
};
mediaSession?: State;
presentation?: State;
webShare?: State;
viewportWidth?:
| State
| {
state: State;
forcedDesktopValue?: string;
forcedMobileValue?: string;
};
screenLock?: State;
domains?: Domains;
plainTextViewPort?: State;
}
export interface Domain {
/**
Expand All @@ -53,5 +76,5 @@ export interface PatchSetting {
/**
* The value to replace at the specified path
*/
value: string;
value: string | unknown[] | {} | number;
}
Loading