Add ability to customize custom selectors from settings page (#292)

* Add ability to customize custom hints from the settings page

* Transform customSelectors to array for easier displaying in settings page

Author: david-tejada

src/background/messaging/handleRequestFromContent.ts

@@ -108,11 +108,11 @@ export async function handleRequestFromContent(
 			return getTabMarker(tabId);
 
 		case "storeCustomSelectors":
-			await storeCustomSelectors(request.pattern, request.selectors);
+			await storeCustomSelectors(request.url, request.selectors);
 			break;
 
 		case "resetCustomSelectors":
-			return resetCustomSelectors(request.pattern);
+			return resetCustomSelectors(request.url);
 
 		case "removeReference":
 			return removeReference(request.hostPattern, request.name);

src/background/utils/arrayUtils.ts

@@ -0,0 +1,11 @@
+// Function to filter an array in place
+export function filterInPlace<T>(
+	array: T[],
+	filter: (item: T, index: number) => boolean
+) {
+	for (let i = array.length - 1; i >= 0; i--) {
+		if (!filter(array[i]!, i)) {
+			array.splice(i, 1);
+		}
+	}
+}

src/background/utils/storeCustomSelectors.ts

@@ -1,11 +1,11 @@
 import assertNever from "assert-never";
 import { debounce } from "lodash";
-import { CustomSelectorsForPattern } from "../../typings/StorageSchema";
+import { CustomSelector } from "../../typings/StorageSchema";
 import { notify } from "./notify";
 import { withLockedStorageAccess } from "./withLockedStorageValue";
+import { filterInPlace } from "./arrayUtils";
 
 let notifySuccess = false;
-const modifiedSelectors = new Set<string>();
 let batchUpdatePromise: Promise<void> | undefined;
 let batchUpdatePromiseResolve: (() => void) | undefined;
 
@@ -37,9 +37,8 @@ const debouncedNotifyAndReset = debounce(async (action: ActionType) => {
 
 	await notify(message, { type });
 
-	// Reset the success flag and clear the set after the debounce period.
+	// Reset the success flag after the debounce period.
 	notifySuccess = false;
-	modifiedSelectors.clear();
 
 	if (batchUpdatePromiseResolve) {
 		batchUpdatePromiseResolve();
@@ -50,35 +49,34 @@ const debouncedNotifyAndReset = debounce(async (action: ActionType) => {
 
 async function updateCustomSelectors(
 	action: ActionType,
-	pattern: string,
-	selectors?: CustomSelectorsForPattern
+	url: string,
+	selectors?: CustomSelector[]
 ) {
 	const selectorsAffected = await withLockedStorageAccess(
 		"customSelectors",
 		async (customSelectors) => {
-			const selectorsForPattern = customSelectors.get(pattern) ?? {
-				include: [],
-				exclude: [],
-			};
-
 			if (action === "store") {
 				if (!selectors) throw new Error("No selectors provided to store");
+				customSelectors.push(...selectors);
 
-				selectorsForPattern.include = Array.from(
-					new Set([...selectorsForPattern.include, ...selectors.include])
-				);
-				selectorsForPattern.exclude = Array.from(
-					new Set([...selectorsForPattern.exclude, ...selectors.exclude])
-				);
-				customSelectors.set(pattern, selectorsForPattern);
-				return [...selectors.include, ...selectors.exclude];
+				return selectors;
 			}
 
 			if (action === "reset") {
-				customSelectors.delete(pattern);
-				return selectorsForPattern
-					? [...selectorsForPattern.include, ...selectorsForPattern.exclude]
-					: [];
+				const selectorsForPattern =
+					customSelectors.filter(({ pattern }) => {
+						const patternRe = new RegExp(pattern);
+						return patternRe.test(url);
+					}) ?? [];
+
+				// We need to filter the array in place because assigning would just
+				// modify the argument.
+				filterInPlace(customSelectors, ({ pattern }) => {
+					const patternRe = new RegExp(pattern);
+					return !patternRe.test(url);
+				});
+
+				return selectorsForPattern ?? [];
 			}
 
 			return assertNever(action);
@@ -88,10 +86,6 @@ async function updateCustomSelectors(
 	// Update notifySuccess to true if any of the calls within the debounce period is successful.
 	if (selectorsAffected.length > 0) notifySuccess = true;
 
-	for (const selector of selectorsAffected) {
-		modifiedSelectors.add(selector);
-	}
-
 	await debouncedNotifyAndReset(action);
 
 	if (batchUpdatePromise) {
@@ -105,21 +99,21 @@ async function updateCustomSelectors(
 }
 
 /**
- * Stores the custom selectors for the given URL pattern. It handles being
+ * Stores the custom selectors for the given URL. It handles being
  * called multiple times to handle multiple frames wanting to change the custom
  * selectors. It waits for a sequence of calls to finish before returning. Once
  * calls have stopped it notifies if storing the custom selectors was
  * successful, that is if any of the calls in the sequence resulted in custom
  * selectors being added.
  *
- * @param pattern The URL pattern where selectors apply
- * @param selectors An object with `include` and `exclude` CSS selectors for the given pattern
+ * @param url The URL where selectors apply
+ * @param selectors The selectors to store for the URL
  */
 export async function storeCustomSelectors(
-	pattern: string,
-	selectors: CustomSelectorsForPattern
+	url: string,
+	selectors: CustomSelector[]
 ) {
-	await updateCustomSelectors("store", pattern, selectors);
+	await updateCustomSelectors("store", url, selectors);
 }
 
 /**
@@ -130,8 +124,8 @@ export async function storeCustomSelectors(
  * successful, that is if any of the calls in the sequence resulted in custom
  * selectors being removed.
  *
- * @param pattern The URL pattern where selectors apply
+ * @param url The URL pattern where selectors apply
  */
-export async function resetCustomSelectors(pattern: string) {
-	await updateCustomSelectors("reset", pattern);
+export async function resetCustomSelectors(url: string) {
+	await updateCustomSelectors("reset", url);
 }

src/common/settings.ts

@@ -1,8 +1,5 @@
 import Color from "color";
-import {
-	CustomSelectorsForPattern,
-	StorageSchema,
-} from "../typings/StorageSchema";
+import { CustomSelector, StorageSchema } from "../typings/StorageSchema";
 
 export const defaultSettings = {
 	hintUppercaseLetters: false,
@@ -36,7 +33,7 @@ export const defaultSettings = {
 	uppercaseTabMarkers: true,
 	keyboardClicking: false,
 	keysToExclude: new Array<[string, string]>(),
-	customSelectors: new Map<string, CustomSelectorsForPattern>(),
+	customSelectors: new Array<CustomSelector>(),
 	customScrollPositions: new Map<string, Map<string, number>>(),
 	references: new Map<string, Map<string, string>>(),
 	showWhatsNewPageOnUpdate: true,

src/common/storage.ts

@@ -1,18 +1,18 @@
 /* eslint-disable no-await-in-loop */
 import { Mutex } from "async-mutex";
 import browser from "webextension-polyfill";
-import {
-	CustomSelectorsForPattern,
-	StorageSchema,
-	zStorageSchema,
-} from "../typings/StorageSchema";
+import { StorageSchema, zStorageSchema } from "../typings/StorageSchema";
 import { defaultStorage } from "./defaultStorage";
 import {
 	Settings,
 	defaultSettings,
 	isSetting,
 	isValidSetting,
 } from "./settings";
+import {
+	prepareSettingForStoring,
+	upgradeCustomSelectors,
+} from "./transformSettings";
 
 const useLocalStorage = new Set<keyof StorageSchema>([
 	"hintsToggleTabs",
@@ -56,16 +56,17 @@ export async function store<T extends keyof StorageSchema>(
 ): Promise<StorageSchema[T]> {
 	if (isSetting(key) && !isValidSetting(key, value)) return retrieve(key);
 
+	const prepared = prepareSettingForStoring(key, value);
 	const stringified = JSON.stringify(
-		zStorageSchema.shape[key].parse(value),
+		zStorageSchema.shape[key].parse(prepared),
 		replacer
 	);
 
 	await (useLocalStorage.has(key)
 		? browser.storage.local.set({ [key]: stringified })
 		: browser.storage.sync.set({ [key]: stringified }));
 
-	return value;
+	return prepared;
 }
 
 async function parseStorageItem(key: keyof StorageSchema) {
@@ -98,17 +99,12 @@ async function parseStorageItem(key: keyof StorageSchema) {
 async function initStorageItem<T extends keyof StorageSchema>(key: T) {
 	const item = await parseStorageItem(key);
 
-	// Handle customSelectors type conversion from an object to a Map. This is
-	// only necessary temporarily in order not to lose user's customizations.
-	// Introduced in v0.5.0.
-	if (item && key === "customSelectors") {
+	if (key === "customSelectors") {
 		try {
-			const normalized = new Map<string, CustomSelectorsForPattern>(
-				Object.entries(item as Record<string, CustomSelectorsForPattern>)
-			) as StorageSchema[T];
+			const upgraded = upgradeCustomSelectors(item);
 
 			const parsed = zStorageSchema.shape[key].parse(
-				normalized
+				upgraded
 			) as StorageSchema[T];
 			return await store(key, parsed);
 		} catch {

src/common/transformSettings.test.ts

@@ -0,0 +1,53 @@
+import { upgradeCustomSelectors } from "./transformSettings";
+
+describe("upgradeCustomSelectors", () => {
+	test("upgrades custom selectors from plain object format", () => {
+		const value = {
+			pattern1: { include: ["selector1", "selector2"], exclude: ["selector3"] },
+			pattern2: { include: ["selector4"], exclude: ["selector5", "selector6"] },
+		};
+
+		const upgraded = upgradeCustomSelectors(value);
+
+		expect(upgraded).toEqual([
+			{ pattern: "pattern1", type: "include", selector: "selector1" },
+			{ pattern: "pattern1", type: "include", selector: "selector2" },
+			{ pattern: "pattern1", type: "exclude", selector: "selector3" },
+			{ pattern: "pattern2", type: "include", selector: "selector4" },
+			{ pattern: "pattern2", type: "exclude", selector: "selector5" },
+			{ pattern: "pattern2", type: "exclude", selector: "selector6" },
+		]);
+	});
+
+	test("upgrades custom selectors from Map format", () => {
+		const value = new Map([
+			[
+				"pattern1",
+				{ include: ["selector1", "selector2"], exclude: ["selector3"] },
+			],
+			[
+				"pattern2",
+				{ include: ["selector4"], exclude: ["selector5", "selector6"] },
+			],
+		]);
+
+		const upgraded = upgradeCustomSelectors(value);
+
+		expect(upgraded).toEqual([
+			{ pattern: "pattern1", type: "include", selector: "selector1" },
+			{ pattern: "pattern1", type: "include", selector: "selector2" },
+			{ pattern: "pattern1", type: "exclude", selector: "selector3" },
+			{ pattern: "pattern2", type: "include", selector: "selector4" },
+			{ pattern: "pattern2", type: "exclude", selector: "selector5" },
+			{ pattern: "pattern2", type: "exclude", selector: "selector6" },
+		]);
+	});
+
+	test("returns original value if not upgradable", () => {
+		const value = "invalid value";
+
+		const upgraded = upgradeCustomSelectors(value);
+
+		expect(upgraded).toBe(value);
+	});
+});