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.

Sign up for GitHub

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] Tweak regex to match forgot password attribute #712

Merged
merged 2 commits into from
Dec 6, 2024
Merged

[FormAnalyzer] Tweak regex to match forgot password attribute #712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0d05868
fix: tweak regex to match the form attribute
dbajpeyi Nov 29, 2024
1836ccf
Merge branch 'main' into dbajpeyi/bug/sharperimages.com
dbajpeyi Dec 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dist/autofill-debug.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/autofill.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/Form/matching-config/__generated__/compiled-matching-config.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/Form/matching-config/matching-config-source.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ const matchingConfiguration = {
},
resetPasswordLink: {
match:
"(forgot(ten)?|reset|don't remember) (your )?password|password forgotten" +
"(forgot(ten)?|reset|don't remember).?(your )?password|password forgotten" +
// Italian
'|password dimenticata|reset(?:ta) password|recuper[ao] password' +
// German
Expand Down
2 changes: 1 addition & 1 deletion swift-package/Resources/assets/autofill-debug.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion swift-package/Resources/assets/autofill.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion test-forms/index.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@
{ "html": "samash_signup.html" },
{ "html": "financialtimes_login.html", "title": "Login" },
{ "html": "containerstore_login_signup.html" },
{ "html": "containerstore_forgot_password.html", "expectedFailures": ["username"] },
{ "html": "containerstore_forgot_password.html", "expectedFailures": ["password.new"] },
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So here we are breaking the password field now, but on the other hand we're fixing the username field. I think its a net win, given the site in question (sharperimages.com).

Copy link
Member

Choose a reason for hiding this comment

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

This is unfortunate. I've spent 30 mins looking for a quick win but couldn't find anything obvious.

Note that containerstore has a higher domain rank than what we're fixing now, but in this case we're just changing what breaks on containerstore, so it's not a net regression. FYI, you can check domain rank using https://tranco-list.eu/query.

Copy link
Collaborator Author

@dbajpeyi dbajpeyi Dec 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah i've been using that tool for triaging as well - but I also considered this is a neutral change as mentioned in the comment, regardless of the rank.

Actually we fix the "first step" of the process which is arguably better 😄

{ "html": "containerstore_newsletter.html" },
{ "html": "containerstore_checkout.html", "expectedSubmitFalsePositives": 1, "expectedSubmitFalseNegatives": 2 },
{ "html": "containerstore_hidden_contact.html" },
Expand Down
Loading