-
Notifications
You must be signed in to change notification settings - Fork 31
Support navigation based conditional matching #1610
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
Changes from all commits
e7e0ac9
aed4fb3
ee0ba65
698d54f
f5ec3ea
9d37c04
bbf3c5b
7543748
19519bd
310cc95
7d04e76
061b06d
42d6241
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| { | ||
| "features": { | ||
| "apiManipulation": { | ||
| "state": "enabled", | ||
| "settings": { | ||
| "apiChanges": { | ||
| "Navigator.prototype.hardwareConcurrency": { | ||
| "type": "descriptor", | ||
| "getterValue": { | ||
| "type": "number", | ||
| "value": 222 | ||
| } | ||
| } | ||
| }, | ||
| "conditionalChanges": [ | ||
| { | ||
| "condition": { | ||
| "urlPattern": "/test/*" | ||
| }, | ||
| "patchSettings": [ | ||
| { | ||
| "op": "replace", | ||
| "path": "/apiChanges/Navigator.prototype.hardwareConcurrency/getterValue/value", | ||
| "value": 333 | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| }, | ||
| "unprotectedTemporary": [] | ||
| } | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| <!DOCTYPE html> | ||
| <html> | ||
| <head> | ||
| <meta charset="utf-8"> | ||
| <meta name="viewport" content="width=device-width"> | ||
| <title>API Interventions</title> | ||
| <link rel="stylesheet" href="../shared/style.css"> | ||
| </head> | ||
| <body> | ||
| <p><a href="../../index.html">[Home]</a></p> | ||
| <ul> | ||
| <li><a href="./pages/conditional-matching.html">Conditional matching</a> - <a href="./config/conditional-matching.json">Config</a></li> | ||
| </ul> | ||
| </body> | ||
| </html> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| <!DOCTYPE html> | ||
| <html> | ||
| <head> | ||
| <meta charset="utf-8"> | ||
| <meta name="viewport" content="width=device-width"> | ||
| <title>Conditional Matching</title> | ||
| <link rel="stylesheet" href="../../shared/style.css"> | ||
| </head> | ||
| <body> | ||
| <script src="../../shared/utils.js"></script> | ||
| <p><a href="../index.html">[Infra]</a></p> | ||
|
|
||
| <p>This page verifies that APIs get modified</p> | ||
|
|
||
| <script> | ||
| test('Conditional matching', async () => { | ||
| const results = [ | ||
| { | ||
| name: "APIs changing, expecting to always match", | ||
| result: navigator.hardwareConcurrency, | ||
| expected: 222 | ||
| } | ||
| ]; | ||
| const oldPathname = window.location.pathname; | ||
| const newUrl = new URL(window.location.href); | ||
| newUrl.pathname = "/test/test/path"; | ||
| window.history.pushState(null, '', newUrl.href); | ||
| await new Promise(resolve => requestIdleCallback(resolve)); | ||
| results.push({ | ||
| name: "Expect URL to be changed", | ||
| result: window.location.pathname, | ||
| expected: '/test/test/path' | ||
| }) | ||
| results.push({ | ||
| name: "APIs changing, expecting to match only when the URL is correct", | ||
| result: navigator.hardwareConcurrency, | ||
| expected: 333 | ||
| }) | ||
| const popStatePromise = new Promise(resolve => { | ||
| window.addEventListener('popstate', resolve, { once: true }); | ||
| }); | ||
| // Call pop state to revert the URL | ||
| window.history.back(); | ||
| await popStatePromise; | ||
| results.push({ | ||
| name: "Expect URL to be reverted", | ||
| result: window.location.pathname, | ||
| expected: oldPathname | ||
| }) | ||
| results.push({ | ||
| name: "APIs changing, expecting to match only when the URL is correct", | ||
| result: navigator.hardwareConcurrency, | ||
| expected: 222 | ||
| }) | ||
|
|
||
| return results; | ||
| }); | ||
|
|
||
|
|
||
| // eslint-disable-next-line no-undef | ||
| renderResults(); | ||
| </script> | ||
| </body> | ||
| </html> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,8 @@ import { processAttr } from '../utils'; | |
| * @internal | ||
| */ | ||
| export default class ApiManipulation extends ContentFeature { | ||
| listenForUrlChanges = true; | ||
|
|
||
| init() { | ||
| const apiChanges = this.getFeatureSetting('apiChanges'); | ||
| if (apiChanges) { | ||
|
|
@@ -26,6 +28,10 @@ export default class ApiManipulation extends ContentFeature { | |
| } | ||
| } | ||
|
|
||
| urlChanged() { | ||
| this.init(); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This means we'll rerun rules on navigation change. |
||
| } | ||
|
|
||
| /** | ||
| * Checks if the config API change is valid. | ||
| * @param {any} change | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| import { DDGProxy, DDGReflect, isBeingFramed } from './utils.js'; | ||
| import ContentFeature from './content-feature.js'; | ||
|
|
||
| const urlChangeListeners = new Set(); | ||
| /** | ||
| * Register a listener to be called when the URL changes. | ||
| * @param {function} listener | ||
| */ | ||
| export function registerForURLChanges(listener) { | ||
| if (urlChangeListeners.size === 0) { | ||
| listenForURLChanges(); | ||
| } | ||
| urlChangeListeners.add(listener); | ||
| } | ||
|
|
||
| function handleURLChange() { | ||
| for (const listener of urlChangeListeners) { | ||
| listener(); | ||
| } | ||
| } | ||
|
|
||
| function listenForURLChanges() { | ||
| const urlChangedInstance = new ContentFeature('urlChanged', {}, {}); | ||
| if ('navigation' in globalThis && 'addEventListener' in globalThis.navigation) { | ||
| // if the browser supports the navigation API, we can use that to listen for URL changes | ||
| // Listening to navigatesuccess instead of navigate to ensure the navigation is committed. | ||
| globalThis.navigation.addEventListener('navigatesuccess', () => { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dbajpeyi the navigate event actually fired too early, I validated this works as expected. The API has a way to modify and deny navigations before they happen. |
||
| handleURLChange(); | ||
| }); | ||
| // Exit early if the navigation API is supported | ||
| return; | ||
| } | ||
| if (isBeingFramed()) { | ||
| // don't run if we're in an iframe | ||
| return; | ||
| } | ||
| // single page applications don't have a DOMContentLoaded event on navigations, so | ||
| // we use proxy/reflect on history.pushState to call applyRules on page navigations | ||
| const historyMethodProxy = new DDGProxy(urlChangedInstance, History.prototype, 'pushState', { | ||
| apply(target, thisArg, args) { | ||
| const changeResult = DDGReflect.apply(target, thisArg, args); | ||
| handleURLChange(); | ||
| return changeResult; | ||
| }, | ||
| }); | ||
| historyMethodProxy.overload(); | ||
| // listen for popstate events in order to run on back/forward navigations | ||
| window.addEventListener('popstate', () => { | ||
| handleURLChange(); | ||
| }); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dbajpeyi this is now split out so feature authors don't need to call super() as before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking: do we forsee any major issues if we have both
recomputeSiteObjectandurlChangedbeing called? Can there be any inconsistencies?