feat(scrobblers): use BrowserWindow instead of shell.openExternal (#1758)

src/i18n/resources/en.json

@@ -579,6 +579,14 @@
     },
     "scrobbler": {
       "description": "Add scrobbling support (etc. last.fm, Listenbrainz)",
+      "dialog": {
+        "lastfm": {
+          "auth-failed": {
+            "title": "Authentication Failed",
+            "message": "Failed to authenticate with Last.fm\nHide the popup until the next restart."
+          }
+        }
+      },
       "menu": {
         "scrobble-other-media": "Scrobble other media",
         "lastfm": {

src/plugins/scrobbler/main.ts

@@ -1,10 +1,13 @@
+import { BrowserWindow } from 'electron';
+
 import registerCallback, { MediaType, type SongInfo } from '@/providers/song-info';
 import { createBackend } from '@/utils';
 
-import { ScrobblerPluginConfig } from './index';
 import { LastFmScrobbler } from './services/lastfm';
 import { ListenbrainzScrobbler } from './services/listenbrainz';
-import { ScrobblerBase } from './services/base';
+
+import type { ScrobblerPluginConfig } from './index';
+import type { ScrobblerBase } from './services/base';
 
 export type SetConfType = (
   conf: Partial<Omit<ScrobblerPluginConfig, 'enabled'>>,
@@ -12,14 +15,17 @@ export type SetConfType = (
 
 export const backend = createBackend<{
   config?: ScrobblerPluginConfig;
+  window?: BrowserWindow;
   enabledScrobblers: Map<string, ScrobblerBase>;
-  toggleScrobblers(config: ScrobblerPluginConfig): void;
+  toggleScrobblers(config: ScrobblerPluginConfig, window: BrowserWindow): void;
+  createSessions(config: ScrobblerPluginConfig, setConfig: SetConfType): Promise<void>;
+  setConfig?: SetConfType;
 }, ScrobblerPluginConfig>({
   enabledScrobblers: new Map(),
 
-  toggleScrobblers(config: ScrobblerPluginConfig) {
+  toggleScrobblers(config: ScrobblerPluginConfig, window: BrowserWindow) {
     if (config.scrobblers.lastfm && config.scrobblers.lastfm.enabled) {
-      this.enabledScrobblers.set('lastfm', new LastFmScrobbler());
+      this.enabledScrobblers.set('lastfm', new LastFmScrobbler(window));
     } else {
       this.enabledScrobblers.delete('lastfm');
     }
@@ -31,20 +37,27 @@ export const backend = createBackend<{
     }
   },
 
-  async start({
+  async createSessions(config: ScrobblerPluginConfig, setConfig: SetConfType) {
-    getConfig,
-    setConfig,
-  }) {
-    const config = this.config = await getConfig();
-    // This will store the timeout that will trigger addScrobble
-    let scrobbleTimer: NodeJS.Timeout | undefined;
-
-    this.toggleScrobblers(config);
     for (const [, scrobbler] of this.enabledScrobblers) {
       if (!scrobbler.isSessionCreated(config)) {
         await scrobbler.createSession(config, setConfig);
       }
     }
+  },
+
+  async start({
+    getConfig,
+    setConfig,
+    window,
+  }) {
+    const config = this.config = await getConfig();
+    // This will store the timeout that will trigger addScrobble
+    let scrobbleTimer: NodeJS.Timeout | undefined;
+
+    this.window = window;
+    this.toggleScrobblers(config, window);
+    await this.createSessions(config, setConfig);
+    this.setConfig = setConfig;
 
     registerCallback((songInfo: SongInfo) => {
       // Set remove the old scrobble timer
@@ -52,7 +65,7 @@ export const backend = createBackend<{
       if (!songInfo.isPaused) {
         const configNonnull = this.config!;
         // Scrobblers normally have no trouble working with official music videos
-        if (!configNonnull.scrobble_other_media && (songInfo.mediaType !== MediaType.Audio && songInfo.mediaType !== MediaType.OriginalMusicVideo)) {
+        if (!configNonnull.scrobbleOtherMedia && (songInfo.mediaType !== MediaType.Audio && songInfo.mediaType !== MediaType.OriginalMusicVideo)) {
           return;
         }
 
@@ -71,12 +84,25 @@ export const backend = createBackend<{
     });
   },
 
-  onConfigChange(newConfig: ScrobblerPluginConfig) {
+  async onConfigChange(newConfig: ScrobblerPluginConfig) {
     this.enabledScrobblers.clear();
 
-    this.config = newConfig;
+    this.toggleScrobblers(newConfig, this.window!);
+    for (const [scrobblerName, scrobblerConfig] of Object.entries(newConfig.scrobblers)) {
+      if (scrobblerConfig.enabled) {
+        const scrobbler = this.enabledScrobblers.get(scrobblerName);
+        if (
+          this.config?.scrobblers?.[scrobblerName as keyof typeof newConfig.scrobblers]?.enabled !== scrobblerConfig.enabled &&
+          scrobbler &&
+          !scrobbler.isSessionCreated(newConfig) &&
+          this.setConfig
+        ) {
+          await scrobbler.createSession(newConfig, this.setConfig);
+        }
+      }
+    }
 
-    this.toggleScrobblers(this.config);
+    this.config = newConfig;
   }
 });
 

src/plugins/scrobbler/menu.ts

@@ -20,7 +20,7 @@ async function promptLastFmOptions(options: ScrobblerPluginConfig, setConfig: Se
       multiInputOptions: [
         {
           label: t('plugins.scrobbler.prompt.lastfm.api-key'),
-          value: options.scrobblers.lastfm?.api_key,
+          value: options.scrobblers.lastfm?.apiKey,
           inputAttrs: {
             type: 'text'
           }
@@ -42,7 +42,7 @@ async function promptLastFmOptions(options: ScrobblerPluginConfig, setConfig: Se
 
   if (output) {
     if (output[0]) {
-      options.scrobblers.lastfm.api_key = output[0];
+      options.scrobblers.lastfm.apiKey = output[0];
     }
 
     if (output[1]) {
@@ -82,9 +82,9 @@ export const onMenu = async ({
     {
       label: t('plugins.scrobbler.menu.scrobble-other-media'),
       type: 'checkbox',
-      checked: Boolean(config.scrobble_other_media),
+      checked: Boolean(config.scrobbleOtherMedia),
       click(item) {
-        config.scrobble_other_media = item.checked;
+        config.scrobbleOtherMedia = item.checked;
         setConfig(config);
       },
     },
@@ -96,7 +96,7 @@ export const onMenu = async ({
           type: 'checkbox',
           checked: Boolean(config.scrobblers.lastfm?.enabled),
           click(item) {
-            backend.toggleScrobblers(config);
+            backend.toggleScrobblers(config, window);
             config.scrobblers.lastfm.enabled = item.checked;
             setConfig(config);
           },
@@ -117,7 +117,7 @@ export const onMenu = async ({
           type: 'checkbox',
           checked: Boolean(config.scrobblers.listenbrainz?.enabled),
           click(item) {
-            backend.toggleScrobblers(config);
+            backend.toggleScrobblers(config, window);
             config.scrobblers.listenbrainz.enabled = item.checked;
             setConfig(config);
           },

src/plugins/scrobbler/services/base.ts

@@ -1,6 +1,5 @@
-import { ScrobblerPluginConfig } from '../index';
+import type { ScrobblerPluginConfig } from '../index';
-import { SetConfType } from '../main';
+import type { SetConfType } from '../main';
-
 import type { SongInfo } from '@/providers/song-info';
 
 export abstract class ScrobblerBase {

src/plugins/scrobbler/services/lastfm.ts

@@ -1,12 +1,13 @@
 import crypto from 'node:crypto';
 
-import { net, shell } from 'electron';
+import { BrowserWindow, dialog, net } from 'electron';
 
 import { ScrobblerBase } from './base';
 
-import { ScrobblerPluginConfig } from '../index';
+import { t } from '@/i18n';
-import { SetConfType } from '../main';
 
+import type { ScrobblerPluginConfig } from '../index';
+import type { SetConfType } from '../main';
 import type { SongInfo } from '@/providers/song-info';
 
 interface LastFmData {
@@ -28,11 +29,22 @@ interface LastFmSongData {
 }
 
 export class LastFmScrobbler extends ScrobblerBase {
-  isSessionCreated(config: ScrobblerPluginConfig): boolean {
+  mainWindow: BrowserWindow;
+
+  constructor(mainWindow: BrowserWindow) {
+    super();
+
+    this.mainWindow = mainWindow;
+  }
+
+  override isSessionCreated(config: ScrobblerPluginConfig): boolean {
     return !!config.scrobblers.lastfm.sessionKey;
   }
 
-  async createSession(config: ScrobblerPluginConfig, setConfig: SetConfType): Promise<ScrobblerPluginConfig> {
+  override async createSession(
+    config: ScrobblerPluginConfig,
+    setConfig: SetConfType,
+  ): Promise<ScrobblerPluginConfig> {
     // Get and store the session key
     const data = {
       api_key: config.scrobblers.lastfm.apiKey,
@@ -52,8 +64,15 @@ export class LastFmScrobbler extends ScrobblerBase {
     };
     if (json.error) {
       config.scrobblers.lastfm.token = await createToken(config);
-      await authenticate(config);
+      // If is successful, we need retry the request
-      setConfig(config);
+      authenticate(config, this.mainWindow).then((it) => {
+        if (it) {
+          this.createSession(config, setConfig);
+        } else {
+          // failed
+          setConfig(config);
+        }
+      });
     }
     if (json.session) {
       config.scrobblers.lastfm.sessionKey = json.session.key;
@@ -62,7 +81,7 @@ export class LastFmScrobbler extends ScrobblerBase {
     return config;
   }
 
-  setNowPlaying(songInfo: SongInfo, config: ScrobblerPluginConfig, setConfig: SetConfType): void {
+  override setNowPlaying(songInfo: SongInfo, config: ScrobblerPluginConfig, setConfig: SetConfType): void {
     if (!config.scrobblers.lastfm.sessionKey) {
       return;
     }
@@ -74,7 +93,7 @@ export class LastFmScrobbler extends ScrobblerBase {
     this.postSongDataToAPI(songInfo, config, data, setConfig);
   }
 
-  addScrobble(songInfo: SongInfo, config: ScrobblerPluginConfig, setConfig: SetConfType): void {
+  override addScrobble(songInfo: SongInfo, config: ScrobblerPluginConfig, setConfig: SetConfType): void {
     if (!config.scrobblers.lastfm.sessionKey) {
       return;
     }
@@ -87,7 +106,7 @@ export class LastFmScrobbler extends ScrobblerBase {
     this.postSongDataToAPI(songInfo, config, data, setConfig);
   }
 
-  async postSongDataToAPI(
+  private async postSongDataToAPI(
     songInfo: SongInfo,
     config: ScrobblerPluginConfig,
     data: LastFmData,
@@ -128,8 +147,14 @@ export class LastFmScrobbler extends ScrobblerBase {
             // Session key is invalid, so remove it from the config and reauthenticate
             config.scrobblers.lastfm.sessionKey = undefined;
             config.scrobblers.lastfm.token = await createToken(config);
-            await authenticate(config);
+            authenticate(config, this.mainWindow).then((it) => {
-            setConfig(config);
+              if (it) {
+                this.createSession(config, setConfig);
+              } else {
+                // failed
+                setConfig(config);
+              }
+            });
           } else {
             console.error(error);
           }
@@ -168,17 +193,17 @@ const createQueryString = (
 
 const createApiSig = (parameters: LastFmSongData, secret: string) => {
   // This function creates the api signature, see: https://www.last.fm/api/authspec
-  const keys = Object.keys(parameters);
-
-  keys.sort();
   let sig = '';
-  for (const key of keys) {
-    if (key === 'format') {
-      continue;
-    }
 
-    sig += `${key}${parameters[key as keyof LastFmSongData]}`;
+  Object
-  }
+    .entries(parameters)
+    .sort(([a], [b]) => a.localeCompare(b))
+    .forEach(([key, value]) => {
+      if (key === 'format') {
+        return;
+      }
+      sig += key + value;
+    });
 
   sig += secret;
   sig = crypto.createHash('md5').update(sig, 'utf-8').digest('hex');
@@ -195,7 +220,11 @@ const createToken = async ({
   }
 }: ScrobblerPluginConfig) => {
   // Creates and stores the auth token
-  const data = {
+  const data: {
+    method: string;
+    api_key: string;
+    format: string;
+  } = {
     method: 'auth.gettoken',
     api_key: apiKey,
     format: 'json',
@@ -208,9 +237,68 @@ const createToken = async ({
   return json?.token;
 };
 
-const authenticate = async (config: ScrobblerPluginConfig) => {
+let authWindowOpened = false;
-  // Asks the user for authentication
+let latestAuthResult = false;
-  await shell.openExternal(
+
-    `https://www.last.fm/api/auth/?api_key=${config.scrobblers.lastfm.apiKey}&token=${config.scrobblers.lastfm.token}`,
+const authenticate = async (config: ScrobblerPluginConfig, mainWindow: BrowserWindow) => {
-  );
+  return new Promise<boolean>((resolve) => {
+    if (!authWindowOpened) {
+      authWindowOpened = true;
+      const url = `https://www.last.fm/api/auth/?api_key=${config.scrobblers.lastfm.apiKey}&token=${config.scrobblers.lastfm.token}`;
+      const browserWindow = new BrowserWindow({
+        width: 500,
+        height: 600,
+        show: false,
+        webPreferences: {
+          nodeIntegration: false,
+        },
+        autoHideMenuBar: true,
+        parent: mainWindow,
+        minimizable: false,
+        maximizable: false,
+        paintWhenInitiallyHidden: true,
+        modal: true,
+        center: true,
+      });
+      browserWindow.loadURL(url).then(() => {
+        browserWindow.show();
+        browserWindow.webContents.on('did-navigate', async (_, newUrl) => {
+          const url = new URL(newUrl);
+          if (url.hostname.endsWith('last.fm')) {
+            if (url.pathname === '/api/auth') {
+              const isApproveScreen = await browserWindow.webContents.executeJavaScript(
+                '!!document.getElementsByName(\'confirm\').length'
+              ) as boolean;
+              // successful authentication
+              if (!isApproveScreen) {
+                resolve(true);
+                latestAuthResult = true;
+                browserWindow.close();
+              }
+            } else if (url.pathname === '/api/None') {
+              resolve(false);
+              latestAuthResult = false;
+              browserWindow.close();
+            }
+          }
+        });
+        browserWindow.on('closed', () => {
+          if (!latestAuthResult) {
+            dialog.showMessageBox({
+              title: t('plugins.scrobbler.dialog.lastfm.auth-failed.title'),
+              message: t('plugins.scrobbler.dialog.lastfm.auth-failed.message'),
+              type: 'error'
+            });
+          }
+          authWindowOpened = false;
+        });
+      });
+    } else {
+      // wait for the previous window to close
+      while (authWindowOpened) {
+        // wait
+      }
+      resolve(latestAuthResult);
+    }
+  });
 };

src/plugins/scrobbler/services/listenbrainz.ts

@@ -2,10 +2,8 @@ import { net } from 'electron';
 
 import { ScrobblerBase } from './base';
 
-import { SetConfType } from '../main';
+import type { SetConfType } from '../main';
-
 import type { SongInfo } from '@/providers/song-info';
-
 import type { ScrobblerPluginConfig } from '../index';
 
 interface ListenbrainzRequestBody {
@@ -27,15 +25,15 @@ interface ListenbrainzRequestBody {
 }
 
 export class ListenbrainzScrobbler extends ScrobblerBase {
-  isSessionCreated(): boolean {
+  override isSessionCreated(): boolean {
     return true;
   }
 
-  createSession(config: ScrobblerPluginConfig, _setConfig: SetConfType): Promise<ScrobblerPluginConfig> {
+  override createSession(config: ScrobblerPluginConfig, _setConfig: SetConfType): Promise<ScrobblerPluginConfig> {
     return Promise.resolve(config);
   }
 
-  setNowPlaying(songInfo: SongInfo, config: ScrobblerPluginConfig, _setConfig: SetConfType): void {
+  override setNowPlaying(songInfo: SongInfo, config: ScrobblerPluginConfig, _setConfig: SetConfType): void {
     if (!config.scrobblers.listenbrainz.apiRoot || !config.scrobblers.listenbrainz.token) {
       return;
     }
@@ -44,7 +42,7 @@ export class ListenbrainzScrobbler extends ScrobblerBase {
     submitListen(body, config);
   }
 
-  addScrobble(songInfo: SongInfo, config: ScrobblerPluginConfig, _setConfig: SetConfType): void {
+  override addScrobble(songInfo: SongInfo, config: ScrobblerPluginConfig, _setConfig: SetConfType): void {
     if (!config.scrobblers.listenbrainz.apiRoot || !config.scrobblers.listenbrainz.token) {
       return;
     }