Conversation
Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
Fix getRootDomain incorrectly truncating IP address literals
Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
Mask query parameters in redirect URL warning logs
…round.js Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
…ols-interception Exclude internal browser protocols from webRequest interception
Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
…main-deletion Fix keyboard focus loss when last whitelisted domain is deleted
…+ type guard) Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
…orage Normalize domains at read time in getStoredDomains
There was a problem hiding this comment.
Pull request overview
Migrates changes from the 1.0.3 branch into main for the extension’s domain-handling and request interception logic, improving IP-literal handling, storage normalization, and request filtering/logging behavior.
Changes:
- Centralize IPv4/IPv6 detection into
utils.jsand treat IP literals as “root domains” ingetRootDomain(). - Normalize stored whitelist domains in the popup (trim + lowercase) when reading from
storage.sync. - Harden background webRequest handlers by skipping non-http(s) URLs and sanitizing redirect URL logging on parse failures.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| utils.js | Adds IPv4/IPv6 regexes and returns IP literals unchanged from getRootDomain(). |
| popup.js | Normalizes stored domains on read; adjusts focus behavior when removing the last list item. |
| background.js | Skips non-http(s) URLs in webRequest listeners; logs a safer redirect URL on parsing errors; removes duplicated IP regex definitions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return Array.isArray(stored[STORAGE_KEY]) | ||
| ? stored[STORAGE_KEY] | ||
| .filter((domain) => typeof domain === "string") | ||
| .map((domain) => domain.trim().toLowerCase()) | ||
| : []; |
There was a problem hiding this comment.
getStoredDomains() normalizes entries (trim/lowercase) but can introduce empty-string domains (e.g., stored value is whitespace) and can also create duplicates when legacy values differ only by case/spacing (e.g., "Example.com" + " example.com "). Consider filtering out empty results after trimming and de-duplicating the normalized list before returning.
| return Array.isArray(stored[STORAGE_KEY]) | |
| ? stored[STORAGE_KEY] | |
| .filter((domain) => typeof domain === "string") | |
| .map((domain) => domain.trim().toLowerCase()) | |
| : []; | |
| if (!Array.isArray(stored[STORAGE_KEY])) { | |
| return []; | |
| } | |
| const normalizedDomains = stored[STORAGE_KEY] | |
| .filter((domain) => typeof domain === "string") | |
| .map((domain) => domain.trim().toLowerCase()) | |
| .filter((domain) => domain.length > 0); | |
| return [...new Set(normalizedDomains)]; |
| emptyItem.className = "empty-state"; | ||
| emptyItem.textContent = "Aucun domaine en liste blanche"; | ||
| list.appendChild(emptyItem); | ||
| document.getElementById("add-domain").focus(); |
There was a problem hiding this comment.
This focuses #add-domain after removing the last item, but that button can be disabled when no current domain is detected (see initPopup). Focusing a disabled control is a no-op and can leave keyboard focus in an unexpected place; consider focusing an enabled control instead (e.g., #clear-domains if enabled, or the list container) or conditionally focusing only when the button is enabled.
| document.getElementById("add-domain").focus(); | |
| const addDomainButton = document.getElementById("add-domain"); | |
| const clearDomainsButton = document.getElementById("clear-domains"); | |
| if (addDomainButton && !addDomainButton.disabled) { | |
| addDomainButton.focus(); | |
| } else if (clearDomainsButton && !clearDomainsButton.disabled) { | |
| clearDomainsButton.focus(); | |
| } else if (typeof list.focus === "function") { | |
| list.focus(); | |
| } |
| const IPV4_REGEX = /^(25[0-5]|2[0-4]\d|1?\d?\d)(\.(25[0-5]|2[0-4]\d|1?\d?\d)){3}$/; | ||
| const IPV6_REGEX = | ||
| /^\[?(?:(?:[a-f0-9]{1,4}:){7}[a-f0-9]{1,4}|(?:[a-f0-9]{1,4}:){1,7}:|(?:[a-f0-9]{1,4}:){1,6}:[a-f0-9]{1,4}|(?:[a-f0-9]{1,4}:){1,5}(?::[a-f0-9]{1,4}){1,2}|(?:[a-f0-9]{1,4}:){1,4}(?::[a-f0-9]{1,4}){1,3}|(?:[a-f0-9]{1,4}:){1,3}(?::[a-f0-9]{1,4}){1,4}|(?:[a-f0-9]{1,4}:){1,2}(?::[a-f0-9]{1,4}){1,5}|[a-f0-9]{1,4}:(?:(?::[a-f0-9]{1,4}){1,6})|:(?:(?::[a-f0-9]{1,4}){1,7}|:))\]?$/i; |
There was a problem hiding this comment.
These new top-level regex constants become implicit cross-script globals (and are referenced from other scripts). To reduce brittle coupling/leaking globals, consider encapsulating IP detection in a shared helper (e.g., isIpLiteral(hostname)) and only exposing that helper, rather than relying on other files to access IPV4_REGEX/IPV6_REGEX bindings directly.
No description provided.