superset icon indicating copy to clipboard operation
superset copied to clipboard

fix(SearchFilter): add data-1p-ignore attribute to Search input for improved accessibility

Open LuisSanchez opened this issue 1 month ago • 3 comments

SUMMARY

Password managers like 1Password ignore autocomplete="off" when the input has name="name" because they recognize common field name patterns (like "name", "email", "password", etc.). The happens only if the ListViewFilters has id: 'name' and input: 'search'.

Solution: Added a new inputName property to the ListViewFilter type that allows you to set a custom name for the input element that password managers won't recognize:

  1. types.ts - Added inputName?: string; to the ListViewFilter interface.
  2. Filters/index.tsx - Updated to use inputName ?? id when passing the name prop to SearchFilter
  3. GroupsList/index.tsx - Changed the Name filter to use inputName: 'group_list_search'

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

A 1password auto-fill icon was shown for some search components when the name was not passed correctly. Resulting in an attr like the following:

name="name"
image

After:

Example of the pages failing

Annotation Layer, Alert Report List, Groups List, Roles List, Row Level Security List and Tags.

name="annotation_layer_list_search"
image

TESTING INSTRUCTIONS

  1. Login.
  2. Enter user settings that has a search input box, for example Annotations Layers.
  3. Try typing on the search box, the 1password icon must not appear.

ADDITIONAL INFORMATION

  • [x] Has associated issue:
  • [ ] Required feature flags:
  • [ ] Changes UI
  • [ ] Includes DB Migration (follow approval process in SIP-59)
    • [ ] Migration is atomic, supports rollback & is backwards-compatible
    • [ ] Confirm DB migration upgrade and downgrade tested
    • [ ] Runtime estimates and downtime expectations provided
  • [ ] Introduces new feature or API
  • [ ] Removes existing feature or API

LuisSanchez avatar Nov 20 '25 13:11 LuisSanchez

Code Review Agent Run #130649

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: 143b23a..bb79f69
    • superset-frontend/src/components/ListView/Filters/Search.tsx
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by Bito Logo

bito-code-review[bot] avatar Nov 20 '25 13:11 bito-code-review[bot]

Can we check what 1password tries to fill this in with and change name/id attributes accordingly?

Sure, I'll try to check it and see if with an update of the name/id will suffice

LuisSanchez avatar Nov 24 '25 04:11 LuisSanchez

@msyavuz I think we're good for a re-review here :)

rusackas avatar Dec 08 '25 17:12 rusackas

CodeAnt AI is reviewing your PR.

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review

  • [ ] Memo dependency bug
    The filters useMemo has an empty dependency array but references variables from the component scope (for example user and isReportEnabled indirectly). This can lead to stale filter config if props change. Verify and update the dependencies so the memo recalculates when relevant inputs change.

  • [ ] Ensure DOM passthrough
    The Input component is from a shared component library; confirm that autoComplete is forwarded to the actual DOM . If that wrapper doesn't forward unknown props, the autoComplete setting won't reach the browser. If not forwarded, update the wrapper or explicitly set props on the underlying input element.

  • [ ] Missing anti-autofill attribute
    The PR title mentions adding a data-1p-ignore attribute to the search input to help avoid password-manager autofill. The file change only adds an autoComplete prop but does not add the data-1p-ignore attribute to the rendered Input. Verify whether the data attribute should be added here (or intentionally omitted and implemented elsewhere) to reliably prevent 1Password/other managers from showing autofill icons when fields use common names.

  • [ ] Prop forwarding
    The new props (inputName and autoComplete) are passed into SearchFilter here, but it must be verified that SearchFilter accepts and forwards those props to the underlying . If not forwarded, the changes will have no effect (particularly the autoComplete behavior and any data attribute used to suppress password-manager behavior).

  • [ ] Effective mitigation for 1Password
    The PR description mentions adding data-1p-ignore to inputs, but this file only changes the name/autoComplete props. Confirm that SearchFilter (and other consumers) set the data-1p-ignore attribute (or an equivalent approach) on the DOM input; otherwise 1Password may still behave undesirably.

  • [ ] Behavior verification
    Ensure the SearchFilter/ListView pipeline actually uses inputName to set the final input name attribute (and/or data-1p-ignore) on the rendered . If SearchFilter still favors id or ignores inputName, the change will have no effect.

  • [ ] Propagation check
    Verify that the new inputName field is actually consumed downstream (Filters/index.tsx -> SearchFilter) so that the input element receives the custom name attribute and the data-1p-ignore attribute required to suppress password-manager UI. If inputName is added here but not used by the filter rendering component, the change will have no effect.

  • [ ] Attribute propagation risk
    Adding inputName assumes the downstream ListView/SearchFilter implementation consumes and applies it to the rendered input element. If the component that renders the search input doesn't read this property, this change will have no effect. Verify the prop is passed through and applied as the name attribute on the actual .

CodeAnt AI finished reviewing your PR.

Let us know if any of the codeant suggestions are worth consideration, but otherwise happy to merge.

rusackas avatar Dec 11 '25 23:12 rusackas

Let's also update the pr title as well

msyavuz avatar Dec 12 '25 07:12 msyavuz

@msyavuz I checked the codeant suggestions and I don't think they are worth to be taken into consideration, mostly because it is confused about the 'id' param and we send a 'name' value (which is totally understandable), other than that, I think we are good for the merge!

LuisSanchez avatar Dec 12 '25 20:12 LuisSanchez

CodeAnt AI is running Incremental review

Rebase

LuisSanchez avatar Dec 16 '25 03:12 LuisSanchez

CodeAnt AI is running Incremental review

CodeAnt AI is running Incremental review


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X · Reddit · LinkedIn