Skip to content

helium/ui/find-bar: add match case and whole word toggles to find menu - #2762

Open
finaIcutpro wants to merge 3 commits into
imputnet:mainfrom
finaIcutpro:find-exact-match
Open

finaIcutpro wants to merge 3 commits into
imputnet:mainfrom
finaIcutpro:find-exact-match

Conversation

@finaIcutpro

@finaIcutpro finaIcutpro commented Oct 20, 2026 •

Copy link
Copy Markdown

For your pull request to not get closed without review, please confirm that:

  • An issue exists where the maintainers agreed that this should be implemented
    (an approved feature request, or confirmed bug).
  • I tested that my contribution works locally, and does not break anything,
    otherwise I have marked my PR as draft.
  • If my contribution is non-trivial, I did not use AI to write most of it.
  • I understand that I will be permanently banned from interacting with this
    organization if I lied by checking any of these checkboxes.

Tested on (check one or more):

  • Windows
  • macOS
  • Linux

I found it annoying to have to deal with exact word and case sensitive matching in the Cmd + F menu. This fixes that.
Screenshot 2026-10-19 at 9 51 23 PM

Screen.Recording.2026-10-19.at.9.51.37.PM.mov

@greptile-apps

greptile-apps Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

RetriggerView in Greptile

[Medium impact] The change affects the browser's find-in-page workflow.

The PR appears safe to merge, with a non-blocking issue where open windows can overwrite each other's search options.

Reviews (1) · Last reviewed commit: "add find in page switches" · Reviewed by Greptile

Comment thread patches/helium/ui/find-bar-options.patch Outdated
@finaIcutpro

Copy link
Copy Markdown
Author

I have no further changes I want to make && am happy with the code. I believe this is ready to merge.

@dumbmoron dumbmoron added the bot:review Request a Helium bot review label Oct 20, 2026
@helium-gardener helium-gardener Bot removed the bot:review Request a Helium bot review label Oct 20, 2026
@helium-gardener

Copy link
Copy Markdown

Possibly related issues

These relationships were inferred from the effective change and may need maintainer confirmation:

  • May be resolved by this PR: imputnet/helium#311 — Adds the requested case-sensitive find-bar control and routes it through find requests to Blink's case-sensitive matcher; also adds the whole-word option discussed on the issue.

@helium-gardener helium-gardener Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found four actionable correctness issues involving cached iframe matches, PDF searches, and shared find-option state.

Review walkthrough

Changes

Files Summary
patches/helium/core/find-exact-match.patch Adds whole_word to Mojo find options and forwards it to Blink's active-match and scoping matchers. Reviewed renderer caching and the separate plugin/PDF path.
patches/helium/ui/find-bar-options.patch Adds animated option rows, profile-level matching state, and search-session detection for option changes. Reviewed callbacks, ownership, focus handling, cross-window synchronization, and PDF restarts.
i18n/source.gen.json
patches/series
Registers three matching UI strings and orders the renderer prerequisite before the UI patch, which follows the existing find-bar patch.

Review checks

Check Result Notes
Scope ✅ Passed Confirmed four changed repository files and ten effective Chromium targets. No version transition or patch-application warnings; reported locations match reviewable provenance.
Project Policy ✅ Passed Neither new patch changes upstream tests, test data, expectations, or test targets.
Correctness And Safety ⚠️ Finding Identified stale renderer caches, two PDF integration defects, and unsynchronized shared controls. Reviewed desktop build wiring, lifetime, and incognito service separation.
Chromium Idioms ✅ Passed The implementation uses existing Blink whole-word matching, Views builders, toggle controls, and animation APIs.
Simplicity And Decomposition ✅ Passed The row helper and callbacks have local responsibilities; no separate actionable abstraction or decomposition concern.
Patch Organization ✅ Passed Renderer support and UI integration are coherent patches, with prerequisites ordered correctly and UI work adjacent to the existing find-bar patch.

Analysis record

  • Changed files considered: 4
  • Findings: 4 new, 0 retained, 0 resolved
  • Source snapshot: e5b1aeac84f7
  • Codex: gpt-6.1-sol, reasoning max, fast mode on, web search cached
  • Patch application: clean
  • Chromium context tool calls: none
  • Maintainer conventions applied: ignore-disabled-telemetry, preserve-incremental-loading, script-resource-lifecycle, ui-command-contexts, validate-supported-ui-paths, verify-runtime-selection-paths, visible-ui-anchors

Limitations

  • Static inspection only; builds, tests, and interactive UI validation were not run.
  • The supplied screenshots were available, but the PR's video attachment was unavailable.

Comment thread patches/helium/core/find-exact-match.patch
Comment thread patches/helium/ui/find-bar-options.patch Outdated
Comment thread patches/helium/ui/find-bar-options.patch
Comment thread patches/helium/ui/find-bar-options.patch Outdated
@finaIcutpro

finaIcutpro commented Oct 20, 2026 •

Copy link
Copy Markdown
Author

all gardener suggestions have been triaged && resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants