-
-
Notifications
You must be signed in to change notification settings - Fork 58
Description
Bug Description
In popup/visuals.js, there is an off-by-one error in the link highlight removal function. On line 20, the code loops through the links array instead of the removeLinkHighlight array:
const removeLinkHighlight = document.getElementsByClassName("remove-link-hg");
for (let i = 0; i < links.length; i++) { // BUG: Should be removeLinkHighlight.length
removeLinkHighlight[i].addEventListener('click', function (e) {Impact
This causes only the first N elements to have the event listener attached (where N is the length of the links array). If there are more remove buttons than link buttons, the extra remove buttons won't have the event listener. Conversely, if there are fewer remove buttons, this could cause an index error.
Fix
Change line 20 from:
for (let i = 0; i < links.length; i++) {To:
for (let i = 0; i < removeLinkHighlight.length; i++) {Additional Improvements Identified
-
Error handling in speech.js: The
fetchMeaning()function doesn't handle cases where the API response doesn't have the expected structure. Should add validation forjson[0].meanings[0].definitions[0]. -
Memory leaks in zoom functionality: The zoom event listeners in
visuals.jsare attached every time, but they should be debounced or checked to prevent multiple listeners on the same element. -
Race conditions in extension permissions: The manifest.json requests broad permissions (
<all_urls>). Consider limiting to specific domains or implementing dynamic permission requests. -
Missing null checks: Multiple files don't check if DOM elements exist before adding event listeners, which could throw errors on certain pages.