From c594545658f18cde36d6464c932e4920fc685ef2 Mon Sep 17 00:00:00 2001 From: ChrisScott9456 Date: Sat, 11 Jul 2026 17:28:49 -0400 Subject: [PATCH] Improve tag editor add-field UX and fix custom tag writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix custom tag keys being silently dropped by TagLib — unknown keys are now uppercased before write since TagLib requires all-caps Vorbis field names - Add real-time validation for invalid characters (= and non-ASCII) in custom tag keys - Show 'Field already exists' error on Enter/+ attempt rather than silently doing nothing - Resolve typed tag labels to canonical keys (e.g. 'Album Sort' → albumSort) to prevent duplicate entries under a different key - Atomic duplicate check in hook functional updater prevents race conditions from rapid adds - Scroll to and focus newly added field after it is inserted - Consolidate FIELD_LABEL_OVERRIDES, FIELD_TYPE_OVERRIDES, and PICARD_EXTRAS into a single TAG_CONFIG object; extract resolveTagKey helper --- .../core/tag-editor/taglib-service.ts | 162 +++++++----------- .../tag-editor/components/song-edit-modal.tsx | 99 +++++++++-- .../tag-editor/hooks/use-metadata-editor.ts | 19 +- .../features/tag-editor/utils/known-tags.ts | 119 +++++++------ .../features/tag-editor/utils/utils.ts | 12 -- src/shared/types/tag-editor.ts | 52 ------ 6 files changed, 226 insertions(+), 237 deletions(-) diff --git a/src/main/features/core/tag-editor/taglib-service.ts b/src/main/features/core/tag-editor/taglib-service.ts index 10b1781de..70565b699 100644 --- a/src/main/features/core/tag-editor/taglib-service.ts +++ b/src/main/features/core/tag-editor/taglib-service.ts @@ -1,11 +1,8 @@ import type { ArtworkKind, ArtworkOp, BatchFileError } from '/@/shared/types/tag-editor'; -import type { ICommonTagsResult } from 'music-metadata'; import { constants, promises as fsPromises } from 'fs'; -import * as mm from 'music-metadata'; -import { TagLib } from 'taglib-wasm'; +import { PROPERTIES, TagLib } from 'taglib-wasm'; -import { EDITOR_FIELD_KEYS } from '/@/shared/types/tag-editor'; import { getImageMimeTypeFromPath } from '/@/shared/utils/image-mime'; let _taglib: null | TagLib = null; @@ -26,44 +23,6 @@ export async function readLocalImageFile(filePath: string) { }; } -const pickFrontCover = (pictures: T[]): T | undefined => - pictures.find((p) => p.type === 'Front Cover') ?? pictures[0]; - -type TagAccessor = (c: ICommonTagsResult) => null | number | string | undefined; - -// Only fields where the editor key differs from the music-metadata key. -const MM_RENAMES: Partial> = { - acoustidId: 'acoustid_id', - albumArtist: 'albumartist', - albumArtistSort: 'albumartistsort', - albumSort: 'albumsort', - artistSort: 'artistsort', - catalogNumber: 'catalognumber', - composerSort: 'composersort', - musicbrainzArtistId: 'musicbrainz_artistid', - musicbrainzReleaseArtistId: 'musicbrainz_albumartistid', - musicbrainzReleaseGroupId: 'musicbrainz_releasegroupid', - musicbrainzReleaseId: 'musicbrainz_albumid', - musicbrainzReleaseTrackId: 'musicbrainz_trackid', - musicbrainzTrackId: 'musicbrainz_recordingid', - musicbrainzWorkId: 'musicbrainz_workid', - originalAlbum: 'originalalbum', - originalArtist: 'originalartist', - originalDate: 'originaldate', - remixedBy: 'remixer', - titleSort: 'titlesort', -}; - -// Only fields that need sub-property access or live on a nested object. -const MM_CUSTOM: Partial> = { - comment: (c) => c.comment?.[0]?.text, - discNumber: (c) => c.disk.no, - lyrics: (c) => c.lyrics?.[0]?.text, - totalDiscs: (c) => c.disk.of, - totalTracks: (c) => c.track.of, - trackNumber: (c) => c.track.no, -}; - /** Returns an error entry for each path that is missing or not writable by the current process. */ export async function checkPathsWritable(paths: string[]): Promise { const failed: BatchFileError[] = []; @@ -82,16 +41,23 @@ export async function checkPathsWritable(paths: string[]): Promise { +/** Flattens a taglib-wasm PropertyMap to a string record, dropping ALL_CAPS alias keys already covered by a camelCase equivalent. */ +function flattenProperties(props: Record): Record { const flat: Record = {}; - for (const key of EDITOR_FIELD_KEYS) { - const custom = MM_CUSTOM[key]; - const raw: unknown = custom - ? custom(common) - : common[(MM_RENAMES[key] ?? key) as keyof ICommonTagsResult]; - const value = Array.isArray(raw) ? raw[0] : raw; - if (value != null && value !== '') flat[key] = String(value); + for (const [key, values] of Object.entries(props)) { + if (values && values.length > 0 && values[0] !== '') { + flat[key] = values[0]; + } + } + const coveredByUpperCase = new Set( + Object.keys(flat) + .filter((k) => k !== k.toUpperCase()) + .map((k) => k.toUpperCase()), + ); + for (const key of Object.keys(flat)) { + if (key === key.toUpperCase() && coveredByUpperCase.has(key)) { + delete flat[key]; + } } return flat; } @@ -116,9 +82,10 @@ const mapWithConcurrency = async ( }; /** - * Reads tags and artwork from a batch of audio files via music-metadata (which preserves full ISO dates). + * Reads all tags and artwork from a batch of audio files via taglib-wasm. + * Returns every tag present on disk — not limited to any known-field list. * Values that differ across files are merged to `null` and shown as "(Multiple Values)" in the editor. - * Artwork bytes are only loaded for one representative file, after confirming all files share the same cover. + * Artwork bytes are captured from the first successful file; subsequent files only contribute their size for comparison. */ export async function readFilesMetadataBatch( filePaths: string[], @@ -136,57 +103,59 @@ export async function readFilesMetadataBatch( }> { const totalCount = filePaths.length; const failedFiles: BatchFileError[] = []; - - // Merged incrementally; safe because JS is single-threaded between awaits. const tagSummary: Record = {}; let artworkKind = 'none' as ArtworkKind; let artworkByteSize: number | undefined; - let firstSuccessPath: string | undefined; + let artworkData: string | undefined; + let artworkMimeType: string | undefined; let readCount = 0; let processed = 0; + const taglib = await getTagLib(); + await mapWithConcurrency( filePaths, BATCH_CONCURRENCY, async (filePath) => { try { - const { common } = await mm.parseFile(filePath); - const flat = flattenMusicMetadata(common); - const hasCoverArt = (common.picture?.length ?? 0) > 0; - // Only access artwork bytes if we still need them for comparison. - const picSize = - artworkKind !== 'mixed' && hasCoverArt - ? pickFrontCover(common.picture!)?.data.length - : undefined; - // common.picture's Uint8Array is not stored anywhere — eligible for GC here. + const file = await taglib.open(filePath); + try { + const flat = flattenProperties(file.properties()); + const pictures = file.getPictures(); + const frontCover = pictures.find((p) => p.type === 'FrontCover') ?? pictures[0]; + const hasCoverArt = frontCover !== undefined; + const picSize = hasCoverArt ? frontCover.data.length : undefined; - if (readCount === 0) { - // First success: seed the summary. - Object.assign(tagSummary, flat); - firstSuccessPath = filePath; - artworkKind = hasCoverArt ? 'common' : 'none'; - artworkByteSize = picSize; - } else { - // Merge tags: keys already null stay null; new divergences become null. - for (const k of Object.keys(tagSummary)) { - if (tagSummary[k] !== null && flat[k] !== tagSummary[k]) - tagSummary[k] = null; - } - for (const k of Object.keys(flat)) { - if (!(k in tagSummary)) tagSummary[k] = null; - } - // Merge artwork. - if (artworkKind !== 'mixed') { - if ( - hasCoverArt !== (artworkKind === 'common') || - picSize !== artworkByteSize - ) { - artworkKind = 'mixed'; + if (readCount === 0) { + Object.assign(tagSummary, flat); + artworkKind = hasCoverArt ? 'common' : 'none'; + artworkByteSize = picSize; + if (frontCover) { + artworkData = Buffer.from(frontCover.data).toString('base64'); + artworkMimeType = frontCover.mimeType; + } + } else { + for (const k of Object.keys(tagSummary)) { + if (tagSummary[k] !== null && flat[k] !== tagSummary[k]) + tagSummary[k] = null; + } + for (const k of Object.keys(flat)) { + if (!(k in tagSummary)) tagSummary[k] = null; + } + if (artworkKind !== 'mixed') { + if ( + hasCoverArt !== (artworkKind === 'common') || + picSize !== artworkByteSize + ) { + artworkKind = 'mixed'; + } } } - } - readCount += 1; + readCount += 1; + } finally { + file.dispose(); + } } catch (err) { failedFiles.push({ error: err instanceof Error ? err.message : String(err), @@ -199,19 +168,6 @@ export async function readFilesMetadataBatch( signal, ); - let artworkData: string | undefined; - let artworkMimeType: string | undefined; - - // Re-read artwork from one file only after confirming all files share the same cover. - if (artworkKind === 'common' && firstSuccessPath) { - const { common } = await mm.parseFile(firstSuccessPath); - const pic = pickFrontCover(common.picture ?? []); - if (pic) { - artworkData = Buffer.from(pic.data).toString('base64'); - artworkMimeType = pic.format; - } - } - return { artworkKind, failedFiles, @@ -264,7 +220,7 @@ export async function writeFilesTags( try { await taglib.edit(path, (file) => { for (const [k, v] of Object.entries(mergedEdits)) { - file.setProperty(k.toUpperCase(), v); + file.setProperty(k in PROPERTIES ? k : k.toUpperCase(), v); } if (artworkOp?.type === 'clear') { file.removePictures(); diff --git a/src/renderer/features/tag-editor/components/song-edit-modal.tsx b/src/renderer/features/tag-editor/components/song-edit-modal.tsx index b41381c66..309a808ce 100644 --- a/src/renderer/features/tag-editor/components/song-edit-modal.tsx +++ b/src/renderer/features/tag-editor/components/song-edit-modal.tsx @@ -1,16 +1,19 @@ +import { Autocomplete } from '@mantine/core'; import { closeAllModals } from '@mantine/modals'; -import { useRef } from 'react'; +import { useRef, useState } from 'react'; +import { RiAddLine } from 'react-icons/ri'; import { useTranslation } from 'react-i18next'; import { useMetadataEditor } from '../hooks/use-metadata-editor'; +import { KNOWN_TAG_MAP, resolveTagKey } from '../utils/known-tags'; import { ArtworkPanel } from './artwork-panel'; import styles from './song-edit-modal.module.css'; import { TagFieldRow } from './tag-field-row'; +import { ActionIcon } from '/@/shared/components/action-icon/action-icon'; import { Button } from '/@/shared/components/button/button'; import { Checkbox } from '/@/shared/components/checkbox/checkbox'; import { Group } from '/@/shared/components/group/group'; -import { Select } from '/@/shared/components/select/select'; import { Spinner } from '/@/shared/components/spinner/spinner'; import { Stack } from '/@/shared/components/stack/stack'; import { Table } from '/@/shared/components/table/table'; @@ -21,27 +24,55 @@ import { Song } from '/@/shared/types/domain-types'; export const SongEditModal = ({ songs }: { songs: Song[] }) => { const { t } = useTranslation(); const tableContainerRef = useRef(null); + const skipNextEnterRef = useRef(false); + const skipNextOnChangeResetRef = useRef(false); + const [addFieldInput, setAddFieldInput] = useState(''); + const [duplicateAttempted, setDuplicateAttempted] = useState(false); - // Opens the metadata editor modal that displays a table of editable fields for the songs, as well as an artwork panel for changing the songs' artwork const editor = useMetadataEditor({ browser: window.api.browser, songs, utils: window.api.utils, }); - // After adding the field, scrolls to and focuses the new field's row in the table. - const handleAddField = (key: null | string) => { - editor.handleAddField(key); - if (!key) return; + const trimmedInput = addFieldInput.trim(); + const customKeyError = + trimmedInput && !KNOWN_TAG_MAP.has(trimmedInput) + ? trimmedInput.includes('=') + ? "Tag key cannot contain '='" + : /[^\x00-\x7F]/.test(trimmedInput) + ? 'Tag key must use ASCII characters only' + : null + : null; + + const resolvedInputKey = trimmedInput ? resolveTagKey(trimmedInput) : ''; + const duplicateError = + trimmedInput && !customKeyError && editor.sortedFieldEntries.some(([k]) => k === resolvedInputKey) + ? 'Field already exists' + : null; + const fieldError = customKeyError ?? (duplicateAttempted ? duplicateError : null); + + const handleAddField = (key: string): boolean => { + const trimmed = key.trim(); + if (!trimmed) return false; + if (!KNOWN_TAG_MAP.has(trimmed) && (trimmed.includes('=') || /[^\x00-\x7F]/.test(trimmed))) return false; + const normalizedKey = resolveTagKey(trimmed); + if (editor.sortedFieldEntries.some(([k]) => k === normalizedKey)) { + setDuplicateAttempted(true); + return false; + } + setDuplicateAttempted(false); + editor.handleAddField(normalizedKey); + setAddFieldInput(''); - // Scroll to focus on the newly added field requestAnimationFrame(() => { const row = tableContainerRef.current?.querySelector( - `[data-field-key="${key}"]`, + `[data-field-key="${normalizedKey}"]`, ); row?.scrollIntoView({ block: 'nearest' }); row?.querySelector('input, textarea')?.focus(); }); + return true; }; // While loading, shows a spinner @@ -86,15 +117,47 @@ export const SongEditModal = ({ songs }: { songs: Song[] }) => { {editor.readWarning} )} - {editor.availableToAdd.length > 0 && ( -