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] add external link check for submit buttons #736

Merged
merged 2 commits into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
30 changes: 21 additions & 9 deletions dist/autofill-debug.js

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

30 changes: 21 additions & 9 deletions dist/autofill.js

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

40 changes: 27 additions & 13 deletions src/Form/FormAnalyzer.js
Original file line number Diff line number Diff line change
Expand Up @@ -231,25 +231,28 @@ class FormAnalyzer {
}

/**
* Function that checks if the element is an external link or a custom web element that
* encapsulates a link.
* Function that checks if the element is link like and navigating away from the current page
* @param {any} el
* @returns {boolean}
*/
isElementExternalLink(el) {
isOutboundLink(el) {
// Checks if the element is present in the cusotm elements registry and ends with a '-link' suffix.
// If it does, it checks if it contains an anchor element inside.
const tagName = el.nodeName.toLowerCase();
const isCustomWebElementLink =
customElements?.get(tagName) != null && /-link$/.test(tagName) && findElementsInShadowTree(el, 'a').length > 0;

// if an external link matches one of the regexes, we assume the match is not pertinent to the current form
const isElementLink =
(el instanceof HTMLAnchorElement && el.href && el.getAttribute('href') !== '#') ||
(el.getAttribute('role') || '').toUpperCase() === 'LINK' ||
el.matches('button[class*=secondary]');

return isCustomWebElementLink || isElementLink;
const isElementLikelyALink = (el) => {
if (el == null) return false;
return (
(el instanceof HTMLAnchorElement && el.href && !el.getAttribute('href')?.startsWith('#')) ||
(el.getAttribute('role') || '').toUpperCase() === 'LINK' ||
el.matches('button[class*=secondary]')
);
};

return isCustomWebElementLink || isElementLikelyALink(el) || isElementLikelyALink(el.closest('a'));
Copy link
Collaborator Author

@dbajpeyi dbajpeyi Jan 14, 2025

Choose a reason for hiding this comment

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

Added the use of closest here, it seems to be performant as it's natively implemented.

}

evaluateElement(el) {
Expand Down Expand Up @@ -278,15 +281,26 @@ class FormAnalyzer {
}
});
} else {
// Here we don't think this is a submit, so if there is another submit in the form, flip the score
const thereIsASubmitButton = Boolean(this.form.querySelector('input[type=submit], button[type=submit]'));
shouldFlip = thereIsASubmitButton && this.shouldFlipScoreForButtonText(string);
// Here we don't think this is a submit, so determine if we should flip the score
const hasAnotherSubmitButton = Boolean(this.form.querySelector('input[type=submit], button[type=submit]'));
const buttonText = string;

if (hasAnotherSubmitButton) {
// If there's another submit button, flip based on text content alone
shouldFlip = this.shouldFlipScoreForButtonText(buttonText);
} else {
// With no submit button, only flip if it's an outbound link that navigates away, and also match the text
// Here we want to be more conservative, because we don't want to flip for every link given that there was no
// submit button detected on the form, hence the extra check for the link.
const isOutboundLink = this.isOutboundLink(el);
shouldFlip = isOutboundLink && this.shouldFlipScoreForButtonText(buttonText);
}
}
const strength = likelyASubmit ? 20 : 4;
this.updateSignal({ string, strength, signalType: `button: ${string}`, shouldFlip });
return;
}
if (this.isElementExternalLink(el)) {
if (this.isOutboundLink(el)) {
let shouldFlip = true;
let strength = 1;
// Don't flip forgotten password links
Expand Down
30 changes: 21 additions & 9 deletions swift-package/Resources/assets/autofill-debug.js

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

30 changes: 21 additions & 9 deletions swift-package/Resources/assets/autofill.js

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

Loading
Loading