Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FormAnalyzer] Evaluate checkboxes #734

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dbajpeyi
Copy link
Collaborator

@dbajpeyi dbajpeyi commented Jan 8, 2025

Reviewer: @alistairjcbrown
Asana: NA

Description

Use text from "Stay signed in" and "Remember me" checkboxes to decrease score (aka increase login score). This is a pretty good hint for login forms, where those checkboxes are available.

The approach/change needed:

  1. In evaluateElement add a new code block to check if the element matches a checkbox (enabled only)
  2. Modify the getShallowText logic to get the related label string (only the first one to keep it simple),
  3. Decrease the score if the the label text matches "stay signed in" or "remember me" (case insensitive).

Steps to test

NA

@dbajpeyi dbajpeyi changed the title feat: evaluate checkboxes [WIP] feat: evaluate checkboxes Jan 8, 2025
Copy link
Member

@alistairjcbrown alistairjcbrown left a comment

Choose a reason for hiding this comment

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

This logic of the code makes sense to me 👍
My only concern is we’re adding it because it feels like a good idea, but without any supporting use case for it — are there any forms/tests where we’ve seen a weaker signal and having this improves them?

src/Form/FormAnalyzer.js Outdated Show resolved Hide resolved
src/autofill-utils.js Show resolved Hide resolved
@dbajpeyi dbajpeyi force-pushed the dbajpeyi/feat/checkbox-evaluation branch from f7a6e90 to adda2ab Compare January 8, 2025 18:58
@dbajpeyi dbajpeyi marked this pull request as ready for review January 8, 2025 19:03
@dbajpeyi dbajpeyi changed the title [WIP] feat: evaluate checkboxes [FormAnalyzer] Evaluate checkboxes Jan 8, 2025
@dbajpeyi
Copy link
Collaborator Author

dbajpeyi commented Jan 8, 2025

@alistairjcbrown
Clarification: This PR IMO is less critical the other ones, so we can continue the review of this after the others where the solutions are clearer.

This just an idea that came as a conversation topic related to https://app.asana.com/0/1200930669568058/1209022781094891/f (I cannot find the comment right now sorry :(). The freshdirect.com example is the one we can try it out with, where the signal without it is -3, and with it is -6. I think the form is clearly a sign-in, even without forgot password, looking at it visually and the "Stay signed in" checkbox is part of that:
Screenshot 2025-01-09 at 13 07 00

If it creates regression or something we will see it in reports, and I can revert it. But i feel confident enough that we need it!

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