refactor: optimize event listener for vue:settled to handle HTMLTableElement variants#234
Conversation
vue:settled to handle HTMLTableElement variants
There was a problem hiding this comment.
Pull request overview
Refactors the vue:settled event handler in the userscript to better detect/hide elements (including table-related layouts) by extracting the OCR pass into a helper and expanding the DOM/style-based heuristics.
Changes:
- Extracts the OCR-based targeting logic into a new
ExecuteOCR(...)helper and adds size constraints for OCR candidates. - Expands targeting heuristics to account for
HTMLTableElement/ table-cell variants and adds an additional exclusion filter. - Replaces the previous “FrameTargeted” handling with table-specific targeting and hiding logic.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .filter(Child => | ||
| Child instanceof HTMLImageElement || | ||
| getComputedStyle(Child).backgroundImage !== 'none' | ||
| ) | ||
| ).filter(Child => parseFloat(getComputedStyle(Child).getPropertyValue('width')) >= 5 && parseFloat(getComputedStyle(Child).getPropertyValue('height')) >= 5) | ||
| .filter(Child => parseFloat(getComputedStyle(Child).getPropertyValue('width')) <= 50 && parseFloat(getComputedStyle(Child).getPropertyValue('height')) <= 50) |
There was a problem hiding this comment.
This chain calls getComputedStyle(Child) multiple times per element for width/height checks, which can be expensive and can trigger repeated style calculations. Consider caching the computed style (or width/height) per Child before filtering to reduce overhead in the vue:settled handler.
| if (Children.filter(Child => | ||
| parseFloat(getComputedStyle(Child).getPropertyValue('padding-top')) >= 5 && | ||
| parseFloat(getComputedStyle(Child).getPropertyValue('border-bottom-width')) >= 0.1 | ||
| ).length === 1) return true | ||
| // HTMLTableElement | ||
| return Children.filter(Child => (Child instanceof HTMLTableElement || Child instanceof HTMLTableCellElement) && | ||
| parseFloat(getComputedStyle(Child).getPropertyValue('padding-top')) >= 5 && parseFloat(getComputedStyle(Child).getPropertyValue('padding-bottom')) >= 5).length >= 2 | ||
| }) | ||
| Targeted = Targeted.filter(Ele => { | ||
| let Children = [...Ele.querySelectorAll('*')].filter(Child => Child instanceof HTMLElement) |
There was a problem hiding this comment.
Ele.querySelectorAll('*') is executed again in subsequent Targeted = Targeted.filter(...) steps (e.g., the next filter starting at line 101). Consider collecting Children once per Ele (or combining these predicates) to avoid repeated full DOM traversals for the same element.
| if (Children.filter(Child => | |
| parseFloat(getComputedStyle(Child).getPropertyValue('padding-top')) >= 5 && | |
| parseFloat(getComputedStyle(Child).getPropertyValue('border-bottom-width')) >= 0.1 | |
| ).length === 1) return true | |
| // HTMLTableElement | |
| return Children.filter(Child => (Child instanceof HTMLTableElement || Child instanceof HTMLTableCellElement) && | |
| parseFloat(getComputedStyle(Child).getPropertyValue('padding-top')) >= 5 && parseFloat(getComputedStyle(Child).getPropertyValue('padding-bottom')) >= 5).length >= 2 | |
| }) | |
| Targeted = Targeted.filter(Ele => { | |
| let Children = [...Ele.querySelectorAll('*')].filter(Child => Child instanceof HTMLElement) | |
| const MatchesStructure = Children.filter(Child => | |
| parseFloat(getComputedStyle(Child).getPropertyValue('padding-top')) >= 5 && | |
| parseFloat(getComputedStyle(Child).getPropertyValue('border-bottom-width')) >= 0.1 | |
| ).length === 1 || | |
| // HTMLTableElement | |
| Children.filter(Child => (Child instanceof HTMLTableElement || Child instanceof HTMLTableCellElement) && | |
| parseFloat(getComputedStyle(Child).getPropertyValue('padding-top')) >= 5 && parseFloat(getComputedStyle(Child).getPropertyValue('padding-bottom')) >= 5).length >= 2 | |
| if (!MatchesStructure) return false |
| // non-HTMLTableElement | ||
| if (Children.filter(Child => | ||
| parseFloat(getComputedStyle(Child).getPropertyValue('padding-top')) >= 5 && | ||
| parseFloat(getComputedStyle(Child).getPropertyValue('border-bottom-width')) >= 0.1 | ||
| ).length === 1) return true |
There was a problem hiding this comment.
The comment // non-HTMLTableElement is misleading here: the predicate does not check that the child is not a table element, so table-related DOM could still satisfy this branch. Please either enforce the non-table condition or reword the comment to match the actual logic.
| let RealTabletTargeted = Targeted.filter(Ele => { | ||
| if (!(Ele instanceof HTMLElement) || !(Ele instanceof HTMLTableElement)) return false | ||
| let Children = [...Ele.querySelectorAll('*')].filter(Child => Child instanceof HTMLElement) | ||
| return Children.some(Child => parseFloat(getComputedStyle(Child).getPropertyValue('padding-top')) >= 5 && parseFloat(getComputedStyle(Child).getPropertyValue('padding-bottom')) >= 5) | ||
| }) | ||
| console.debug(`[${UserscriptName}] vue:settled RealTabletTargeted`, RealTabletTargeted) | ||
| RealTabletTargeted.forEach(Ele => { |
There was a problem hiding this comment.
Variable name RealTabletTargeted looks like a typo given the surrounding table-related logic (HTMLTableElement). Consider renaming to RealTableTargeted (or similar) to avoid confusion and to keep log messages consistent.
| let RealTabletTargeted = Targeted.filter(Ele => { | |
| if (!(Ele instanceof HTMLElement) || !(Ele instanceof HTMLTableElement)) return false | |
| let Children = [...Ele.querySelectorAll('*')].filter(Child => Child instanceof HTMLElement) | |
| return Children.some(Child => parseFloat(getComputedStyle(Child).getPropertyValue('padding-top')) >= 5 && parseFloat(getComputedStyle(Child).getPropertyValue('padding-bottom')) >= 5) | |
| }) | |
| console.debug(`[${UserscriptName}] vue:settled RealTabletTargeted`, RealTabletTargeted) | |
| RealTabletTargeted.forEach(Ele => { | |
| let RealTableTargeted = Targeted.filter(Ele => { | |
| if (!(Ele instanceof HTMLElement) || !(Ele instanceof HTMLTableElement)) return false | |
| let Children = [...Ele.querySelectorAll('*')].filter(Child => Child instanceof HTMLElement) | |
| return Children.some(Child => parseFloat(getComputedStyle(Child).getPropertyValue('padding-top')) >= 5 && parseFloat(getComputedStyle(Child).getPropertyValue('padding-bottom')) >= 5) | |
| }) | |
| console.debug(`[${UserscriptName}] vue:settled RealTableTargeted`, RealTableTargeted) | |
| RealTableTargeted.forEach(Ele => { |
| async function ExecuteOCR(Targeted: HTMLElement[]) { | ||
| const NextTargeted = [] |
There was a problem hiding this comment.
ExecuteOCR returns an untyped empty array (const NextTargeted = []), so TypeScript infers any[] and you lose type-safety for Targeted after await ExecuteOCR(Targeted). Please type NextTargeted (e.g., HTMLElement[]) and/or add an explicit return type like Promise<HTMLElement[]>.
| async function ExecuteOCR(Targeted: HTMLElement[]) { | |
| const NextTargeted = [] | |
| async function ExecuteOCR(Targeted: HTMLElement[]): Promise<HTMLElement[]> { | |
| const NextTargeted: HTMLElement[] = [] |
| } | ||
|
|
||
| ArticleHTMLElement.addEventListener('vue:settled', async () => { | ||
| let Targeted = [...document.querySelectorAll('#app div[class] div[class] ~ div[class]')].filter(Ele => Ele instanceof HTMLElement) |
There was a problem hiding this comment.
This handler uses the global document for queries, while the rest of the userscript consistently uses the injected BrowserWindow.document (e.g., WaitForElement('#app', BrowserWindow.document) at line 47 and DOM creation at lines 132–135). For consistency and to avoid userscript sandbox/unsafeWindow mismatches, prefer querying via BrowserWindow.document (or ArticleHTMLElement.ownerDocument).
| let Targeted = [...document.querySelectorAll('#app div[class] div[class] ~ div[class]')].filter(Ele => Ele instanceof HTMLElement) | |
| let Targeted = [...BrowserWindow.document.querySelectorAll('#app div[class] div[class] ~ div[class]')].filter(Ele => Ele instanceof HTMLElement) |
| return parseFloat(getComputedStyle(Child).getPropertyValue('margin-bottom')) >= 10 && parseFloat(getComputedStyle(Child).getPropertyValue('padding-bottom')) >= 1 && parseFloat(getComputedStyle(Child).getPropertyValue('padding-top')) >= 1 && | ||
| parseFloat(getComputedStyle(Child).getPropertyValue('border-top-width')) >= 0.25 && parseFloat(getComputedStyle(Child).getPropertyValue('border-bottom-width')) >= 0.25 |
There was a problem hiding this comment.
Inside this some(...) predicate, getComputedStyle(Child) is effectively invoked multiple times per element (once per property). Consider caching const Style = getComputedStyle(Child) and reading all needed properties from it to reduce repeated style lookups.
| return parseFloat(getComputedStyle(Child).getPropertyValue('margin-bottom')) >= 10 && parseFloat(getComputedStyle(Child).getPropertyValue('padding-bottom')) >= 1 && parseFloat(getComputedStyle(Child).getPropertyValue('padding-top')) >= 1 && | |
| parseFloat(getComputedStyle(Child).getPropertyValue('border-top-width')) >= 0.25 && parseFloat(getComputedStyle(Child).getPropertyValue('border-bottom-width')) >= 0.25 | |
| const Style = getComputedStyle(Child) | |
| return parseFloat(Style.getPropertyValue('margin-bottom')) >= 10 && parseFloat(Style.getPropertyValue('padding-bottom')) >= 1 && parseFloat(Style.getPropertyValue('padding-top')) >= 1 && | |
| parseFloat(Style.getPropertyValue('border-top-width')) >= 0.25 && parseFloat(Style.getPropertyValue('border-bottom-width')) >= 0.25 |
No description provided.