From 809c99d5bba1102e244ab4a876baa6ce23e6a1e3 Mon Sep 17 00:00:00 2001
From: debont80 <dmbont23@mailbox.org>
Date: Wed, 1 Jul 2026 11:17:23 -0400
Subject: [PATCH] fix(renderer): L138/L145 clipboard privacy+dedup, M14
 state-driven settings search
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- L138: gate the clipboard check on the settings `loaded` flag (not just
  clipboardWatch), so it never reads using the pre-load fallback; the effect
  re-runs when loaded flips true so a pre-launch copied link is still offered.
- L145: lastSeen is module-level, so a dismissed clipboard link stays suppressed
  across the hook's remount on a Downloads<->Library tab switch.
- M14: SettingsView renders each card in a React-owned wrapper and drives its
  visibility from a `hidden` state array — the search no longer writes display:none
  onto a card's own DOM node. Matching still reads textContent (full label coverage,
  no per-card keyword upkeep).

typecheck + 268 tests + eslint + prettier green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 CODE-AUDIT.md                                | 20 ++++-
 src/renderer/src/components/SettingsView.tsx | 86 +++++++++++---------
 src/renderer/src/useClipboardLink.ts         | 25 ++++--
 3 files changed, 82 insertions(+), 49 deletions(-)

diff --git a/CODE-AUDIT.md b/CODE-AUDIT.md
index 6257598..67bd0e4 100644
--- a/CODE-AUDIT.md
+++ b/CODE-AUDIT.md
@@ -363,9 +363,14 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
   those to the shared hook incrementally.*
 - [x] **M13 — Inconsistent secret-field masking.** `updateToken` uses `type="password"`; `proxy`
   (may carry `user:pass@`) and `youtubePoToken` (a token) are plain-text Inputs.
-- [ ] **M14 — Settings search mutates React-owned DOM.** The search toggles each card's
+- [x] **M14 — Settings search mutates React-owned DOM.** The search toggles each card's
   `el.style.display` directly; fragile (breaks if a card ever gets a conditional `style`) and
-  matches on `textContent` incl. hidden text. Prefer state-driven filtering.
+  matches on `textContent` incl. hidden text. Prefer state-driven filtering. *Fixed: each card renders
+  inside a React-owned wrapper whose visibility is driven by a `hidden` state array in
+  [SettingsView.tsx](src/renderer/src/components/SettingsView.tsx) — the filter no longer writes
+  `display` onto a card's own node, so a card setting its own inline style can't break it. Matching still
+  reads the wrapper's `textContent` (so search covers every visible label without hand-maintained
+  per-card keywords); the minor "matches hidden descendant text" point is unchanged and left as-is.*
 - [x] **M15 — Nested interactive controls in `role="button"` (a11y).** LibraryView's group header
   is a `role="button"` div containing `<Button>`s (All / Download) — invalid ARIA / keyboard
   semantics. Make the header a real element with sibling buttons.
@@ -789,9 +794,13 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
   instead of an indeterminate state. *Fixed: `parseProgress` now sets `sizeUnknown: !totalBytes` on
   `DownloadProgress`; the store threads it onto the item and `QueueItem` renders an indeterminate bar +
   "Downloading…" (instead of a frozen 0%) when it's set, reusing the existing SR7 indeterminate path.*
-- [ ] **L138 — Clipboard read on every window focus.** [useClipboardLink.ts](src/renderer/src/useClipboardLink.ts)
+- [x] **L138 — Clipboard read on every window focus.** [useClipboardLink.ts](src/renderer/src/useClipboardLink.ts)
   reads the full clipboard on each `focus` (and once on mount, possibly before `clipboardWatch` has
   loaded — FALLBACK is `true`). Reading all clipboard text whenever focused may surprise privacy-conscious users.
+  *Fixed: the check now gates on the settings store's `loaded` flag as well as `clipboardWatch`, so the
+  clipboard is never read using the pre-load fallback (the mount-before-settings-load window); the effect
+  re-runs when `loaded` flips true, so a link copied before launch is still offered once the real setting
+  is known. (Per-focus reads were already gated on `clipboardWatch`; the privacy default itself is SR4.)*
 - [ ] **L139 — Auto-update vs concurrent spawn race.** `runStartupYtdlpAutoUpdate` can overwrite
   `yt-dlp.exe` on launch while a deep-link/queued download spawns the same binary — a Windows
   file-in-use/corrupt-spawn window.
@@ -816,8 +825,11 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
   (mitigated today by the embedded fallback tray icon).
 - [ ] **L144 — Duplicate guard checks the queue only, not history.** Re-adding a URL downloaded earlier
   (already cleared from the queue) gives no "already downloaded" hint (roadmap notes this as a possible extension).
-- [ ] **L145 — `useClipboardLink` re-offers a dismissed link after a tab switch.** `lastSeen` is
+- [x] **L145 — `useClipboardLink` re-offers a dismissed link after a tab switch.** `lastSeen` is
   per-hook-instance; switching Downloads↔Library remounts it, so a previously-dismissed clipboard link is offered again.
+  *Fixed: `lastSeen` is now a module-level variable (not a per-instance `useRef`), so a dismissed/accepted
+  clipboard link stays suppressed across hook remounts. Only one field is mounted at a time (tab switch), so
+  there's no cross-field contention.*
 - [x] **L146 — `parseTrimSections` accepts malformed multi-colon times.** The `\d+(?::\d{1,2})*` pattern
   passes tokens like `1:2:3:4-5:6:7:8`, which then reach yt-dlp's `--download-sections` and fail there
   rather than being rejected up front.
diff --git a/src/renderer/src/components/SettingsView.tsx b/src/renderer/src/components/SettingsView.tsx
index 558ec55..b420930 100644
--- a/src/renderer/src/components/SettingsView.tsx
+++ b/src/renderer/src/components/SettingsView.tsx
@@ -22,41 +22,52 @@ const useStyles = makeStyles({
   }
 })
 
+// The cards, in display order. Rendered inside React-owned wrappers so the search
+// filter controls each card's visibility via state instead of mutating the card's
+// own DOM `style` (M14) — which broke if a card ever set its own inline style.
+const CARDS = [
+  DownloadsCard,
+  AppearanceCard,
+  PostProcessingCard,
+  NetworkCard,
+  CookiesCard,
+  CustomCommandsCard,
+  FilenamesCard,
+  BackupCard,
+  DiagnosticsCard,
+  SoftwareUpdateCard,
+  AboutCard
+]
+
 export function SettingsView(): React.JSX.Element {
   const styles = useStyles()
   const screen = useScreenStyles()
 
-  // Settings search (Phase O). Rather than thread a query through all ~11 cards,
-  // filter by toggling each card's `display` based on whether its text matches.
-  // The cards are the root's direct children (one <Card> DOM node each); the
-  // search box (marked data-settings-search) is skipped. React never sets `style`
-  // on the cards, so our inline display survives their re-renders.
+  // Settings search (Phase O). Matching still reads each card's rendered text (so a
+  // search covers every visible label without hand-maintaining per-card keywords),
+  // but the result drives a `hidden` state array on the wrappers — React owns the
+  // visibility, rather than us writing `display:none` onto React's own card nodes (M14).
   const [search, setSearch] = useState('')
-  const [noResults, setNoResults] = useState(false)
-  const rootRef = useRef<HTMLDivElement>(null)
+  const cardRefs = useRef<(HTMLDivElement | null)[]>([])
+  const [hidden, setHidden] = useState<boolean[]>(() => CARDS.map(() => false))
+
   useEffect(() => {
-    const root = rootRef.current
-    if (!root) return
     const q = search.trim().toLowerCase()
-    let shown = 0
-    for (const el of Array.from(root.children) as HTMLElement[]) {
-      if (el.dataset.settingsSearch !== undefined) continue
-      const match = !q || (el.textContent ?? '').toLowerCase().includes(q)
-      el.style.display = match ? '' : 'none'
-      if (match) shown++
-    }
-    setNoResults(q !== '' && shown === 0)
+    setHidden(
+      CARDS.map((_, i) => {
+        if (!q) return false
+        const text = (cardRefs.current[i]?.textContent ?? '').toLowerCase()
+        return !text.includes(q)
+      })
+    )
   }, [search])
 
+  const noResults = search.trim() !== '' && hidden.length > 0 && hidden.every(Boolean)
+
   return (
-    <div className={mergeClasses(styles.root, screen.width)} ref={rootRef}>
-      <div data-settings-search>
-        <ScreenHeader
-          title="Settings"
-          description="Folders, formats, network, cookies, and more."
-        />
-      </div>
-      <div data-settings-search>
+    <div className={mergeClasses(styles.root, screen.width)}>
+      <ScreenHeader title="Settings" description="Folders, formats, network, cookies, and more." />
+      <div>
         <Input
           input={{ 'aria-label': 'Search settings' }}
           value={search}
@@ -67,22 +78,23 @@ export function SettingsView(): React.JSX.Element {
         />
         {noResults && (
           <Caption1 style={{ color: tokens.colorNeutralForeground3, marginTop: '8px' }}>
-            No settings match "{search}".
+            No settings match &quot;{search}&quot;.
           </Caption1>
         )}
       </div>
 
-      <DownloadsCard />
-      <AppearanceCard />
-      <PostProcessingCard />
-      <NetworkCard />
-      <CookiesCard />
-      <CustomCommandsCard />
-      <FilenamesCard />
-      <BackupCard />
-      <DiagnosticsCard />
-      <SoftwareUpdateCard />
-      <AboutCard />
+      {CARDS.map((Card, i) => (
+        <div
+          // Static array, never reordered/filtered (cards only hide) — index key is stable.
+          key={i}
+          ref={(el) => {
+            cardRefs.current[i] = el
+          }}
+          style={hidden[i] ? { display: 'none' } : undefined}
+        >
+          <Card />
+        </div>
+      ))}
     </div>
   )
 }
diff --git a/src/renderer/src/useClipboardLink.ts b/src/renderer/src/useClipboardLink.ts
index 178bcb7..22c94b5 100644
--- a/src/renderer/src/useClipboardLink.ts
+++ b/src/renderer/src/useClipboardLink.ts
@@ -45,6 +45,11 @@ interface ClipboardLink {
  * clipboard guess; links pushed via `offer()` bypass it (and the other guards),
  * since they're explicit requests rather than guesses.
  */
+// Module-level (not a per-instance ref) so a dismissed/accepted link stays
+// suppressed across hook remounts — switching Downloads↔Library remounts the hook,
+// and the same clipboard link shouldn't be re-offered after being dismissed (L145).
+let lastSeen: string | null = null
+
 export function useClipboardLink(
   currentValue: string,
   filter?: (url: string) => boolean
@@ -57,18 +62,22 @@ export function useClipboardLink(
   // focus listener from re-subscribing every render.
   const filterRef = useRef(filter)
   filterRef.current = filter
-  // The last link we offered or that the user dismissed — so re-focusing doesn't
-  // keep re-offering the same one.
-  const lastSeen = useRef<string | null>(null)
   // Mirror the live suggestion so accept/dismiss can read it without depending on
   // it — keeps their identity stable across renders.
   const suggestionRef = useRef<string | null>(null)
   suggestionRef.current = suggestion
+  // Re-run the check once persisted settings arrive, so a link copied before launch
+  // is still offered (the mount check is a no-op until `loaded`, L138).
+  const loaded = useSettings((s) => s.loaded)
 
   useEffect(() => {
     let active = true
     async function check(): Promise<void> {
-      if (!useSettings.getState().clipboardWatch || valueRef.current.trim()) return
+      const s = useSettings.getState()
+      // Don't touch the clipboard until real settings have loaded (L138): before
+      // that we'd be acting on the pre-load fallback for clipboardWatch instead of
+      // the user's actual choice — and we never read it when the watcher is off.
+      if (!s.loaded || !s.clipboardWatch || valueRef.current.trim()) return
       let text = ''
       try {
         text = (await window.api.readClipboard()) ?? ''
@@ -78,7 +87,7 @@ export function useClipboardLink(
       if (!active) return
       text = text.trim()
       const wanted = looksLikeUrl(text) && (filterRef.current?.(text) ?? true)
-      if (wanted && text !== lastSeen.current) {
+      if (wanted && text !== lastSeen) {
         setSource('clipboard')
         setSuggestion(text)
       }
@@ -89,18 +98,18 @@ export function useClipboardLink(
       active = false
       window.removeEventListener('focus', check)
     }
-  }, [])
+  }, [loaded])
 
   const accept = useCallback((): string | null => {
     const link = suggestionRef.current
     if (!link) return null
-    lastSeen.current = link
+    lastSeen = link
     setSuggestion(null)
     return link
   }, [])
 
   const dismiss = useCallback((): void => {
-    lastSeen.current = suggestionRef.current
+    lastSeen = suggestionRef.current
     setSuggestion(null)
   }, [])