fix(renderer): L138/L145 clipboard privacy+dedup, M14 state-driven settings search
- 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>
This commit is contained in:
+16
-4
@@ -363,9 +363,14 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
|
|||||||
those to the shared hook incrementally.*
|
those to the shared hook incrementally.*
|
||||||
- [x] **M13 — Inconsistent secret-field masking.** `updateToken` uses `type="password"`; `proxy`
|
- [x] **M13 — Inconsistent secret-field masking.** `updateToken` uses `type="password"`; `proxy`
|
||||||
(may carry `user:pass@`) and `youtubePoToken` (a token) are plain-text Inputs.
|
(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
|
`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
|
- [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
|
is a `role="button"` div containing `<Button>`s (All / Download) — invalid ARIA / keyboard
|
||||||
semantics. Make the header a real element with sibling buttons.
|
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
|
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 +
|
`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.*
|
"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
|
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.
|
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
|
- [ ] **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
|
`yt-dlp.exe` on launch while a deep-link/queued download spawns the same binary — a Windows
|
||||||
file-in-use/corrupt-spawn window.
|
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).
|
(mitigated today by the embedded fallback tray icon).
|
||||||
- [ ] **L144 — Duplicate guard checks the queue only, not history.** Re-adding a URL downloaded earlier
|
- [ ] **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).
|
(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.
|
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
|
- [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
|
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.
|
rather than being rejected up front.
|
||||||
|
|||||||
@@ -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 {
|
export function SettingsView(): React.JSX.Element {
|
||||||
const styles = useStyles()
|
const styles = useStyles()
|
||||||
const screen = useScreenStyles()
|
const screen = useScreenStyles()
|
||||||
|
|
||||||
// Settings search (Phase O). Rather than thread a query through all ~11 cards,
|
// Settings search (Phase O). Matching still reads each card's rendered text (so a
|
||||||
// filter by toggling each card's `display` based on whether its text matches.
|
// search covers every visible label without hand-maintaining per-card keywords),
|
||||||
// The cards are the root's direct children (one <Card> DOM node each); the
|
// but the result drives a `hidden` state array on the wrappers — React owns the
|
||||||
// search box (marked data-settings-search) is skipped. React never sets `style`
|
// visibility, rather than us writing `display:none` onto React's own card nodes (M14).
|
||||||
// on the cards, so our inline display survives their re-renders.
|
|
||||||
const [search, setSearch] = useState('')
|
const [search, setSearch] = useState('')
|
||||||
const [noResults, setNoResults] = useState(false)
|
const cardRefs = useRef<(HTMLDivElement | null)[]>([])
|
||||||
const rootRef = useRef<HTMLDivElement>(null)
|
const [hidden, setHidden] = useState<boolean[]>(() => CARDS.map(() => false))
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
const root = rootRef.current
|
|
||||||
if (!root) return
|
|
||||||
const q = search.trim().toLowerCase()
|
const q = search.trim().toLowerCase()
|
||||||
let shown = 0
|
setHidden(
|
||||||
for (const el of Array.from(root.children) as HTMLElement[]) {
|
CARDS.map((_, i) => {
|
||||||
if (el.dataset.settingsSearch !== undefined) continue
|
if (!q) return false
|
||||||
const match = !q || (el.textContent ?? '').toLowerCase().includes(q)
|
const text = (cardRefs.current[i]?.textContent ?? '').toLowerCase()
|
||||||
el.style.display = match ? '' : 'none'
|
return !text.includes(q)
|
||||||
if (match) shown++
|
})
|
||||||
}
|
)
|
||||||
setNoResults(q !== '' && shown === 0)
|
|
||||||
}, [search])
|
}, [search])
|
||||||
|
|
||||||
|
const noResults = search.trim() !== '' && hidden.length > 0 && hidden.every(Boolean)
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className={mergeClasses(styles.root, screen.width)} ref={rootRef}>
|
<div className={mergeClasses(styles.root, screen.width)}>
|
||||||
<div data-settings-search>
|
<ScreenHeader title="Settings" description="Folders, formats, network, cookies, and more." />
|
||||||
<ScreenHeader
|
<div>
|
||||||
title="Settings"
|
|
||||||
description="Folders, formats, network, cookies, and more."
|
|
||||||
/>
|
|
||||||
</div>
|
|
||||||
<div data-settings-search>
|
|
||||||
<Input
|
<Input
|
||||||
input={{ 'aria-label': 'Search settings' }}
|
input={{ 'aria-label': 'Search settings' }}
|
||||||
value={search}
|
value={search}
|
||||||
@@ -67,22 +78,23 @@ export function SettingsView(): React.JSX.Element {
|
|||||||
/>
|
/>
|
||||||
{noResults && (
|
{noResults && (
|
||||||
<Caption1 style={{ color: tokens.colorNeutralForeground3, marginTop: '8px' }}>
|
<Caption1 style={{ color: tokens.colorNeutralForeground3, marginTop: '8px' }}>
|
||||||
No settings match "{search}".
|
No settings match "{search}".
|
||||||
</Caption1>
|
</Caption1>
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<DownloadsCard />
|
{CARDS.map((Card, i) => (
|
||||||
<AppearanceCard />
|
<div
|
||||||
<PostProcessingCard />
|
// Static array, never reordered/filtered (cards only hide) — index key is stable.
|
||||||
<NetworkCard />
|
key={i}
|
||||||
<CookiesCard />
|
ref={(el) => {
|
||||||
<CustomCommandsCard />
|
cardRefs.current[i] = el
|
||||||
<FilenamesCard />
|
}}
|
||||||
<BackupCard />
|
style={hidden[i] ? { display: 'none' } : undefined}
|
||||||
<DiagnosticsCard />
|
>
|
||||||
<SoftwareUpdateCard />
|
<Card />
|
||||||
<AboutCard />
|
</div>
|
||||||
|
))}
|
||||||
</div>
|
</div>
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -45,6 +45,11 @@ interface ClipboardLink {
|
|||||||
* clipboard guess; links pushed via `offer()` bypass it (and the other guards),
|
* clipboard guess; links pushed via `offer()` bypass it (and the other guards),
|
||||||
* since they're explicit requests rather than guesses.
|
* 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(
|
export function useClipboardLink(
|
||||||
currentValue: string,
|
currentValue: string,
|
||||||
filter?: (url: string) => boolean
|
filter?: (url: string) => boolean
|
||||||
@@ -57,18 +62,22 @@ export function useClipboardLink(
|
|||||||
// focus listener from re-subscribing every render.
|
// focus listener from re-subscribing every render.
|
||||||
const filterRef = useRef(filter)
|
const filterRef = useRef(filter)
|
||||||
filterRef.current = 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
|
// Mirror the live suggestion so accept/dismiss can read it without depending on
|
||||||
// it — keeps their identity stable across renders.
|
// it — keeps their identity stable across renders.
|
||||||
const suggestionRef = useRef<string | null>(null)
|
const suggestionRef = useRef<string | null>(null)
|
||||||
suggestionRef.current = suggestion
|
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(() => {
|
useEffect(() => {
|
||||||
let active = true
|
let active = true
|
||||||
async function check(): Promise<void> {
|
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 = ''
|
let text = ''
|
||||||
try {
|
try {
|
||||||
text = (await window.api.readClipboard()) ?? ''
|
text = (await window.api.readClipboard()) ?? ''
|
||||||
@@ -78,7 +87,7 @@ export function useClipboardLink(
|
|||||||
if (!active) return
|
if (!active) return
|
||||||
text = text.trim()
|
text = text.trim()
|
||||||
const wanted = looksLikeUrl(text) && (filterRef.current?.(text) ?? true)
|
const wanted = looksLikeUrl(text) && (filterRef.current?.(text) ?? true)
|
||||||
if (wanted && text !== lastSeen.current) {
|
if (wanted && text !== lastSeen) {
|
||||||
setSource('clipboard')
|
setSource('clipboard')
|
||||||
setSuggestion(text)
|
setSuggestion(text)
|
||||||
}
|
}
|
||||||
@@ -89,18 +98,18 @@ export function useClipboardLink(
|
|||||||
active = false
|
active = false
|
||||||
window.removeEventListener('focus', check)
|
window.removeEventListener('focus', check)
|
||||||
}
|
}
|
||||||
}, [])
|
}, [loaded])
|
||||||
|
|
||||||
const accept = useCallback((): string | null => {
|
const accept = useCallback((): string | null => {
|
||||||
const link = suggestionRef.current
|
const link = suggestionRef.current
|
||||||
if (!link) return null
|
if (!link) return null
|
||||||
lastSeen.current = link
|
lastSeen = link
|
||||||
setSuggestion(null)
|
setSuggestion(null)
|
||||||
return link
|
return link
|
||||||
}, [])
|
}, [])
|
||||||
|
|
||||||
const dismiss = useCallback((): void => {
|
const dismiss = useCallback((): void => {
|
||||||
lastSeen.current = suggestionRef.current
|
lastSeen = suggestionRef.current
|
||||||
setSuggestion(null)
|
setSuggestion(null)
|
||||||
}, [])
|
}, [])
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user