From 9ab91fc6151c1c993921346824e1466ca0aaf1b3 Mon Sep 17 00:00:00 2001 From: Deepankar Bajpeyi Date: Thu, 16 Jan 2025 17:18:28 +0100 Subject: [PATCH] [FormAnalyzer] add external link check for submit buttons (#736) * fix: check for outbound links * feat: check for fragment links --- dist/autofill-debug.js | 30 +++++--- dist/autofill.js | 30 +++++--- src/Form/FormAnalyzer.js | 40 +++++++---- .../Resources/assets/autofill-debug.js | 30 +++++--- swift-package/Resources/assets/autofill.js | 30 +++++--- test-forms/fedex_login.html | 69 +++++++++++++++++++ test-forms/index.json | 3 +- 7 files changed, 182 insertions(+), 50 deletions(-) create mode 100644 test-forms/fedex_login.html diff --git a/dist/autofill-debug.js b/dist/autofill-debug.js index 58030101b..4f46cf8de 100644 --- a/dist/autofill-debug.js +++ b/dist/autofill-debug.js @@ -11188,20 +11188,22 @@ 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) && (0, _autofillUtils.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')); } evaluateElement(el) { const string = (0, _autofillUtils.getTextShallow)(el); @@ -11228,9 +11230,19 @@ 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({ @@ -11241,7 +11253,7 @@ class FormAnalyzer { }); return; } - if (this.isElementExternalLink(el)) { + if (this.isOutboundLink(el)) { let shouldFlip = true; let strength = 1; // Don't flip forgotten password links diff --git a/dist/autofill.js b/dist/autofill.js index a4d1a1296..c7d5d0058 100644 --- a/dist/autofill.js +++ b/dist/autofill.js @@ -6825,20 +6825,22 @@ 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) && (0, _autofillUtils.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')); } evaluateElement(el) { const string = (0, _autofillUtils.getTextShallow)(el); @@ -6865,9 +6867,19 @@ 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({ @@ -6878,7 +6890,7 @@ class FormAnalyzer { }); return; } - if (this.isElementExternalLink(el)) { + if (this.isOutboundLink(el)) { let shouldFlip = true; let strength = 1; // Don't flip forgotten password links diff --git a/src/Form/FormAnalyzer.js b/src/Form/FormAnalyzer.js index 0bef7c7c8..5bbe03826 100644 --- a/src/Form/FormAnalyzer.js +++ b/src/Form/FormAnalyzer.js @@ -231,12 +231,11 @@ 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(); @@ -244,12 +243,16 @@ class FormAnalyzer { 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')); } evaluateElement(el) { @@ -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 diff --git a/swift-package/Resources/assets/autofill-debug.js b/swift-package/Resources/assets/autofill-debug.js index 58030101b..4f46cf8de 100644 --- a/swift-package/Resources/assets/autofill-debug.js +++ b/swift-package/Resources/assets/autofill-debug.js @@ -11188,20 +11188,22 @@ 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) && (0, _autofillUtils.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')); } evaluateElement(el) { const string = (0, _autofillUtils.getTextShallow)(el); @@ -11228,9 +11230,19 @@ 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({ @@ -11241,7 +11253,7 @@ class FormAnalyzer { }); return; } - if (this.isElementExternalLink(el)) { + if (this.isOutboundLink(el)) { let shouldFlip = true; let strength = 1; // Don't flip forgotten password links diff --git a/swift-package/Resources/assets/autofill.js b/swift-package/Resources/assets/autofill.js index a4d1a1296..c7d5d0058 100644 --- a/swift-package/Resources/assets/autofill.js +++ b/swift-package/Resources/assets/autofill.js @@ -6825,20 +6825,22 @@ 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) && (0, _autofillUtils.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')); } evaluateElement(el) { const string = (0, _autofillUtils.getTextShallow)(el); @@ -6865,9 +6867,19 @@ 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({ @@ -6878,7 +6890,7 @@ class FormAnalyzer { }); return; } - if (this.isElementExternalLink(el)) { + if (this.isOutboundLink(el)) { let shouldFlip = true; let strength = 1; // Don't flip forgotten password links diff --git a/test-forms/fedex_login.html b/test-forms/fedex_login.html new file mode 100644 index 000000000..784d8477f --- /dev/null +++ b/test-forms/fedex_login.html @@ -0,0 +1,69 @@ +CREATE A USER + ID +
+ +
+
+ +
+
+
+ +
\ No newline at end of file diff --git a/test-forms/index.json b/test-forms/index.json index ed90df92e..7e94d12fd 100644 --- a/test-forms/index.json +++ b/test-forms/index.json @@ -551,5 +551,6 @@ { "html": "deltamath_reset_password.html"}, { "html": "freshdirect_com_login.html"}, { "html": "auth_wetransfer_com_login.html"}, - { "html": "revolut_business_login.html"} + { "html": "revolut_business_login.html"}, + { "html": "fedex_login.html"} ]