Repository: web-scrobbler/web-scrobbler
Pull request #3796 (2023-05-28 10:57:53)

Fix regex edit flag conditions
Previous flag was not triggering flagging the song as edited when there was no actual change to the field content.
This broke using regex as overrides.


Patch:
diff --git a/scripts/create-distributable.ts b/scripts/create-distributable.ts
index 8663b072..c6b82fac 100644
--- a/scripts/create-distributable.ts
+++ b/scripts/create-distributable.ts
@@ -16,7 +16,9 @@ async function createSrcArchive() {
 	const output = fs.createWriteStream(outputFile);
 
 	const archive = archiver('zip', {
-		zlib: { level: 0 },
+		zlib: {
+			level: 5,
+		},
 	});
 
 	archive.pipe(output);
@@ -45,7 +47,9 @@ export default async function createDistributable() {
 	const output = fs.createWriteStream(outputFile);
 
 	const archive = archiver('zip', {
-		zlib: { level: 0 },
+		zlib: {
+			level: 5,
+		},
 	});
 
 	archive.pipe(output);
diff --git a/src/core/background/action.ts b/src/core/background/action.ts
index 94651131..fdda163b 100644
--- a/src/core/background/action.ts
+++ b/src/core/background/action.ts
@@ -1,5 +1,5 @@
 import browser from 'webextension-polyfill';
-import { contextMenus, getBrowserPreferredTheme, getCurrentTab } from './util';
+import { contextMenus, getBrowserPreferredTheme } from './util';
 import { sendPopupMessage } from '@/util/communication';
 import { ManagerTab } from '@/core/storage/wrapper';
 import * as ControllerMode from '@/core/object/controller/controller-mode';
@@ -24,13 +24,6 @@ export async function performUpdateAction(tab: ManagerTab) {
 	setAction(tab);
 }
 
-/**
- * Fetches the tab to update action state from, and then updates the action state
- */
-export async function updateAction() {
-	await getCurrentTab();
-}
-
 /**
  * Updates which context menu items are displayed.
  *
diff --git a/src/core/background/main.ts b/src/core/background/main.ts
index 20bad3b5..f4465697 100644
--- a/src/core/background/main.ts
+++ b/src/core/background/main.ts
@@ -1,10 +1,8 @@
-import { getConnectorByUrl } from '@/util/util-connector';
 import * as ControllerMode from '@/core/object/controller/controller-mode';
 import * as BrowserStorage from '@/core/storage/browser-storage';
 import { ManagerTab } from '@/core/storage/wrapper';
 import {
 	backgroundListener,
-	sendBackgroundMessage,
 	setupBackgroundListeners,
 } from '@/util/communication';
 import browser from 'webextension-polyfill';
@@ -12,16 +10,21 @@ import {
 	DEFAULT_STATE,
 	contextMenus,
 	getState,
-	filterAsync,
 	filterInactiveTabs,
-	getCurrentTab,
 	setState,
 	unlockState,
 	enableConnector,
 	disableConnector,
 	disableUntilClosed,
+	updateTabsFromTabList,
+	getCurrentTab,
+	getActiveTabDetails,
+	getCurrentTabId,
 } from './util';
-import { ControllerModeStr } from '@/core/object/controller/controller';
+import {
+	ControllerModeStr,
+	controllerModePriorityObject,
+} from '@/core/object/controller/controller';
 import { CloneableSong } from '@/core/object/song';
 import {
 	clearNowPlaying,
@@ -30,7 +33,6 @@ import {
 } from '@/util/notifications';
 import ClonedSong from '@/core/object/cloned-song';
 import { openTab } from '@/util/util-browser';
-import { updateAction } from './action';
 import { setRegexDefaults } from '@/util/regex';
 
 const disabledTabs = BrowserStorage.getStorage(BrowserStorage.DISABLED_TABS);
@@ -38,9 +40,9 @@ const disabledTabs = BrowserStorage.getStorage(BrowserStorage.DISABLED_TABS);
 // Set up listeners. These must all be synchronously set up at startup time (Manifest V3 service worker)
 browser.runtime.onStartup.addListener(startupFunc);
 browser.runtime.onInstalled.addListener(startupFunc);
-browser.tabs.onRemoved.addListener(onRemovedUpdate);
-browser.tabs.onUpdated.addListener(updateTabList);
-browser.tabs.onActivated.addListener(onActivatedUpdate);
+browser.tabs.onRemoved.addListener(onTabRemoved);
+browser.tabs.onUpdated.addListener(onTabUpdated);
+browser.tabs.onActivated.addListener(onTabActivated);
 browser.contextMenus?.onClicked.addListener(contextMenuHandler);
 
 /**
@@ -48,14 +50,16 @@ browser.contextMenus?.onClicked.addListener(contextMenuHandler);
  *
  * @param tabId - tab ID of closed tab
  */
-async function onRemovedUpdate(tabId: number) {
+async function onTabRemoved(tabId: number) {
 	const curState = await getState();
 
+	const filteredTabs = await filterInactiveTabs(curState.activeTabs);
 	await setState({
-		activeTabs: await filterInactiveTabs(curState.activeTabs),
+		activeTabs: filteredTabs,
 		browserPreferredTheme: curState.browserPreferredTheme,
 	});
-	updateAction();
+	const newActiveTab = await getCurrentTabId();
+	updateTabsFromTabList(filteredTabs, newActiveTab);
 
 	const tabs = await disabledTabs.get();
 	if (tabs?.[tabId]) {
@@ -69,10 +73,10 @@ async function onRemovedUpdate(tabId: number) {
  *
  * @param activeInfo - Information about the switch of tabs
  */
-async function onActivatedUpdate(
+async function onTabActivated(
 	activeInfo: browser.Tabs.OnActivatedActiveInfoType
 ) {
-	await updateTabList(activeInfo.tabId);
+	await updateTabList(activeInfo.tabId, true);
 }
 
 /**
@@ -80,36 +84,53 @@ async function onActivatedUpdate(
  * We generally want information about more recently used tabs displayed first.
  *
  * @param tabId - currently active tab.
- * @param _ - unused
+ * @param changeInfo - information about the change in tab.
  * @param tab - currently active tab details, if they exist.
  */
-async function updateTabList(tabId: number, _?: any, tab?: browser.Tabs.Tab) {
+async function onTabUpdated(
+	tabId: number,
+	changeInfo: browser.Tabs.OnUpdatedChangeInfoType,
+	tab?: browser.Tabs.Tab
+) {
+	if (tab?.active && changeInfo.status === 'complete') {
+		await updateTabList(tabId, false);
+	}
+}
+
+/**
+ * Update the active tabs list to reflect the order of priority.
+ * We generally want information about more recently used tabs displayed first.
+ *
+ * @param tabId - currently active tab.
+ * @param activated - whether the tab was just activated or not.
+ */
+async function updateTabList(tabId: number, activated: boolean) {
 	const curState = await getState();
-	let curTab: ManagerTab = {
-		tabId,
-		mode: ControllerMode.Unsupported,
-		song: null,
-	};
-	let newTabs =
-		(await filterAsync(curState.activeTabs, async (active) => {
-			if (active.tabId !== tabId) {
-				return true;
-			}
+	let newTabs = activated
+		? curState.activeTabs.filter((active) => active.tabId !== tabId)
+		: curState.activeTabs;
 
-			const connector = await getConnectorByUrl(tab?.url ?? '');
-			if (!tab || connector) {
-				curTab = active;
-			}
-			return false;
-		})) ?? [];
+	const curTab = await getActiveTabDetails(newTabs, tabId);
 
-	newTabs = [curTab, ...newTabs];
+	if (
+		controllerModePriorityObject[curTab.mode] !== 0 &&
+		curTab.tabId === tabId
+	) {
+		if (activated) {
+			newTabs = [curTab, ...newTabs];
+		} else {
+			newTabs = newTabs.map((active) =>
+				active.tabId === tabId ? curTab : active
+			);
+		}
+	}
+	const filteredTabs = await filterInactiveTabs(newTabs);
 
 	await setState({
-		activeTabs: await filterInactiveTabs(newTabs),
+		activeTabs: filteredTabs,
 		browserPreferredTheme: curState.browserPreferredTheme,
 	});
-	updateAction();
+	updateTabsFromTabList(filteredTabs, tabId);
 }
 
 /**
@@ -142,22 +163,23 @@ async function updateTab(
 				activeTabs,
 				browserPreferredTheme: curState.browserPreferredTheme,
 			});
-			updateAction();
+			updateTabsFromTabList(activeTabs, tabId);
 			return;
 		}
 		performedSet = true;
+		const newTabs = [
+			fn({
+				tabId,
+				mode: ControllerMode.Unsupported,
+				song: null,
+			}),
+			...activeTabs,
+		];
 		await setState({
-			activeTabs: [
-				fn({
-					tabId,
-					mode: ControllerMode.Unsupported,
-					song: null,
-				}),
-				...activeTabs,
-			],
+			activeTabs: newTabs,
 			browserPreferredTheme: curState.browserPreferredTheme,
 		});
-		updateAction();
+		updateTabsFromTabList(activeTabs, tabId);
 	} catch (err) {
 		if (!performedSet) {
 			unlockState();
diff --git a/src/core/background/util.ts b/src/core/background/util.ts
index 1d91a273..fe60af50 100644
--- a/src/core/background/util.ts
+++ b/src/core/background/util.ts
@@ -3,7 +3,10 @@ import { ManagerTab, StateManagement } from '@/core/storage/wrapper';
 import browser from 'webextension-polyfill';
 import * as ControllerMode from '@/core/object/controller/controller-mode';
 import * as BrowserStorage from '@/core/storage/browser-storage';
-import { controllerModePriority } from '@/core/object/controller/controller';
+import {
+	controllerModePriority,
+	controllerModePriorityObject,
+} from '@/core/object/controller/controller';
 import { performUpdateAction } from './action';
 import { sendBackgroundMessage } from '@/util/communication';
 
@@ -77,6 +80,117 @@ export async function setState(data: StateManagement): Promise<void> {
 	return state.setLocking(data);
 }
 
+/**
+ * Takes a list of tabs and updates the action state based on the first tab that matches a priority group.
+ *
+ * @param tabs - tabs from state
+ * @param tabId - Currently active tab id if known
+ * @returns the tab that was used to update the action state
+ */
+export async function updateTabsFromTabList(
+	tabs: ManagerTab[],
+	tabId?: number
+) {
+	const curTab = await getActiveTabDetails(tabs, tabId);
+	performUpdateAction(curTab);
+	return curTab;
+}
+
+/**
+ * Takes a list of tabs and returns the details of the first tab that matches a priority group.
+ *
+ * @param tabs - tabs from state
+ * @param tabId - Currently active tab id if known
+ * @returns the tab that was used to update the action state
+ */
+export async function getActiveTabDetails(
+	tabs: ManagerTab[],
+	tabId?: number
+): Promise<ManagerTab> {
+	const tab = getPriorityTabDetails(tabs, tabId);
+	if (!tabId) {
+		tabId = await getCurrentTabId();
+	}
+	const curTab = await getTabDetails(tabId);
+	if (
+		tab &&
+		controllerModePriorityObject[tab.mode] >
+			controllerModePriorityObject[curTab.mode]
+	) {
+		return tab;
+	}
+	return curTab;
+}
+
+/**
+ * Get the controller state of a tab.
+ * @param tabId - tab to get state of
+ * @returns the tab with controller details
+ */
+async function getTabDetails(tabId: number): Promise<ManagerTab> {
+	try {
+		const tab = await browser.tabs.get(tabId);
+		if (!tab) {
+			throw new Error('Tab not found');
+		}
+		const tabState = await sendBackgroundMessage(tabId, {
+			type: 'getConnectorDetails',
+			payload: undefined,
+		});
+		const curTab: ManagerTab = {
+			tabId,
+			mode: tabState.mode,
+			song: tabState.song,
+		};
+		return curTab;
+	} catch (err) {
+		return {
+			tabId,
+			mode: ControllerMode.Unsupported,
+			song: null,
+		};
+	}
+}
+
+/**
+ * Get the highest priority priority tab for action state.
+ * This tab will be prioritized over the active tab.
+ * @param tabs - list of priority tabs from state
+ * @param tabId - Currently active tab id if known
+ * @returns the tab that is the current priority for action state
+ */
+function getPriorityTabDetails(
+	tabs: ManagerTab[],
+	tabId?: number
+): ManagerTab | null {
+	for (const priorityGroup of controllerModePriority.slice(0, -1)) {
+		let toReturn: ManagerTab | null = null;
+		for (const tab of tabs) {
+			if (priorityGroup.includes(tab.mode)) {
+				if (tabId && tab.tabId === tabId) {
+					return tab;
+				}
+				toReturn = tab;
+			}
+		}
+		if (toReturn) {
+			return toReturn;
+		}
+	}
+	return null;
+}
+
+/**
+ * Returns the ID of the currently active tab in the browser.
+ * @returns the current tab id
+ */
+export async function getCurrentTabId() {
+	const queryOptions = { active: true, lastFocusedWindow: true };
+	// `tab` will either be a `tabs.Tab` instance or `undefined`.
+	const [tab] = await browser.tabs.query(queryOptions);
+	return tab?.id ?? -1;
+}
+
 /**
  * Gets the tab to base action and context menus on.
  * Generally, this prioritizes the most recent tab.
@@ -86,27 +200,7 @@ export async function setState(data: StateManagement): Promise<void> {
  */
 export async function getCurrentTab(): Promise<ManagerTab> {
 	const curState = await getState();
-	const filteredTabs = await filterInactiveTabs(curState.activeTabs);
-	await setState({
-		activeTabs: filteredTabs,
-		browserPreferredTheme: curState.browserPreferredTheme,
-	});
-
-	for (const priorityGroup of controllerModePriority) {
-		for (const tab of filteredTabs) {
-			if (priorityGroup.includes(tab.mode)) {
-				performUpdateAction(tab);
-				return tab;
-			}
-		}
-	}
-	const defaultTab: ManagerTab = {
-		tabId: -1,
-		mode: ControllerMode.Unsupported,
-		song: null,
-	};
-	performUpdateAction(defaultTab);
-	return defaultTab;
+	return getActiveTabDetails(curState.activeTabs);
 }
 
 /**
diff --git a/src/core/content/main.ts b/src/core/content/main.ts
index 462d1405..16171ba0 100644
--- a/src/core/content/main.ts
+++ b/src/core/content/main.ts
@@ -6,6 +6,8 @@ import start from '@/core/content/starter';
 import browser from 'webextension-polyfill';
 import scrobbleService from '@/core/object/scrobble-service';
 import { sendContentMessage } from '@/util/communication';
+import savedEdits from '../storage/saved-edits';
+import regexEdits from '../storage/regex-edits';
 
 main();
 async function main() {
@@ -29,11 +31,18 @@ async function fetchConnector(): Promise<void> {
 		return;
 	}
 
+	// Don't run the connector in frames if it's not allowed to run in frames
+	if (window !== top && !connector.allFrames) {
+		return;
+	}
+
 	window.Connector = new BaseConnector(connector);
 	window.Util = Util;
 	window.MetadataFilter = MetadataFilter;
 	window.webScrobblerScripts = {};
 	await scrobbleService.bindAllScrobblers();
+	savedEdits.init();
+	regexEdits.init();
 
 	try {
 		await import(browser.runtime.getURL(`connectors/${connector?.js}`));
diff --git a/src/core/object/controller/controller.ts b/src/core/object/controller/controller.ts
index 5dc19fd1..fd43d5b6 100644
--- a/src/core/object/controller/controller.ts
+++ b/src/core/object/controller/controller.ts
@@ -52,6 +52,22 @@ export const controllerModePriority: ControllerModeStr[][] = [
 	],
 ];
 
+/**
+ * Priorities of each state as an object
+ */
+export const controllerModePriorityObject: Record<ControllerModeStr, number> = {
+	[ControllerMode.Base]: 0,
+	[ControllerMode.Skipped]: 0,
+	[ControllerMode.Disabled]: 0,
+	[ControllerMode.Unsupported]: 0,
+	[ControllerMode.Playing]: 1,
+	[ControllerMode.Scrobbled]: 1,
+	[ControllerMode.Loading]: 2,
+	[ControllerMode.Unknown]: 3,
+	[ControllerMode.Ignored]: 4,
+	[ControllerMode.Err]: 4,
+};
+
 type updateEvent = {
 	updateEditStatus: (isEditing: boolean) => void;
 };
@@ -145,6 +161,13 @@ export default class Controller {
 			contentListener({
 				type: 'disableConnectorUntilTabIsClosed',
 				fn: () => this.disableUntilTabIsClosed(),
+			}),
+			contentListener({
+				type: 'getConnectorDetails',
+				fn: () => ({
+					mode: this.mode,
+					song: this.currentSong?.getCloneableData() ?? null,
+				}),
 			})
 		);
 	}
diff --git a/src/core/storage/custom-storage.ts b/src/core/storage/custom-storage.ts
index 7835596b..1cf2234d 100644
--- a/src/core/storage/custom-storage.ts
+++ b/src/core/storage/custom-storage.ts
@@ -1,13 +1,21 @@
 import StorageWrapper, { DataModels } from '@/core/storage/wrapper';
+import { debugLog } from '@/util/util';
 
 /**
  * This class provides a base interface for custom storage wrappers.
  */
 export abstract class CustomStorage<K extends keyof DataModels> {
-	public storageRef: StorageWrapper<K>;
+	public storageRef: StorageWrapper<K> | null;
 
-	constructor(storageRef: StorageWrapper<K>) {
+	constructor() {
+		this.storageRef = null;
+	}
+
+	abstract init(): void;
+
+	protected _init(storageRef: StorageWrapper<K>): void {
 		this.storageRef = storageRef;
+
 		/* @ifdef DEVELOPMENT */
 		void this.storageRef.debugLog();
 		/* @endif */
@@ -22,6 +30,9 @@ export abstract class CustomStorage<K extends keyof DataModels> {
 	 * Remove all data from the storage.
 	 */
 	async clear(): Promise<void> {
+		if (!this.storageRef) {
+			return;
+		}
 		return await this.storageRef.clear();
 	}
 
@@ -31,6 +42,10 @@ export abstract class CustomStorage<K extends keyof DataModels> {
 	 * @returns Storage data
 	 */
 	async getData(): Promise<DataModels[K] | null> {
+		if (!this.storageRef) {
+			debugLog('Storage reference is not initialized', 'warn');
+			return null;
+		}
 		return await this.storageRef.get();
 	}
 
@@ -41,6 +56,10 @@ export abstract class CustomStorage<K extends keyof DataModels> {
 	 * @returns Storage data
 	 */
 	async saveData(data: DataModels[K]): Promise<void> {
+		if (!this.storageRef) {
+			debugLog('Storage reference is not initialized', 'warn');
+			return;
+		}
 		return await this.storageRef.set(data);
 	}
 
@@ -51,6 +70,10 @@ export abstract class CustomStorage<K extends keyof DataModels> {
 	 * @returns Storage data
 	 */
 	async updateData(data: DataModels[K]): Promise<void> {
+		if (!this.storageRef) {
+			debugLog('Storage reference is not initialized', 'warn');
+			return;
+		}
 		return await this.storageRef.update(data);
 	}
 }
diff --git a/src/core/storage/regex-edits.ts b/src/core/storage/regex-edits.ts
index a9de4047..77e21440 100644
--- a/src/core/storage/regex-edits.ts
+++ b/src/core/storage/regex-edits.ts
@@ -11,8 +11,8 @@ type T = Record<K, V>;
 class RegexEditsImpl extends RegexEditsModel {
 	private regexEditStorage = this.getStorage();
 
-	constructor() {
-		super(BrowserStorage.getLocalStorage(BrowserStorage.REGEX_EDITS));
+	public init() {
+		this._init(BrowserStorage.getLocalStorage(BrowserStorage.REGEX_EDITS));
 
 		/* @ifdef DEBUG */
 		void this.regexEditStorage.debugLog();
diff --git a/src/core/storage/saved-edits.ts b/src/core/storage/saved-edits.ts
index f9ddaa64..12dfae1c 100644
--- a/src/core/storage/saved-edits.ts
+++ b/src/core/storage/saved-edits.ts
@@ -12,8 +12,8 @@ class SavedEditsImpl extends SavedEditsModel {
 		BrowserStorage.LOCAL_CACHE
 	);
 
-	constructor() {
-		super(BrowserStorage.getLocalStorage(BrowserStorage.LOCAL_CACHE));
+	public init() {
+		this._init(BrowserStorage.getLocalStorage(BrowserStorage.LOCAL_CACHE));
 
 		/* @ifdef DEBUG */
 		void this.songInfoStorage.debugLog();
diff --git a/src/core/storage/wrapper.ts b/src/core/storage/wrapper.ts
index d164ba6a..6eea787f 100644
--- a/src/core/storage/wrapper.ts
+++ b/src/core/storage/wrapper.ts
@@ -146,6 +146,9 @@ export default class StorageWrapper<K extends keyof DataModels> {
 	 * @returns Storage data
 	 */
 	async get(): Promise<DataModels[K] | null> {
+		// #v-ifdef VITE_DEV
+		console.log('StorageWrapper#get: ', this.namespace);
+		// #v-endif
 		const data = await this.storage.get();
 		if (data && this.namespace in data) {
 			return data[this.namespace] as DataModels[K];
diff --git a/src/util/communication.ts b/src/util/communication.ts
index aed89096..a85a1080 100644
--- a/src/util/communication.ts
+++ b/src/util/communication.ts
@@ -86,6 +86,13 @@ interface BackgroundCommunications {
 		payload: boolean;
 		response: void;
 	};
+	getConnectorDetails: {
+		payload: undefined;
+		response: {
+			mode: ControllerModeStr;
+			song: CloneableSong | null;
+		};
+	};
 	disableConnectorUntilTabIsClosed: {
 		payload: undefined;
 		response: void;
diff --git a/src/util/regex.ts b/src/util/regex.ts
index 8f3544a1..c38cb887 100644
--- a/src/util/regex.ts
+++ b/src/util/regex.ts
@@ -196,7 +196,7 @@ export function editSong(edit: RegexEdit, song: Song) {
 
 	for (const [_key, field] of Object.entries(fields)) {
 		const key = _key as FieldType;
-		if (field !== getSongField(song, key)) {
+		if (edit.search[key] && edit.replace[key]) {
 			song.flags.isRegexEditedByUser[key] = true;
 			song.processed[key] = field;
 		}


Test patch:
diff --git a/tests/background/regex-edits.test.ts b/tests/background/regex-edits.test.ts
index d38b989e..541fd12b 100644
--- a/tests/background/regex-edits.test.ts
+++ b/tests/background/regex-edits.test.ts
@@ -20,6 +20,7 @@ describe('Should edit Regex', () => {
 	beforeEach(() => {
 		webextensionPolyfill.reset();
 		webextensionPolyfill.setUser();
+		regexEdits.init();
 	});
 
 	it('Should add edits to storage', async () => {
diff --git a/tests/background/saved-edits.test.ts b/tests/background/saved-edits.test.ts
index b52fb1d5..64b91699 100644
--- a/tests/background/saved-edits.test.ts
+++ b/tests/background/saved-edits.test.ts
@@ -21,6 +21,9 @@ const connectorStub = getConnectorById('youtube')!;
  * Run all tests.
  */
 function runTests() {
+	beforeAll(() => {
+		savedEdits.init();
+	});
 	describe('should throw an error for empty songs', testSaveEmptySong);
 
 	describe('clear storage', testClearStorage);