fix(dictation): stop the widget window stranding a rectangle, and stop showing a pill - #1411
Merged
Conversation
…p showing a pill The reported symptom was a dark, empty, square-cornered rectangle that appeared on every dictation trigger and stayed until the app was killed. It measured exactly 300x64 — the widget window's declared size — and the pill it should have held is a capsule, so what was on screen was the window painting its own background with nothing in it. One cause explains all three properties. detectIsWidget() asked getCurrentWindow().label, which throws while Tauri's internals are still being injected; the catch then fell back to a URL query Tauri 2 cannot set, so the window concluded it was "main". From there it rendered <App/> instead of <CaptureWidget/>: no data-window="widget" (opaque chrome background), no pill, and — because the idle-hide reconcile lives inside CaptureWidget — nothing left in the process that could hide it again. The window now stamps window.__OV_WINDOW__ from an initialization_script, which is evaluated before any page script and so cannot race a readiness check. #1398 fixed a different path to the same square and left this one. Separately, the pill is gone (owner decision): the widget window is never shown at all. It has to keep existing — getUserMedia, MediaRecorder and the transcription WebSocket all live in CaptureWidget — but it is now a hidden host, so dictation records, transcribes and pastes with nothing on screen. Two consequences handled: - The tray Start/Stop item inferred recording from widget.is_visible(), which a permanently hidden window makes meaningless — Stop would have been unreachable. It now reads an AppFlags.dictating flag kept current by the frontend's existing set_tray_recording call. - States that need the user to act (Accessibility ungranted, mic denied, a failed transcription) used to surface in the pill and would otherwise have gone silent, making a blocked hotkey indistinguishable from a broken one. They are emitted to the main window and shown as a toast, with the button that opens the relevant OS pane. Regression cover: DictationNoPillWindow.test.jsx pins the identity marker (2 of its 3 cases fail without the fix), and test_dictation_no_pill_window pins the Rust side — the marker is injected, nothing calls show() on the widget, and the tray toggle no longer reads visibility.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe dictation widget remains hidden and identifies itself before page scripts run. Recording state is tracked in ChangesDictation flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 7 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (7 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Line 25: Add the applicable issue or pull request reference in the format
(`#NNN`) to the end of the Unreleased changelog entry, preserving the existing
description.
In `@frontend/src-tauri/src/lib.rs`:
- Around line 551-555: Prevent the single-instance callback in
frontend/src-tauri/src/lib.rs:551-555 from showing or focusing the "widget"
window, preserving it as a hidden recorder host in pill mode. Update
tests/test_dictation_no_pill_window.py:50-67 to exercise the dynamic pill-mode
target and fail if any second-launch path shows the widget.
In `@frontend/src/components/CaptureWidget.jsx`:
- Around line 1322-1331: Update the emitDictationNotice call in the useEffect to
include an explicit action field, setting openMicrophoneSettings only when
errorInfo.kind is 'mic' and errorInfo.deniedByOs is true; otherwise omit or
clear the action so busy or unavailable microphone errors do not offer
permission settings.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ddb0a0c1-e12c-42c4-a879-6988bcabf81f
📒 Files selected for processing (9)
CHANGELOG.mdfrontend/src-tauri/src/commands.rsfrontend/src-tauri/src/lib.rsfrontend/src/App.jsxfrontend/src/components/CaptureWidget.jsxfrontend/src/main-app.jsxfrontend/src/test/DictationNoPillWindow.test.jsxfrontend/src/utils/dictationNotice.jsxtests/test_dictation_no_pill_window.py
| - The repository moved to github.com/debpalash/VoiceStudio. Every link in the app, docs and scripts now points there; GitHub redirects the old URLs, and the Docker image paths, the app bundle identifier and your data folder are all deliberately unchanged. (#1394) | ||
| - The app is now **VoiceStudio** (previously OmniVoice-Studio). Only the name you see changes — your data folder, settings and the Docker image paths stay put, so upgrading needs nothing from you. On Linux the .deb is now `voicestudio`; remove the old `omnivoice-studio` package once. | ||
| - macOS floor raised to 13.3 (Ventura) — the frontend has required Safari 16.4 for some time, so macOS 12 was a promise the stack could not keep (#1268) | ||
| - Dictation no longer shows a floating pill. The hotkey records, transcribes and pastes with nothing on screen; the tray icon still marks recording, and anything needing your attention (Accessibility, microphone, a failed transcription) now arrives as a notification in the main window. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the release reference.
Line 25 does not end with an issue or PR reference, so this Unreleased entry fails the changelog format.
Add the applicable (#NNN) reference.
As per coding guidelines and path instructions, Unreleased entries must end in (#N).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CHANGELOG.md` at line 25, Add the applicable issue or pull request reference
in the format (`#NNN`) to the end of the Unreleased changelog entry, preserving
the existing description.
Sources: Coding guidelines, Path instructions
Comment on lines
+1322
to
+1331
| useEffect(() => { | ||
| if (state !== 'error' && state !== 'setup') return; | ||
| const kind = state === 'setup' ? 'setup' : errorInfo?.kind || 'transcription'; | ||
| emitDictationNotice({ | ||
| kind, | ||
| // Localize here: this is where the error's context lives, and both | ||
| // windows share one i18n instance and language. | ||
| label: state === 'setup' ? t('capture.a11y_setup') : errorLabel(t, errorInfo), | ||
| }); | ||
| }, [state, errorInfo, t]); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve the microphone remediation condition.
Line 1324 omits errorInfo.deniedByOs, but showDictationNotice opens microphone settings for every kind: 'mic' notice.
A busy or unavailable microphone then gets a permission-settings action.
Send an explicit action field and only expose openMicrophoneSettings when the mic error is OS-denied.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/CaptureWidget.jsx` around lines 1322 - 1331, Update
the emitDictationNotice call in the useEffect to include an explicit action
field, setting openMicrophoneSettings only when errorInfo.kind is 'mic' and
errorInfo.deniedByOs is true; otherwise omit or clear the action so busy or
unavailable microphone errors do not offer permission settings.
…idget
Two CodeRabbit findings, both real.
The single-instance handler picked its focus target as
`if pill_mode { "widget" } else { "main" }` and called show() on it. In
pill mode that put the recorder window on screen — the exact empty
rectangle this PR exists to remove, reached by relaunching the app,
which is precisely what a user does when one is stuck on their desktop.
It now always targets the studio window, matching what the tray's "Open
VoiceStudio" item already does.
The guard test missed it because it only looked for the label written
out literally, and this call site computed it into a variable. Added a
second test that pins every mention of the "widget" label against a
reviewed allowlist, so indirection can't slip past again. Verified it
fails when the old ternary is put back.
Also: the notice toast offered "Open Settings" for every mic error, but
only an OS-level denial has a pane worth opening — a busy or absent mic
arrives under the same kind and would have sent the user somewhere
nothing is wrong. The pill's own button carried that condition; the
notice now carries it too.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_dictation_no_pill_window.py`:
- Line 104: The allowlist in the test can accept unsafe calls when they appear
on the same line as an allowed window lookup. In the line-filtering logic,
reject lines containing `.show(` or `show_pill_noactivate(` before evaluating
`allowed_exact` or `allowed`; add a regression test that fails before and passes
after this change.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d5c6664b-0160-4e7f-b1b2-2865be9e2923
📒 Files selected for processing (4)
frontend/src-tauri/src/lib.rsfrontend/src/components/CaptureWidget.jsxfrontend/src/utils/dictationNotice.jsxtests/test_dictation_no_pill_window.py
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/components/CaptureWidget.jsx
- frontend/src/utils/dictationNotice.jsx
- frontend/src-tauri/src/lib.rs
| stripped = line.strip() | ||
| if stripped.startswith("//") or stripped.startswith("///"): | ||
| continue # prose about the widget is not a call site | ||
| if stripped in allowed_exact or any(ok in line for ok in allowed): |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject unsafe calls before allowlisting the line.
Line 104 skips a line that contains get_webview_window("widget"), so if let Some(win) = ... { win.show(); } would pass this test. Check for .show( and show_pill_noactivate( before the allowlist, or make permitted lookup patterns exact. As per coding guidelines, “Fix the root cause with a fail-before/pass-after regression test and the smallest correct change.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_dictation_no_pill_window.py` at line 104, The allowlist in the
test can accept unsafe calls when they appear on the same line as an allowed
window lookup. In the line-filtering logic, reject lines containing `.show(` or
`show_pill_noactivate(` before evaluating `allowed_exact` or `allowed`; add a
regression test that fails before and passes after this change.
Source: Coding guidelines
…-window # Conflicts: # CHANGELOG.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The rectangle
Reported live: a dark, empty, square-cornered rectangle appearing on every dictation trigger and surviving until the app was killed.
It measured exactly 300×64 — the
widgetwindow's declared size — and the pill it should have contained is a capsule (border-radius: 100px). So what was on screen was the window painting its own background with no pill inside it.One cause explains all three properties:
getCurrentWindow()throws while__TAURI_INTERNALS__is still being injected. The catch then guesses — and its only guess is a URL query Tauri 2 has no way to produce. A widget window that guesses "main" renders<App/>:data-window="widget"never set → body keeps the chrome backgroundCaptureWidget, which never mountedThe window now stamps
window.__OV_WINDOW__from aninitialization_script, evaluated before any page script — there is nothing left for it to race.#1398 fixed a different route to the same square (a dropped press during re-arm) and left this one open.
The pill
Owner decision: dictation shows nothing. The widget window is never shown.
It still has to exist —
getUserMedia,MediaRecorderand the transcription WebSocket all live inCaptureWidget.jsx, so deleting the window deletes dictation. It is now a hidden host.Two things that would have broken silently, handled:
widget.is_visible(). A permanently hidden window is never visible, so Stop would have been unreachable. It now reads anAppFlags.dictatingflag kept current by the frontend's existingset_tray_recordingcall.Verification
DictationNoPillWindow.test.jsx— 2 of 3 cases fail without the fix (verified by reverting it)tests/test_dictation_no_pill_window.py— marker is injected, nothing callsshow()on the widget, tray toggle no longer reads visibilitycargo test109 passed · frontend 1711 passed (214 files) · typecheck clean · no new lint errorsformat:checkreports 3 files —Header.jsx,AppearancePanel.jsx,AppearancePanel.test.jsx— none of which this PR touches; they belong to unrelated in-progress work in the same working tree and are deliberately not included here.Not verified
The hidden-window recording path has not been exercised live — whether macOS keeps media capture alive in a never-shown
WKWebViewis the one claim here that testing can't settle. The webview demonstrably stays alive while hidden today (it services the hotkey listener), but capture previously always started after the window was shown. If capture turns out to be throttled, the fallback is a shown-but-1×1 window.The dictation widget now sets
window.__OV_WINDOW__before page scripts run, remains hidden, and usesAppFlags.dictatingfor tray state. Dictation errors now reach the main window as actionable accessibility, microphone, and transcription toasts. Please review the Tauri event bridge and hidden-window lifecycle for platform-specific regressions.