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] Fix paperlesspost.com login form #711

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

Conversation

dbajpeyi
Copy link
Collaborator

@dbajpeyi dbajpeyi commented Nov 28, 2024

Reviewer: @shakyShane @GioSensation
Asana: https://app.asana.com/0/1200930669568058/1208734403743969/f

Description

paperlesspost.com login form is treated by autofill as signup because the button element with text "Do you have an account? Sign Up" get's marked as "likely a submit", causing the score to skyrocket to +20:

Screenshot 2024-11-28 at 14 52 41

The problem is that this condition of el.type is always 'submit', unless type was explicitly set to something else.

// If there is another element marked as submit and this is not, flip back to false
if (el.type !== 'submit' && el !== submit) {
    likelyASubmit = false;
}

Checking el.type defaults to 'submit' when nothing is set on the button (as buttons are submit by default), although in think in this case we want to be explicitly checking the submit attribute with getAttrinute('type') instead, to know that a button was explicitly set using submit.

Form signals (before)
image

Form signals (after)
Screenshot 2024-11-28 at 14 46 50

Steps to test

  1. Go to paperlesspost.com
  2. Try logging in
  3. (Before fix) you see email protection,
  4. (After fix) you should see login autofill.

@dbajpeyi dbajpeyi changed the title Dbajpeyi/fix/paperlesspost.com [FornAnalyzer] Fix paperlesspost.com login form Nov 28, 2024
@dbajpeyi dbajpeyi changed the title [FornAnalyzer] Fix paperlesspost.com login form [FormAnalyzer] Fix paperlesspost.com login form Nov 28, 2024
@dbajpeyi dbajpeyi marked this pull request as ready for review November 28, 2024 13:54
@@ -272,7 +272,7 @@ class FormAnalyzer {
if (likelyASubmit) {
this.form.querySelectorAll('input[type=submit], button[type=submit]').forEach((submit) => {
// If there is another element marked as submit and this is not, flip back to false
if (el.type !== 'submit' && el !== submit) {
if (el.getAttribute('type') !== 'submit' && el !== submit) {
Copy link
Collaborator Author

@dbajpeyi dbajpeyi Nov 28, 2024

Choose a reason for hiding this comment

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

I think this is what we want to do here - check if this element if it's explicitly a submit type, although I am not sure.

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.

1 participant