Skip to content

[DRAFT] Notification: Define-property improvements #1878

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 37 additions & 6 deletions injected/src/content-feature.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,11 @@ export default class ContentFeature extends ConfigFeature {
/**
* Define a property descriptor with debug flags.
* Mainly used for defining new properties. For overriding existing properties, consider using wrapProperty(), wrapMethod() and wrapConstructor().
* @param {any} object - object whose property we are wrapping (most commonly a prototype, e.g. globalThis.BatteryManager.prototype)
* @param {string} propertyName
* @param {import('./wrapper-utils').StrictPropertyDescriptor} descriptor - requires all descriptor options to be defined because we can't validate correctness based on TS types
* @template Obj
* @template {keyof Obj} Key
* @param {Obj} object - object whose property we are wrapping (most commonly a prototype, e.g. globalThis.BatteryManager.prototype)
* @param {Key} propertyName
* @param {import('./wrapper-utils.js').StrictPropertyDescriptorGeneric<Obj, Key>} descriptor - requires all descriptor options to be defined because we can't validate correctness based on TS types
*/
defineProperty(object, propertyName, descriptor) {
// make sure to send a debug flag when the property is used
Expand All @@ -247,16 +249,45 @@ export default class ContentFeature extends ConfigFeature {
if (typeof descriptorProp === 'function') {
const addDebugFlag = this.addDebugFlag.bind(this);
const wrapper = new Proxy(descriptorProp, {
apply(_, thisArg, argumentsList) {
apply(target, thisArg, argumentsList) {
addDebugFlag();
return Reflect.apply(descriptorProp, thisArg, argumentsList);
return target.apply(thisArg, argumentsList);
},
});
descriptor[k] = wrapToString(wrapper, descriptorProp);
}
});

return defineProperty(object, propertyName, descriptor);
// Build complete strict descriptor all at once so TS doesn't complain about missing properties
let strictDescriptor;

if (descriptor.value !== undefined || descriptor.writable !== undefined) {
// Data descriptor
strictDescriptor = {
configurable: descriptor.configurable ?? false,
enumerable: descriptor.enumerable ?? false,
value: descriptor.value,
writable: descriptor.writable ?? false,
};
} else if (descriptor.get !== undefined || descriptor.set !== undefined) {
// Accessor descriptor
strictDescriptor = {
configurable: descriptor.configurable ?? false,
enumerable: descriptor.enumerable ?? false,
get: descriptor.get ?? (() => undefined),
set: descriptor.set ?? (() => {}),
};
} else {
// Default data descriptor
strictDescriptor = {
configurable: descriptor.configurable ?? false,
enumerable: descriptor.enumerable ?? false,
value: undefined,
writable: false,
};
}

return defineProperty(object, String(propertyName), strictDescriptor);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export default class FingerprintingTemporaryStorage extends ContentFeature {
// @ts-expect-error https://app.asana.com/0/1201614831475344/1203979574128023/f
org.call(navigator.webkitTemporaryStorage, modifiedCallback, err);
};
// @ts-expect-error This doesn't exist in the DOM lib
this.defineProperty(Navigator.prototype, 'webkitTemporaryStorage', {
get: () => tStorage,
enumerable: true,
Expand Down
2 changes: 2 additions & 0 deletions injected/src/features/gpc.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export default class GlobalPrivacyControl extends ContentFeature {
if (args.globalPrivacyControlValue) {
// @ts-expect-error https://app.asana.com/0/1201614831475344/1203979574128023/f
if (navigator.globalPrivacyControl) return;
// @ts-expect-error This doesn't exist in the DOM lib
this.defineProperty(Navigator.prototype, 'globalPrivacyControl', {
get: () => true,
configurable: true,
Expand All @@ -18,6 +19,7 @@ export default class GlobalPrivacyControl extends ContentFeature {
// this may be overwritten by the user agent or other extensions
// @ts-expect-error https://app.asana.com/0/1201614831475344/1203979574128023/f
if (typeof navigator.globalPrivacyControl !== 'undefined') return;
// @ts-expect-error This doesn't exist in the DOM lib
this.defineProperty(Navigator.prototype, 'globalPrivacyControl', {
get: () => false,
configurable: true,
Expand Down
1 change: 1 addition & 0 deletions injected/src/features/navigator-interface.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export default class NavigatorInterface extends ContentFeature {
if (!args.platform || !args.platform.name) {
return;
}
// @ts-expect-error This doesn't exist in the DOM lib
this.defineProperty(Navigator.prototype, 'duckduckgo', {
value: {
platform: args.platform.name,
Expand Down
11 changes: 7 additions & 4 deletions injected/src/features/web-compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,16 +192,17 @@
return;
}
// Expose the API
// window.Notification polyfill is intentionally incompatible with DOM lib types
this.defineProperty(window, 'Notification', {
value: () => {

Check failure on line 197 in injected/src/features/web-compat.js

View workflow job for this annotation

GitHub Actions / snapshots

Type '() => void' is not assignable to type '{ new (title: string, options?: NotificationOptions | undefined): Notification; prototype: Notification; readonly permission: NotificationPermission; requestPermission(deprecatedCallback?: NotificationPermissionCallback | undefined): Promise<...>; } | ((this: Window & typeof globalThis) => { ...; }) | undefined'.

Check failure on line 197 in injected/src/features/web-compat.js

View workflow job for this annotation

GitHub Actions / unit (ubuntu-latest)

Type '() => void' is not assignable to type '{ new (title: string, options?: NotificationOptions | undefined): Notification; prototype: Notification; readonly permission: NotificationPermission; requestPermission(deprecatedCallback?: NotificationPermissionCallback | undefined): Promise<...>; } | ((this: Window & typeof globalThis) => { ...; }) | undefined'.
// noop
},
writable: true,
configurable: true,
enumerable: false,
});

this.defineProperty(window.Notification, 'requestPermission', {
// window.Notification polyfill is intentionally incompatible with DOM lib types
this.defineProperty(/** @type {any} */ (window.Notification), 'requestPermission', {
value: () => {
return Promise.resolve('denied');
},
Expand All @@ -210,13 +211,13 @@
enumerable: true,
});

this.defineProperty(window.Notification, 'permission', {
this.defineProperty(/** @type {any} */ (window.Notification), 'permission', {
get: () => 'denied',
configurable: true,
enumerable: false,
});

this.defineProperty(window.Notification, 'maxActions', {
this.defineProperty(/** @type {any} */ (window.Notification), 'maxActions', {
get: () => 2,
configurable: true,
enumerable: true,
Expand Down Expand Up @@ -400,7 +401,8 @@
};
// TODO: original property is an accessor descriptor
this.defineProperty(Navigator.prototype, 'credentials', {
// validate this
value,

Check failure on line 405 in injected/src/features/web-compat.js

View workflow job for this annotation

GitHub Actions / snapshots

Type '{ get(): Promise<never>; }' is not assignable to type 'CredentialsContainer | ((this: Navigator) => CredentialsContainer) | undefined'.

Check failure on line 405 in injected/src/features/web-compat.js

View workflow job for this annotation

GitHub Actions / unit (ubuntu-latest)

Type '{ get(): Promise<never>; }' is not assignable to type 'CredentialsContainer | ((this: Navigator) => CredentialsContainer) | undefined'.
configurable: true,
enumerable: true,
writable: true,
Expand All @@ -416,6 +418,7 @@
if (window.safari) {
return;
}
// @ts-expect-error https://app.asana.com/0/1201614831475344/1203979574128023/f
this.defineProperty(window, 'safari', {
value: {},
writable: true,
Expand Down
24 changes: 23 additions & 1 deletion injected/src/wrapper-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -366,16 +366,38 @@ export function shimProperty(baseObject, propertyName, implInstance, readOnly, d
*/

/**
* @typedef {Object} BaseStrictPropertyDescriptor
* A generic property descriptor for a property of an object, with correct `this` context for accessors.
*
* @template Obj The object type
* @template {keyof Obj} Key The property key
* @typedef {Object} StrictPropertyDescriptorGeneric
* @property {boolean} configurable
* @property {boolean} enumerable
* @property {boolean} [writable]
* @property {(function(this: Obj): Obj[Key]) |Obj[Key]} [value]
* @property {(function(this: Obj): Obj[Key])} [get]
* @property {(function(this: Obj, Obj[Key]): void)} [set]
*/


/**
* @typedef {Object} BaseStrictPropertyDescriptor
* @property {boolean} configurable
* @property {boolean} enumerable
* */
/**
* @typedef {BaseStrictPropertyDescriptor & { value: any; writable: boolean }} StrictDataDescriptor
* */
/**
* @typedef {BaseStrictPropertyDescriptor & { get: () => any; set: (v: any) => void }} StrictAccessorDescriptor
* */
/**
* @typedef {BaseStrictPropertyDescriptor & { get: () => any }} StrictGetDescriptor
* */
/**
* @typedef {BaseStrictPropertyDescriptor & { set: (v: any) => void }} StrictSetDescriptor
* */
/**
* @typedef {StrictDataDescriptor | StrictAccessorDescriptor | StrictGetDescriptor | StrictSetDescriptor} StrictPropertyDescriptor
*/

Expand Down
Loading