From 55466f10a339843cb79a27af62d5d61c030740b2 Mon Sep 17 00:00:00 2001 From: Emanuele Feliziani Date: Wed, 27 Sep 2023 16:13:27 +0200 Subject: [PATCH] Fix perf regressions (#385) Signed-off-by: Emanuele Feliziani --- dist/autofill-debug.js | 33 +++++++++---------- dist/autofill.js | 33 +++++++++---------- src/Form/FormAnalyzer.js | 18 ++++++---- src/Scanner.js | 3 ++ .../Resources/assets/autofill-debug.js | 33 +++++++++---------- swift-package/Resources/assets/autofill.js | 33 +++++++++---------- 6 files changed, 78 insertions(+), 75 deletions(-) diff --git a/dist/autofill-debug.js b/dist/autofill-debug.js index 4d7388a9c..3472e1d2a 100644 --- a/dist/autofill-debug.js +++ b/dist/autofill-debug.js @@ -10315,6 +10315,8 @@ class FormAnalyzer { shouldCheckUnifiedForm = false, shouldBeConservative = false } = _ref; + // If the string is empty or too long (noisy) do nothing + if (!string || string.length > _constants.constants.TEXT_LENGTH_CUTOFF) return this; const matchesLogin = /current.?password/i.test(string) || this.matching.getDDGMatcherRegex('loginRegex')?.test(string) || this.matching.getDDGMatcherRegex('resetPasswordLink')?.test(string); // Check explicitly for unified login/signup forms @@ -10373,12 +10375,9 @@ class FormAnalyzer { }); } evaluatePageHeadings() { - const headings = document.querySelectorAll('h1, h2, h3, [class*="title"], [id*="title"]'); - headings.forEach(_ref2 => { - let { - textContent - } = _ref2; - textContent = (0, _matching.removeExcessWhitespace)(textContent || ''); + const headings = document.querySelectorAll('h1, h2, h3'); + headings.forEach(heading => { + const textContent = (0, _matching.removeExcessWhitespace)(heading.textContent || ''); this.updateSignal({ string: textContent, strength: 0.5, @@ -10455,15 +10454,12 @@ class FormAnalyzer { }); } else { // any other case - // only consider the el if it's a small text to avoid noisy disclaimers - if ((0, _matching.removeExcessWhitespace)(el.textContent)?.length < _constants.constants.TEXT_LENGTH_CUTOFF) { - this.updateSignal({ - string, - strength: 1, - signalType: `generic: ${string}`, - shouldCheckUnifiedForm: true - }); - } + this.updateSignal({ + string, + strength: 1, + signalType: `generic: ${string}`, + shouldCheckUnifiedForm: true + }); } } evaluateForm() { @@ -10520,11 +10516,11 @@ class FormAnalyzer { } // Read form attributes to find a signal - const hasCCAttribute = [...formEl.attributes].some(_ref3 => { + const hasCCAttribute = [...formEl.attributes].some(_ref2 => { let { name, value - } = _ref3; + } = _ref2; return /(credit|payment).?card/i.test(`${name}=${value}`); }); if (hasCCAttribute) { @@ -13878,6 +13874,9 @@ class DefaultScanner { return; } + // Do not add explicitly search forms + if (parentForm.role === 'search') return; + // Check if the forms we've seen are either disconnected, // or are parent/child of the currently-found form let previouslyFoundParent, childForm; diff --git a/dist/autofill.js b/dist/autofill.js index 2371f2b2b..8fa1a238c 100644 --- a/dist/autofill.js +++ b/dist/autofill.js @@ -6370,6 +6370,8 @@ class FormAnalyzer { shouldCheckUnifiedForm = false, shouldBeConservative = false } = _ref; + // If the string is empty or too long (noisy) do nothing + if (!string || string.length > _constants.constants.TEXT_LENGTH_CUTOFF) return this; const matchesLogin = /current.?password/i.test(string) || this.matching.getDDGMatcherRegex('loginRegex')?.test(string) || this.matching.getDDGMatcherRegex('resetPasswordLink')?.test(string); // Check explicitly for unified login/signup forms @@ -6428,12 +6430,9 @@ class FormAnalyzer { }); } evaluatePageHeadings() { - const headings = document.querySelectorAll('h1, h2, h3, [class*="title"], [id*="title"]'); - headings.forEach(_ref2 => { - let { - textContent - } = _ref2; - textContent = (0, _matching.removeExcessWhitespace)(textContent || ''); + const headings = document.querySelectorAll('h1, h2, h3'); + headings.forEach(heading => { + const textContent = (0, _matching.removeExcessWhitespace)(heading.textContent || ''); this.updateSignal({ string: textContent, strength: 0.5, @@ -6510,15 +6509,12 @@ class FormAnalyzer { }); } else { // any other case - // only consider the el if it's a small text to avoid noisy disclaimers - if ((0, _matching.removeExcessWhitespace)(el.textContent)?.length < _constants.constants.TEXT_LENGTH_CUTOFF) { - this.updateSignal({ - string, - strength: 1, - signalType: `generic: ${string}`, - shouldCheckUnifiedForm: true - }); - } + this.updateSignal({ + string, + strength: 1, + signalType: `generic: ${string}`, + shouldCheckUnifiedForm: true + }); } } evaluateForm() { @@ -6575,11 +6571,11 @@ class FormAnalyzer { } // Read form attributes to find a signal - const hasCCAttribute = [...formEl.attributes].some(_ref3 => { + const hasCCAttribute = [...formEl.attributes].some(_ref2 => { let { name, value - } = _ref3; + } = _ref2; return /(credit|payment).?card/i.test(`${name}=${value}`); }); if (hasCCAttribute) { @@ -9933,6 +9929,9 @@ class DefaultScanner { return; } + // Do not add explicitly search forms + if (parentForm.role === 'search') return; + // Check if the forms we've seen are either disconnected, // or are parent/child of the currently-found form let previouslyFoundParent, childForm; diff --git a/src/Form/FormAnalyzer.js b/src/Form/FormAnalyzer.js index 18dc656f2..c69078daf 100644 --- a/src/Form/FormAnalyzer.js +++ b/src/Form/FormAnalyzer.js @@ -117,6 +117,12 @@ class FormAnalyzer { shouldCheckUnifiedForm = false, shouldBeConservative = false }) { + // If the string is empty or too long (noisy) do nothing + if ( + !string || + string.length > constants.TEXT_LENGTH_CUTOFF + ) return this + const matchesLogin = /current.?password/i.test(string) || this.matching.getDDGMatcherRegex('loginRegex')?.test(string) || this.matching.getDDGMatcherRegex('resetPasswordLink')?.test(string) // Check explicitly for unified login/signup forms @@ -177,9 +183,10 @@ class FormAnalyzer { } evaluatePageHeadings () { - const headings = document.querySelectorAll('h1, h2, h3, [class*="title"], [id*="title"]') - headings.forEach(({textContent}) => { - textContent = removeExcessWhitespace(textContent || '') + const headings = document.querySelectorAll('h1, h2, h3') + headings.forEach((heading) => { + const textContent = removeExcessWhitespace(heading.textContent || '') + this.updateSignal({ string: textContent, strength: 0.5, @@ -256,10 +263,7 @@ class FormAnalyzer { this.updateSignal({string, strength, signalType: `external link: ${string}`, shouldFlip}) } else { // any other case - // only consider the el if it's a small text to avoid noisy disclaimers - if (removeExcessWhitespace(el.textContent)?.length < constants.TEXT_LENGTH_CUTOFF) { - this.updateSignal({string, strength: 1, signalType: `generic: ${string}`, shouldCheckUnifiedForm: true}) - } + this.updateSignal({string, strength: 1, signalType: `generic: ${string}`, shouldCheckUnifiedForm: true}) } } diff --git a/src/Scanner.js b/src/Scanner.js index 2810f9f56..989e14ebf 100644 --- a/src/Scanner.js +++ b/src/Scanner.js @@ -231,6 +231,9 @@ class DefaultScanner { return } + // Do not add explicitly search forms + if (parentForm.role === 'search') return + // Check if the forms we've seen are either disconnected, // or are parent/child of the currently-found form let previouslyFoundParent, childForm diff --git a/swift-package/Resources/assets/autofill-debug.js b/swift-package/Resources/assets/autofill-debug.js index 4d7388a9c..3472e1d2a 100644 --- a/swift-package/Resources/assets/autofill-debug.js +++ b/swift-package/Resources/assets/autofill-debug.js @@ -10315,6 +10315,8 @@ class FormAnalyzer { shouldCheckUnifiedForm = false, shouldBeConservative = false } = _ref; + // If the string is empty or too long (noisy) do nothing + if (!string || string.length > _constants.constants.TEXT_LENGTH_CUTOFF) return this; const matchesLogin = /current.?password/i.test(string) || this.matching.getDDGMatcherRegex('loginRegex')?.test(string) || this.matching.getDDGMatcherRegex('resetPasswordLink')?.test(string); // Check explicitly for unified login/signup forms @@ -10373,12 +10375,9 @@ class FormAnalyzer { }); } evaluatePageHeadings() { - const headings = document.querySelectorAll('h1, h2, h3, [class*="title"], [id*="title"]'); - headings.forEach(_ref2 => { - let { - textContent - } = _ref2; - textContent = (0, _matching.removeExcessWhitespace)(textContent || ''); + const headings = document.querySelectorAll('h1, h2, h3'); + headings.forEach(heading => { + const textContent = (0, _matching.removeExcessWhitespace)(heading.textContent || ''); this.updateSignal({ string: textContent, strength: 0.5, @@ -10455,15 +10454,12 @@ class FormAnalyzer { }); } else { // any other case - // only consider the el if it's a small text to avoid noisy disclaimers - if ((0, _matching.removeExcessWhitespace)(el.textContent)?.length < _constants.constants.TEXT_LENGTH_CUTOFF) { - this.updateSignal({ - string, - strength: 1, - signalType: `generic: ${string}`, - shouldCheckUnifiedForm: true - }); - } + this.updateSignal({ + string, + strength: 1, + signalType: `generic: ${string}`, + shouldCheckUnifiedForm: true + }); } } evaluateForm() { @@ -10520,11 +10516,11 @@ class FormAnalyzer { } // Read form attributes to find a signal - const hasCCAttribute = [...formEl.attributes].some(_ref3 => { + const hasCCAttribute = [...formEl.attributes].some(_ref2 => { let { name, value - } = _ref3; + } = _ref2; return /(credit|payment).?card/i.test(`${name}=${value}`); }); if (hasCCAttribute) { @@ -13878,6 +13874,9 @@ class DefaultScanner { return; } + // Do not add explicitly search forms + if (parentForm.role === 'search') return; + // Check if the forms we've seen are either disconnected, // or are parent/child of the currently-found form let previouslyFoundParent, childForm; diff --git a/swift-package/Resources/assets/autofill.js b/swift-package/Resources/assets/autofill.js index 2371f2b2b..8fa1a238c 100644 --- a/swift-package/Resources/assets/autofill.js +++ b/swift-package/Resources/assets/autofill.js @@ -6370,6 +6370,8 @@ class FormAnalyzer { shouldCheckUnifiedForm = false, shouldBeConservative = false } = _ref; + // If the string is empty or too long (noisy) do nothing + if (!string || string.length > _constants.constants.TEXT_LENGTH_CUTOFF) return this; const matchesLogin = /current.?password/i.test(string) || this.matching.getDDGMatcherRegex('loginRegex')?.test(string) || this.matching.getDDGMatcherRegex('resetPasswordLink')?.test(string); // Check explicitly for unified login/signup forms @@ -6428,12 +6430,9 @@ class FormAnalyzer { }); } evaluatePageHeadings() { - const headings = document.querySelectorAll('h1, h2, h3, [class*="title"], [id*="title"]'); - headings.forEach(_ref2 => { - let { - textContent - } = _ref2; - textContent = (0, _matching.removeExcessWhitespace)(textContent || ''); + const headings = document.querySelectorAll('h1, h2, h3'); + headings.forEach(heading => { + const textContent = (0, _matching.removeExcessWhitespace)(heading.textContent || ''); this.updateSignal({ string: textContent, strength: 0.5, @@ -6510,15 +6509,12 @@ class FormAnalyzer { }); } else { // any other case - // only consider the el if it's a small text to avoid noisy disclaimers - if ((0, _matching.removeExcessWhitespace)(el.textContent)?.length < _constants.constants.TEXT_LENGTH_CUTOFF) { - this.updateSignal({ - string, - strength: 1, - signalType: `generic: ${string}`, - shouldCheckUnifiedForm: true - }); - } + this.updateSignal({ + string, + strength: 1, + signalType: `generic: ${string}`, + shouldCheckUnifiedForm: true + }); } } evaluateForm() { @@ -6575,11 +6571,11 @@ class FormAnalyzer { } // Read form attributes to find a signal - const hasCCAttribute = [...formEl.attributes].some(_ref3 => { + const hasCCAttribute = [...formEl.attributes].some(_ref2 => { let { name, value - } = _ref3; + } = _ref2; return /(credit|payment).?card/i.test(`${name}=${value}`); }); if (hasCCAttribute) { @@ -9933,6 +9929,9 @@ class DefaultScanner { return; } + // Do not add explicitly search forms + if (parentForm.role === 'search') return; + // Check if the forms we've seen are either disconnected, // or are parent/child of the currently-found form let previouslyFoundParent, childForm;