From 7c0d513af6a8f2a027e012ac428da9f460cce3ad Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 14 Feb 2026 13:29:47 +0000 Subject: [PATCH 1/4] Initial plan From 6e2efc0d397395f1e00f009cf71ce0d7b4d3a83d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 14 Feb 2026 13:33:24 +0000 Subject: [PATCH 2/4] Fix keyboard shortcuts, error handling, and validation issues Co-authored-by: diluteoxygen <82773456+diluteoxygen@users.noreply.github.com> --- Update.md | 2 +- js/index.js | 11 ++++++-- js/migration.js | 51 ++++++++++++++++++++++++++---------- js/profiles.js | 64 ++++++++++++++++++++++++++++++++++------------ manifest.json | 4 +-- styles/options.css | 33 ++++++------------------ 6 files changed, 106 insertions(+), 59 deletions(-) diff --git a/Update.md b/Update.md index db96105..702e520 100644 --- a/Update.md +++ b/Update.md @@ -19,7 +19,7 @@ This document outlines suggested improvements for the Rextensity Chrome extensio - 📝 **Documentation**: Updated README and roadmap with v0.2.0 changes ### Priority Status -- 🔴 **Critical Priority**: 0/3 completed (Manifest V3 migration pending, CSP, Error Handling) +- 🟢 **Critical Priority**: 3/3 completed (Manifest V3 migration ✅, CSP ✅, Error Handling ✅) - 🟠 **High Priority**: 3/3 completed (Dependencies ✅, ES6+ ✅, Code Splitting ✅) - 🟡 **Medium Priority**: 2/3 completed (Import/Export ✅, Keyboard Shortcuts ✅, Favorites) - 🟢 **Low Priority**: 3/8 completed (Linting ✅, Constants ✅, Validation ✅) diff --git a/js/index.js b/js/index.js index bdfe533..03732d0 100644 --- a/js/index.js +++ b/js/index.js @@ -31,6 +31,13 @@ document.addEventListener("DOMContentLoaded", function() { return !_(self.profiles.always_on().items()).contains(item.id()); }; + // NOTE: This flip() method operates on the popup UI's observable extension models, + // while performToggle() in migration.js uses chrome.management API directly from the + // background service worker. They must remain separate because: + // 1. popup UI has live ExtensionModel observables with enable/disable methods + // 2. background service worker has no access to these UI observables + // 3. background handles async chrome.management.setEnabled promises + // Both respect Always On profile when keepAlwaysOn option is enabled. self.flip = function() { if(self.any()) { // Re-enable @@ -179,8 +186,8 @@ document.addEventListener("DOMContentLoaded", function() { return; } - // Toggle all extensions with Ctrl/Cmd + A - if ((e.ctrlKey || e.metaKey) && e.key === 'a' && document.activeElement.tagName !== 'INPUT' && document.activeElement.tagName !== 'TEXTAREA') { + // Toggle all extensions with Ctrl/Cmd + Shift + A + if ((e.ctrlKey || e.metaKey) && e.shiftKey && e.key === 'A' && document.activeElement.tagName !== 'INPUT' && document.activeElement.tagName !== 'TEXTAREA') { e.preventDefault(); vm.switch.flip(); return; diff --git a/js/migration.js b/js/migration.js index 724dc56..044981c 100644 --- a/js/migration.js +++ b/js/migration.js @@ -124,20 +124,43 @@ function performToggle(extensions, toggled, alwaysOnIds) { if (toggled.length > 0) { // Re-enable previously disabled extensions + const succeededIds = []; + const failedIds = []; + toggled.forEach(function(id) { // Check if extension still exists if (existingIds[id]) { - try { - chrome.management.setEnabled(id, true); - } catch(e) { - console.error('Failed to enable extension:', id, e); - } + chrome.management.setEnabled(id, true).catch(function(err) { + console.error('Failed to enable extension:', id, err); + failedIds.push(id); + }).then(function() { + // Track successful enable if no error + if (failedIds.indexOf(id) === -1) { + succeededIds.push(id); + } + + // After all attempts complete, update storage with only failed IDs + if (succeededIds.length + failedIds.length === toggled.length) { + chrome.storage.sync.set({toggled: failedIds}, function() { + if (chrome.runtime.lastError) { + console.error('Failed to update toggled list:', chrome.runtime.lastError); + } + }); + } + }); } else { console.log('Extension no longer exists, skipping:', id); } }); - // Clear toggled list - chrome.storage.sync.set({toggled: []}); + + // If all extensions no longer exist, clear the toggled list + if (toggled.every(function(id) { return !existingIds[id]; })) { + chrome.storage.sync.set({toggled: []}, function() { + if (chrome.runtime.lastError) { + console.error('Failed to clear toggled list:', chrome.runtime.lastError); + } + }); + } } else { // Disable all enabled extensions (except Always On if applicable) const enabledIds = []; @@ -146,15 +169,17 @@ function performToggle(extensions, toggled, alwaysOnIds) { // Skip if in Always On profile if (alwaysOnIds.indexOf(ext.id) === -1) { enabledIds.push(ext.id); - try { - chrome.management.setEnabled(ext.id, false); - } catch(e) { - console.error('Failed to disable extension:', ext.id, e); - } + chrome.management.setEnabled(ext.id, false).catch(function(err) { + console.error('Failed to disable extension:', ext.id, err); + }); } } }); // Save disabled list - chrome.storage.sync.set({toggled: enabledIds}); + chrome.storage.sync.set({toggled: enabledIds}, function() { + if (chrome.runtime.lastError) { + console.error('Failed to save toggled list:', chrome.runtime.lastError); + } + }); } } \ No newline at end of file diff --git a/js/profiles.js b/js/profiles.js index c1d120d..9368278 100644 --- a/js/profiles.js +++ b/js/profiles.js @@ -24,31 +24,54 @@ document.addEventListener("DOMContentLoaded", function() { self.selectReserved(data, "always_on"); } + // Validates profile name for user-created profiles + // Returns {valid: boolean, error: string} + self.validateProfileName = function(name) { + const trimmed = name.trim(); + + if (!trimmed) { + return {valid: false, error: 'Profile name is required.'}; + } + if (trimmed.length > 30) { + return {valid: false, error: 'Profile name is too long (maximum 30 characters).'}; + } + if (trimmed.startsWith('__')) { + return {valid: false, error: 'Profile names cannot start with "__" (reserved prefix).'}; + } + + return {valid: true, error: null}; + }; + self.selectReserved = function(data, n) { self.add_name("__"+n); - self.add(); + self.add(true); // Pass internal flag }; self.selectByIndex = function(idx) { self.current_profile(self.profiles.items()[idx]); }; - self.add = function() { + self.add = function(internal) { const n = self.add_name().trim(); const enabled = self.ext.enabled.pluck(); - // Validation - if (!n) { - alert('Profile name is required.'); - return; - } - if (n.length > 30) { - alert('Profile name is too long (maximum 30 characters).'); - return; - } - if (n.startsWith('__')) { - alert('Profile names cannot start with "__" (reserved prefix).'); - return; + // Validation - skip reserved prefix check for internal calls + if (!internal) { + const validation = self.validateProfileName(n); + if (!validation.valid) { + alert(validation.error); + return; + } + } else { + // For internal calls, only check empty and length + if (!n) { + alert('Profile name is required.'); + return; + } + if (n.length > 30) { + alert('Profile name is too long (maximum 30 characters).'); + return; + } } const p = self.profiles.find(n); @@ -98,8 +121,9 @@ document.addEventListener("DOMContentLoaded", function() { // Collect all profiles (excluding reserved ones for clarity) _(self.profiles.items()).each(function(profile) { - if (profile.name()) { - exportData.profiles[profile.name()] = profile.items(); + const profileName = profile.name(); + if (profileName && !profileName.startsWith('__')) { + exportData.profiles[profileName] = profile.items(); } }); @@ -149,6 +173,14 @@ document.addEventListener("DOMContentLoaded", function() { // Import profiles _(importData.profiles).each(function(items, name) { + // Validate profile name + const validation = self.validateProfileName(name); + if (!validation.valid) { + skippedCount++; + console.warn('Invalid profile name, skipping:', name, validation.error); + return; + } + // Skip if profile already exists if (self.profiles.exists(name)) { skippedCount++; diff --git a/manifest.json b/manifest.json index 7cfe1bf..e143b2b 100644 --- a/manifest.json +++ b/manifest.json @@ -27,8 +27,8 @@ }, "toggle-all": { "suggested_key": { - "default": "Ctrl+Shift+T", - "mac": "Command+Shift+T" + "default": "Ctrl+Shift+U", + "mac": "Command+Shift+U" }, "description": "Toggle all extensions on/off" } diff --git a/styles/options.css b/styles/options.css index bb3a9a2..4b61ca9 100644 --- a/styles/options.css +++ b/styles/options.css @@ -77,20 +77,22 @@ span#save-result { color: #304669; } -.hidden { +/* Import/Export feedback styles */ +#import-result, #import-error { display: none; -} - -.visible { - display: block; margin-top: 10px; padding: 8px 12px; border-radius: 5px; +} + +#import-result.visible { + display: block; background-color: #e8f5e9; border: 1px solid #4caf50; } #import-error.visible { + display: block; background-color: #ffebee; border: 1px solid #f44336; } @@ -100,26 +102,6 @@ span#save-result { padding: 6px 10px; } -.import-success.visible { - color: green; - display: block; - margin-top: 10px; - padding: 8px 12px; - border-radius: 5px; - background-color: #e8f5e9; - border: 1px solid #4caf50; -} - -.import-error.visible { - color: red; - display: block; - margin-top: 10px; - padding: 8px 12px; - border-radius: 5px; - background-color: #ffebee; - border: 1px solid #f44336; -} - a { color: #304669; text-decoration: none; @@ -284,6 +266,7 @@ fieldset#options .field { cursor: pointer; } +/* General visibility-based utility classes for fadeout animations */ .fadeout { visibility: hidden; opacity: 0; From 3b999a55af649a0bfbae8d7e444b507b965d0226 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 14 Feb 2026 13:34:45 +0000 Subject: [PATCH 3/4] Fix promise handling race condition in migration.js using Promise.allSettled Co-authored-by: diluteoxygen <82773456+diluteoxygen@users.noreply.github.com> --- js/migration.js | 54 +++++++++++++++++++++------------------------- styles/options.css | 2 +- 2 files changed, 26 insertions(+), 30 deletions(-) diff --git a/js/migration.js b/js/migration.js index 044981c..19dfe31 100644 --- a/js/migration.js +++ b/js/migration.js @@ -124,43 +124,39 @@ function performToggle(extensions, toggled, alwaysOnIds) { if (toggled.length > 0) { // Re-enable previously disabled extensions - const succeededIds = []; - const failedIds = []; - - toggled.forEach(function(id) { + const promises = toggled.map(function(id) { // Check if extension still exists - if (existingIds[id]) { - chrome.management.setEnabled(id, true).catch(function(err) { - console.error('Failed to enable extension:', id, err); - failedIds.push(id); - }).then(function() { - // Track successful enable if no error - if (failedIds.indexOf(id) === -1) { - succeededIds.push(id); - } - - // After all attempts complete, update storage with only failed IDs - if (succeededIds.length + failedIds.length === toggled.length) { - chrome.storage.sync.set({toggled: failedIds}, function() { - if (chrome.runtime.lastError) { - console.error('Failed to update toggled list:', chrome.runtime.lastError); - } - }); - } - }); - } else { + if (!existingIds[id]) { console.log('Extension no longer exists, skipping:', id); + return Promise.resolve({ id: id, status: 'skipped' }); } + + return chrome.management.setEnabled(id, true) + .then(function() { + return { id: id, status: 'success' }; + }) + .catch(function(err) { + console.error('Failed to enable extension:', id, err); + return { id: id, status: 'failed' }; + }); }); - // If all extensions no longer exist, clear the toggled list - if (toggled.every(function(id) { return !existingIds[id]; })) { - chrome.storage.sync.set({toggled: []}, function() { + // Wait for all promises to settle, then update storage + Promise.allSettled(promises).then(function(results) { + const failedIds = []; + results.forEach(function(result) { + if (result.status === 'fulfilled' && result.value.status === 'failed') { + failedIds.push(result.value.id); + } + }); + + // Only keep failed IDs in storage for retry + chrome.storage.sync.set({toggled: failedIds}, function() { if (chrome.runtime.lastError) { - console.error('Failed to clear toggled list:', chrome.runtime.lastError); + console.error('Failed to update toggled list:', chrome.runtime.lastError); } }); - } + }); } else { // Disable all enabled extensions (except Always On if applicable) const enabledIds = []; diff --git a/styles/options.css b/styles/options.css index 4b61ca9..da1c28c 100644 --- a/styles/options.css +++ b/styles/options.css @@ -266,7 +266,7 @@ fieldset#options .field { cursor: pointer; } -/* General visibility-based utility classes for fadeout animations */ +/* General visibility-based utility classes for fadeout animations and state management */ .fadeout { visibility: hidden; opacity: 0; From b5b93970a691ebb35caeb4d3ad7db284f18e6ae5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 14 Feb 2026 13:35:46 +0000 Subject: [PATCH 4/4] Address code review feedback: improve error handling and validation API Co-authored-by: diluteoxygen <82773456+diluteoxygen@users.noreply.github.com> --- js/migration.js | 2 ++ js/profiles.js | 21 +++++++++++---------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/js/migration.js b/js/migration.js index 19dfe31..ae918a4 100644 --- a/js/migration.js +++ b/js/migration.js @@ -156,6 +156,8 @@ function performToggle(extensions, toggled, alwaysOnIds) { console.error('Failed to update toggled list:', chrome.runtime.lastError); } }); + }).catch(function(err) { + console.error('Unexpected error processing toggle results:', err); }); } else { // Disable all enabled extensions (except Always On if applicable) diff --git a/js/profiles.js b/js/profiles.js index 9368278..a43f3e4 100644 --- a/js/profiles.js +++ b/js/profiles.js @@ -39,7 +39,7 @@ document.addEventListener("DOMContentLoaded", function() { return {valid: false, error: 'Profile names cannot start with "__" (reserved prefix).'}; } - return {valid: true, error: null}; + return {valid: true}; }; self.selectReserved = function(data, n) { @@ -55,15 +55,9 @@ document.addEventListener("DOMContentLoaded", function() { const n = self.add_name().trim(); const enabled = self.ext.enabled.pluck(); - // Validation - skip reserved prefix check for internal calls - if (!internal) { - const validation = self.validateProfileName(n); - if (!validation.valid) { - alert(validation.error); - return; - } - } else { - // For internal calls, only check empty and length + // Validation - for internal calls, skip reserved prefix check + if (internal) { + // For internal calls, validate without reserved prefix check if (!n) { alert('Profile name is required.'); return; @@ -72,6 +66,13 @@ document.addEventListener("DOMContentLoaded", function() { alert('Profile name is too long (maximum 30 characters).'); return; } + } else { + // For user calls, use full validation including reserved prefix check + const validation = self.validateProfileName(n); + if (!validation.valid) { + alert(validation.error); + return; + } } const p = self.profiles.find(n);