Conversation
github-actions
Bot
added
needs PR template
|
The PR Policy check is blocking this PR because required template information is missing. Please update the PR description with:
Screenshots or video are required for UI, UX, settings, onboarding, overlay, menu bar, or visual behavior changes. If this PR has no visual changes, check the no-visual-change box in the template. If this remains incomplete for 48 hours after opening, the PR may be closed. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d26f9ae63
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| guard Self.webKitBrowserBundleIdentifiers.contains(bundleIdentifier), | ||
| let title = focusedWindowTitle(), | ||
| let editor = Self.keypressDrivenEditorTitles.first(where: { title.hasSuffix($0) }) |
There was a problem hiding this comment.
When the Google account/UI language localizes the product name (for example, Safari exposes a title ending in the localized equivalent of Google Docs), this exact English suffix check returns nil, so the affected editor stays on the direct-typing path and continues dropping all but the first character. The routing signal needs to be independent of the localized window title, or explicitly support localized variants.
Useful? React with 👍 / 👎.
| self.bench("paste_target_prepared elapsedMs=\(Self.elapsedMs(since: targetStartedAt))") | ||
|
|
||
| return self.withTemporaryPasteboardString(text, restoreDelayMicros: 5_000_000) { | ||
| return self.withTemporaryPasteboardString(text, restoreDelayMicros: 1_500_000) { |
There was a problem hiding this comment.
When the target app's event loop takes more than 1.5 seconds to handle the posted Cmd+V and the focused-text snapshot cannot verify completion—as is common for web editors—postToPid has already returned successfully but cleanup restores the previous clipboard before the app reads it. This can paste the user's old clipboard contents instead of the transcript; the previous five-second grace period should not be shortened for unverifiable asynchronous paste delivery.
Useful? React with 👍 / 👎.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| self.bench("paste_target_prepared elapsedMs=\(Self.elapsedMs(since: targetStartedAt))") | ||
|
|
||
| return self.withTemporaryPasteboardString(text, restoreDelayMicros: 5_000_000) { | ||
| return self.withTemporaryPasteboardString(text, restoreDelayMicros: 1_500_000) { |
There was a problem hiding this comment.
This Safari-specific fix also reduces the clipboard restoration window from five seconds to 1.5 seconds for every reliable, fallback, and AppleScript paste target, including the same changes at lines 1087 and 1127. If a slow destination has not consumed the transient paste when verification times out, the previous clipboard contents are restored and no dictated text may be inserted even though dispatch was reported as successful. This violates the repository requirement that Swift changes stay within the stated scope and avoid risking existing features, so the requirement must be satisfied before merging. Retain the established timeout or validate and test the shorter window independently.
Rule Used: What: Ensure macOS Swift PR changes match the stated scope, don’t introduce unrelated UI/UX/theming work, and don’t risk breaking existing features. Why: Keeps reviews focused, prevents scope creep (especially UI/UX), and avoids regressions or inc... (source)
Knowledge Base Used: Dictation processing and typing
Prompt To Fix With AI
This is a comment left during a code review.
Path: Sources/Fluid/Services/TypingService.swift
Line: 962
Comment:
**Clipboard Window Shortened Globally**
This Safari-specific fix also reduces the clipboard restoration window from five seconds to 1.5 seconds for every reliable, fallback, and AppleScript paste target, including the same changes at lines 1087 and 1127. If a slow destination has not consumed the transient paste when verification times out, the previous clipboard contents are restored and no dictated text may be inserted even though dispatch was reported as successful. This violates the repository requirement that Swift changes stay within the stated scope and avoid risking existing features, so the requirement must be satisfied before merging. Retain the established timeout or validate and test the shorter window independently.
**Rule Used:** What: Ensure macOS Swift PR changes match the stated scope, don’t introduce unrelated UI/UX/theming work, and don’t risk breaking existing features. Why: Keeps reviews focused, prevents scope creep (especially UI/UX), and avoids regressions or inc... ([source](https://app.greptile.com/altic/-/custom-context?memory=c54a31bd-761f-45ed-8fcb-a3cb1158d02e))
**Knowledge Base Used:** [Dictation processing and typing](https://app.greptile.com/altic/-/custom-context/knowledge-base/altic-dev/fluidvoice/-/docs/dictation-processing-and-typing.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| guard Self.webKitBrowserBundleIdentifiers.contains(bundleIdentifier), | ||
| let title = focusedWindowTitle(), | ||
| let editor = Self.keypressDrivenEditorTitles.first(where: { title.hasSuffix($0) }) |
There was a problem hiding this comment.
The routing predicate treats Safari’s page-controlled window title as proof that the destination is Google Docs or Slides. Any ordinary page whose title ends with one of those strings can therefore move otherwise directly typed dictation through NSPasteboard.general, where observers that do not honor the advisory transient markers can read or retain it. Verify the Google origin or another non-spoofable document identity before selecting the clipboard path.
How this was verified: Safari’s page-derived accessibility title is matched by suffix alone, after which the dictated string is written to the general pasteboard with only advisory markers.
Knowledge Base Used: Dictation processing and typing
Prompt To Fix With AI
This is a comment left during a code review.
Path: Sources/Fluid/Services/TypingService.swift
Line: 356-358
Comment:
**Page Title Controls Clipboard**
The routing predicate treats Safari’s page-controlled window title as proof that the destination is Google Docs or Slides. Any ordinary page whose title ends with one of those strings can therefore move otherwise directly typed dictation through `NSPasteboard.general`, where observers that do not honor the advisory transient markers can read or retain it. Verify the Google origin or another non-spoofable document identity before selecting the clipboard path.
**How this was verified:** Safari’s page-derived accessibility title is matched by suffix alone, after which the dictated string is written to the general pasteboard with only advisory markers.
**Knowledge Base Used:** [Dictation processing and typing](https://app.greptile.com/altic/-/custom-context/knowledge-base/altic-dev/fluidvoice/-/docs/dictation-processing-and-typing.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
@altic-dev any way to loosen the rules here since I have everything? |
|
The PR template lets us keep track of things easily. thanks for the PR but please adher to the template when possible :) |
github-actions
Bot
removed
needs PR template
grohith327
left a comment
grohith327
left a comment
There was a problem hiding this comment.
Looks good overall, just a couple of minor comments.
Note - this would be a temp fix though, when #811 is merged the clipboard method will become the default paste method which should fix all such issues
| return self.withTemporaryPasteboardString(text, restoreDelayMicros: 5_000_000) { | ||
| return self.withTemporaryPasteboardString(text, restoreDelayMicros: 1_500_000) { |
There was a problem hiding this comment.
Would recommend leaving this as is, this is already updated in the clipboard paste PR we have open
| /// Safari turns one synthesized unicode key event into a single `keypress` carrying only the | ||
| /// first character. Editors that build their text from `keypress`, which is what Google Docs and | ||
| /// Slides appear to do, therefore drop everything after it and need the clipboard path. The | ||
| /// title is looked up lazily so targets that match on bundle ID alone, and the far more common | ||
| /// targets that match nothing, never pay for an Accessibility round trip. |
There was a problem hiding this comment.
Minor - you can drop this comment
|
Its merged in already to my branch, we can wait on this if needed and
problem occurs after the nextupdate !
…On Sun, Sep 13, 2026 at 4:07 PM Rohith Gandhi G ***@***.***> wrote:
***@***.**** commented on this pull request.
Looks good overall, just a couple of minor comments.
Note - this would be a temp fix though, when #811
<#811> is merged the
clipboard method will become the default paste method which should fix all
such issues
------------------------------
In Sources/Fluid/Services/TypingService.swift
<#960 (comment)>:
> - return self.withTemporaryPasteboardString(text, restoreDelayMicros: 5_000_000) {
+ return self.withTemporaryPasteboardString(text, restoreDelayMicros: 1_500_000) {
Would recommend leaving this as is, this is already updated in the
clipboard paste PR we have open
------------------------------
In Sources/Fluid/Services/TypingService.swift
<#960 (comment)>:
> + /// Safari turns one synthesized unicode key event into a single `keypress` carrying only the
+ /// first character. Editors that build their text from `keypress`, which is what Google Docs and
+ /// Slides appear to do, therefore drop everything after it and need the clipboard path. The
+ /// title is looked up lazily so targets that match on bundle ID alone, and the far more common
+ /// targets that match nothing, never pay for an Accessibility round trip.
Minor - you can drop this comment
—
Reply to this email directly, view it on GitHub
<#960?email_source=notifications&email_token=BVSOW2TAVDNMD7MHZQBKNRD5O4SDXA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMJZGI2TSMJWGU42M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#pullrequestreview-5192591659>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BVSOW2RNU6T62QJJSYM2JET5O4SDXAVCNFSNUABGKJSXA33TNF2G64TZHMYTANRRGMZDOMZRGE5US43TOVSTWNJUGE3DGMZZGE2DJILWAI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
c2d6476 to
6b9d527
Compare
3 participants
Description
FluidVoice types a whole chunk of text as one synthetic keystroke. Safari turns that into a single
keypressevent carrying only the first character, and Google Docs and Slides build their text fromkeypress, so that first character is all they get. Sheets and ordinary text boxes read the full string from a different event, so they are fine, and so is every browser that is not Safari.The keystroke is fine. Safari still hands the whole string to the page through
textInput, soCGEventKeyboardSetUnicodeStringis doing the right thing. Sending one character at a time would dodgekeypressentirely, but f0d3855 moved away from that on purpose, because long text got cut off.This sends the text to the affected apps through the clipboard instead, the same path you get today by choosing Clipboard Paste in settings.
TypingServicealready did that for Ghostty, and now does the same when Safari's focused window is a Google Docs or Slides document. The rule keys on where dictation is known to break, not on the browser internals behind the failure. Sheets, every other site, and every other browser keep typing directly.Type of Change
Screenshots / Video
before-after.mp4
Related Issue or Discussion
Closes #958
Testing
Manual, against the released 1.6.9 build and then this branch: Google Docs and Google Slides in Safari now receive the whole phrase. Sheets, Chrome, and Firefox are unchanged from the matrix in the issue.
Native targets (Notes, Xcode) still take the direct-typing path.
TypingServicePasteOnlyRoutingTestscovers the routing predicate directly: known bundle IDs, Docs and Slides in Safari, Sheets staying on the direct path, Chromium and Gecko staying on the direct path, ordinary browsing, and native apps.