Conversation
Replaces direct localStorage usage with a centralized Storage wrapper (scripts/storage.js) that uses chrome.storage.sync on Chrome and browser.storage.local on Firefox. Key changes: - Add scripts/storage.js: non-defer HEAD script providing window.Storage and window.storageReady (Promise). Pre-populates an in-memory cache synchronously from localStorage for fast reads by defer scripts, then loads chrome.storage asynchronously. - Perform one-time migration of existing user data from localStorage to chrome.storage on first load, keyed by _storage_migrated flag. - Keep local-only keys in localStorage (quotes_*, LoadingScreenColor, weatherParsed*) to respect Chrome sync quotas and avoid redundant data. - Update theme.js to await window.storageReady before applying theme, preventing flash of wrong color on startup. - Update backup-restore.js to include chromeStorage in backup; supports restoring both old (localStorage-only) and new backup formats. - Add "storage" permission to manifest.json and manifest(firefox).json. - Replace all localStorage.getItem/setItem calls across 16 scripts with Storage.getItem/setItem. Closes #<issue-number>
Co-authored-by: Basavaraj Patil <patil.ba@northeastern.edu>
📝 WalkthroughWalkthroughThis PR migrates the extension from Changes
Sequence DiagramsequenceDiagram
participant Page as Page Load
participant StorageJS as scripts/storage.js
participant LocalStorage as localStorage
participant ExtStorage as chrome.storage.sync/local
participant Theme as scripts/theme.js
participant UI as UI Initialization
Page->>StorageJS: Load (before theme scripts)
StorageJS->>StorageJS: Check for chrome.storage availability
StorageJS->>LocalStorage: Pre-populate cache from existing data
StorageJS->>ExtStorage: Async: Load extension storage
StorageJS->>ExtStorage: Check if migration needed (_storage_migrated)
alt First Load (Migration)
StorageJS->>LocalStorage: Read non-local-only keys
StorageJS->>ExtStorage: Write migrated data
StorageJS->>LocalStorage: Delete migrated keys
StorageJS->>ExtStorage: Set _storage_migrated marker
end
StorageJS->>StorageJS: Resolve window.storageReady Promise
Theme->>StorageJS: await window.storageReady
Theme->>StorageJS: Storage.getItem(preferredTheme)
StorageJS-->>Theme: Return theme value from cache
Theme->>UI: Apply theme before render
UI->>UI: Initialize with cached values
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can suggest fixes for GitHub Check annotations.Configure the |
There was a problem hiding this comment.
Pull request overview
This PR migrates the extension’s settings persistence from scattered localStorage usage to a centralized scripts/storage.js wrapper backed by chrome.storage.sync (Chrome) or browser.storage.local (Firefox), including one-time migration and backup/restore compatibility.
Changes:
- Added
scripts/storage.jspluswindow.storageReadyto provide a unified storage API with migration + caching. - Updated multiple feature scripts to use
Storage.getItem/setItem/removeItem(and theme startup to awaitstorageReady). - Extended backup/restore format to include Storage API data and added
storagepermission to manifests.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/widgets-transparency.js | Switches bg opacity persistence to the Storage wrapper. |
| scripts/weather.js | Migrates weather settings/cache reads/writes to Storage wrapper. |
| scripts/voice-search.js | Migrates mic visibility preference to Storage wrapper. |
| scripts/todo-list.js | Migrates todo list persistence to Storage wrapper. |
| scripts/theme.js | Awaits storageReady and migrates theme persistence to Storage wrapper. |
| scripts/storage.js | Introduces the centralized Storage wrapper + migration + cache. |
| scripts/shortcuts.js | Migrates shortcuts persistence to Storage wrapper. |
| scripts/search.js | Migrates search engine/settings persistence to Storage wrapper. |
| scripts/search-suggestions.js | Migrates proxy setting persistence to Storage wrapper. |
| scripts/script.js | Migrates tips/toast persistence to Storage wrapper. |
| scripts/save-load-states.js | Migrates checkbox/display/active state helpers to Storage wrapper. |
| scripts/menu-shortcut-page.js | Migrates language read to Storage wrapper. |
| scripts/languages.js | Migrates language + userText checks to Storage wrapper. |
| scripts/custom-text.js | Migrates custom text visibility/content persistence to Storage wrapper. |
| scripts/clock.js | Migrates clock-related preferences to Storage wrapper. |
| scripts/bookmarks.js | Migrates bookmark sorting preference to Storage wrapper. |
| scripts/backup-restore.js | Backs up/restores both local-only localStorage keys and Storage API settings. |
| manifest.json | Adds storage permission for Chrome. |
| manifest(firefox).json | Adds storage permission for Firefox. |
| index.html | Loads scripts/storage.js early (non-defer) before defer scripts. |
| CHANGELOG.md | Documents the migration, sync behavior, and theme flash fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Check if there's a saved state in Storage | ||
| const savedState = Storage.getItem("micIconVisible"); | ||
| let isMicIconVisible; | ||
|
|
||
| // If saved state exists, use it; otherwise, fallback to initial state based on browser support | ||
| if (savedState !== null) { | ||
| isMicIconVisible = savedState === "true"; | ||
| } else { | ||
| // Default state: Hide mic icon if browser is not supported | ||
| isMicIconVisible = isSupportedBrowser(); | ||
| // Save the initial state based on the user agent | ||
| localStorage.setItem("micIconVisible", isMicIconVisible); | ||
| Storage.setItem("micIconVisible", isMicIconVisible); | ||
| } |
There was a problem hiding this comment.
micIconVisible is parsed as a string (savedState === "true"), but it is initially persisted and later updated as a boolean via Storage.setItem. After the first write, Storage.getItem will return a boolean and this string comparison will fail, flipping the default visibility logic. Store string values consistently or treat the stored value as boolean (savedState === true / coercion).
| // Also clear local-only localStorage data | ||
| const localKeys = Object.keys(localStorage).filter(k => | ||
| !k.startsWith("quotes_") && k !== "LoadingScreenColor" && | ||
| k !== "weatherParsedData" && k !== "weatherParsedTime" && | ||
| k !== "weatherParsedLocation" && k !== "weatherParsedLang" |
There was a problem hiding this comment.
The reset handler intends to clear “local-only” keys, but the filter removes the opposite set (it keeps quotes_*, LoadingScreenColor, and weatherParsed*). This changes behavior from the previous localStorage.clear() and may leave stale quote/weather data behind after a reset. Either invert the filter to remove only local-only keys, or adjust the comment/behavior intentionally.
| // Also clear local-only localStorage data | |
| const localKeys = Object.keys(localStorage).filter(k => | |
| !k.startsWith("quotes_") && k !== "LoadingScreenColor" && | |
| k !== "weatherParsedData" && k !== "weatherParsedTime" && | |
| k !== "weatherParsedLocation" && k !== "weatherParsedLang" | |
| // Also clear local-only localStorage data (quotes_*, LoadingScreenColor, weatherParsed*) | |
| const localKeys = Object.keys(localStorage).filter(k => | |
| k.startsWith("quotes_") || k === "LoadingScreenColor" || | |
| k === "weatherParsedData" || k === "weatherParsedTime" || | |
| k === "weatherParsedLocation" || k === "weatherParsedLang" |
| // --- Restore local-only keys (quotes, weather cache, etc.) --- | ||
| if (backup.localStorage) { | ||
| Object.keys(backup.localStorage).forEach(key => { | ||
| localStorage.setItem(key, backup.localStorage[key]); | ||
| // Restore only keys that belong in localStorage | ||
| // (local-only keys: quotes_*, LoadingScreenColor, weatherParsed*) | ||
| if ( | ||
| key.startsWith("quotes_") || | ||
| key === "LoadingScreenColor" || | ||
| key === "weatherParsedData" || | ||
| key === "weatherParsedTime" || | ||
| key === "weatherParsedLocation" || | ||
| key === "weatherParsedLang" | ||
| ) { |
There was a problem hiding this comment.
The “local-only key” allowlist is duplicated here and in scripts/storage.js (_isLocalOnly). Having two separate lists increases the risk they diverge (e.g., a new local-only key gets added in one place but not the other). Consider centralizing this logic in the Storage wrapper (e.g., exposing Storage.isLocalOnly(key) or a shared constant) so backup/restore always matches runtime routing.
|
|
||
| // Retrieve saved state | ||
| const isHidden = localStorage.getItem("hideWeatherVisible") === "true"; | ||
| const isHidden = Storage.getItem("hideWeatherVisible") === "true"; |
There was a problem hiding this comment.
hideWeatherVisible is read as a string comparison (=== "true"), but the change handler stores a boolean via Storage.setItem("hideWeatherVisible", hidden). Once saved, Storage.getItem will return a boolean and this comparison will fail, causing the hidden state to be lost on reload. Store string values consistently or change the read to handle booleans.
| const isHidden = Storage.getItem("hideWeatherVisible") === "true"; | |
| const storedHideWeather = Storage.getItem("hideWeatherVisible"); | |
| const isHidden = storedHideWeather === true || storedHideWeather === "true"; |
| const minMaxTempCheckbox = document.getElementById("minMaxTempCheckbox"); | ||
| const isMinMaxEnabled = localStorage.getItem("minMaxTempEnabled") === "true"; | ||
| const isMinMaxEnabled = Storage.getItem("minMaxTempEnabled") === "true"; | ||
| minMaxTempCheckbox.checked = isMinMaxEnabled; |
There was a problem hiding this comment.
minMaxTempEnabled is read using === "true", but later the checkbox handler saves a boolean (Storage.setItem("minMaxTempEnabled", isChecked)). After the first toggle, the stored value becomes a boolean and the read will no longer work correctly. Use consistent string storage or update reads to accept booleans.
| // Retrieve saved state from Storage | ||
| const isClockHidden = Storage.getItem("hideClockVisible") === "true"; | ||
| hideClockCheckbox.checked = isClockHidden; | ||
|
|
||
| // Apply initial state | ||
| applyClockState(isClockHidden); | ||
| toggleHideState(isClockHidden); | ||
|
|
||
| if (!isClockHidden) { | ||
| initializeClock(); | ||
| } | ||
|
|
||
| hideClockCheckbox.addEventListener("change", function () { | ||
| const isChecked = hideClockCheckbox.checked; | ||
| localStorage.setItem("hideClockVisible", isChecked); | ||
| Storage.setItem("hideClockVisible", isChecked); | ||
| applyClockState(isChecked); |
There was a problem hiding this comment.
hideClockVisible is read with a string comparison (=== "true"), but the change handler saves a boolean (Storage.setItem("hideClockVisible", isChecked)). After the first toggle, the stored value becomes a boolean and the clock visibility state will not restore correctly on reload. Store string values or update the read to handle booleans.
| // Load and apply the checkbox state | ||
| const isUserTextVisible = localStorage.getItem("userTextVisible") !== "false"; | ||
| const isUserTextVisible = Storage.getItem("userTextVisible") !== "false"; | ||
| userTextCheckbox.checked = isUserTextVisible; | ||
| userTextDiv.style.display = isUserTextVisible ? "block" : "none"; | ||
|
|
||
| // Toggle userText display based on checkbox state | ||
| userTextCheckbox.addEventListener("change", () => { | ||
| const isVisible = userTextCheckbox.checked; | ||
| userTextDiv.style.display = isVisible ? "block" : "none"; | ||
| localStorage.setItem("userTextVisible", isVisible); | ||
| Storage.setItem("userTextVisible", isVisible); | ||
| }); |
There was a problem hiding this comment.
userTextVisible is read using a string sentinel (Storage.getItem("userTextVisible") !== "false"), but the change handler writes a boolean. If the stored value is boolean false, this expression evaluates to true, so the UI will incorrectly show the custom text even when disabled. Store string values or update the read to handle booleans explicitly.
| getItem(key) { | ||
| if (_isLocalOnly(key)) return localStorage.getItem(key); | ||
| return Object.prototype.hasOwnProperty.call(_cache, key) ? _cache[key] : null; | ||
| }, | ||
|
|
||
| // Write to cache immediately and persist to chrome.storage (fire-and-forget). | ||
| // Callers do not need to await this. | ||
| setItem(key, value) { | ||
| if (_isLocalOnly(key)) { localStorage.setItem(key, value); return; } | ||
| _cache[key] = value; | ||
| const storageArea = _getStorageArea(); | ||
| if (storageArea) storageArea.set({ [key]: value }); | ||
| else localStorage.setItem(key, value); | ||
| }, |
There was a problem hiding this comment.
Storage.setItem stores values into chrome.storage without normalizing types. Several call sites still compare against string values like "true", so after a checkbox writes a boolean the subsequent read will no longer match and settings will appear to reset. Consider normalizing all non-null values to strings (localStorage semantics) when writing to the cache / storageArea (and when hydrating from storageArea.get / onChanged), or update all call sites to handle booleans consistently.
| // ---------- Public Storage API ---------- | ||
|
|
||
| window.Storage = { | ||
| // Synchronous read from cache (or localStorage for local-only keys). | ||
| // Safe to call at any time; returns null if key does not exist. | ||
| getItem(key) { | ||
| if (_isLocalOnly(key)) return localStorage.getItem(key); | ||
| return Object.prototype.hasOwnProperty.call(_cache, key) ? _cache[key] : null; | ||
| }, |
There was a problem hiding this comment.
This wrapper assigns to window.Storage, which collides with the built-in Storage interface (the type behind localStorage/sessionStorage). Overwriting that global can break feature detection and any code relying on the native interface. Prefer a non-reserved global name (e.g., window.AppStorage / window.MYNTStorage) and update call sites accordingly (or keep a backwards-compatible alias without clobbering the native).
| let city = parsedData.location.name; | ||
| let maxLength = 10; | ||
| let isLocationHidden = localStorage.getItem("locationHidden") === "true"; | ||
| let isLocationHidden = Storage.getItem("locationHidden") === "true"; |
There was a problem hiding this comment.
locationHidden is read via Storage.getItem("locationHidden") === "true", but the click handler persists a boolean (Storage.setItem("locationHidden", isLocationHidden)). After the first toggle, the read will always be false. Store as strings or change the read to handle booleans.
| let isLocationHidden = Storage.getItem("locationHidden") === "true"; | |
| const storedLocationHidden = Storage.getItem("locationHidden"); | |
| let isLocationHidden = storedLocationHidden === true || storedLocationHidden === "true"; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
scripts/shortcuts.js (1)
136-160:⚠️ Potential issue | 🟠 MajorAwait storage initialization before loading shortcuts.
loadShortcuts()is called duringDOMContentLoadedwithout awaitingwindow.storageReady. On a profile where shortcuts exist only inchrome.storage.sync, the_cacheis initially populated only fromlocalStorage. Sincewindow.storageReady(which populates the sync data into cache) is still pending,Storage.getItem()returns null for user shortcuts, causing the function to fall back to presets. Any edits made before the sync data arrives will write against the preset dataset instead of the user's actual shortcuts, risking data loss.Wrap
loadShortcuts()inawait window.storageReadyto ensure the cache reflects synced data before reading shortcut entries.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/shortcuts.js` around lines 136 - 160, loadShortcuts is running before sync-backed Storage is initialized, causing Storage.getItem to read stale local values; ensure callers await window.storageReady before invoking loadShortcuts (or make loadShortcuts async and await window.storageReady at its start) so the Storage cache is populated from sync prior to any calls to Storage.getItem; update the DOMContentLoaded initialization that currently calls loadShortcuts to await window.storageReady first (or await loadShortcuts if you made it handle waiting itself) so user shortcuts from chrome.storage.sync are available and edits won't overwrite them.scripts/save-load-states.js (1)
17-29:⚠️ Potential issue | 🟠 MajorThese loaders must be reapplied after
storageReadyresolves.The cache is pre-populated from localStorage synchronously, but chrome.storage data loads asynchronously via
window.storageReady. WhenStorage.getItem()is called synchronously during DOMContentLoaded, it returns only the cached localStorage value. If that differs from synced chrome.storage data (e.g., data synced from another device), the correct value is never applied because loaders are one-shot and are not rerun afterstorageReadyresolves.Also applies to lines 36–44 and 51–59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/save-load-states.js` around lines 17 - 29, loadCheckboxState (and the other loader functions that call Storage.getItem during DOMContentLoaded) run before async chrome.storage finishes, so they must be re-run after window.storageReady resolves; change initialization to call loadCheckboxState (and the loaders referenced at lines 36–44 and 51–59) once during DOMContentLoaded as now, and then attach a .then or await on window.storageReady to re-invoke those same loader functions (using the same keys and checkbox elements like bookmarkGridCheckbox) so the cached localStorage values are overwritten with synced chrome.storage values when available; ensure you do not double-bind click handlers—just call the loader functions again after storageReady resolves.scripts/voice-search.js (1)
18-29:⚠️ Potential issue | 🟠 MajorDon't persist the default before synced values are available, and fix boolean/string type mismatch.
The bootstrap code (lines 18-29) runs before
window.storageReadyresolves, so it reads from cache before chrome.storage.sync has synchronized. WhenStorage.getItem("micIconVisible")returns null, it immediately writes the browser-derived default back, which can overwrite a synced preference arriving from another device.Additionally, line 29 writes a boolean but line 24 reads and compares against the string
"true". SinceStorage.setItem()stores values as-is in the cache (not stringified), this fails the round-trip:true === "true"is false, so reads will always treat the stored boolean as missing and recalculate the default.The same boolean write issue affects line 43 in
toggleMicIconVisibility().Await
window.storageReadybefore reading/writing, and ensure consistent string serialization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/voice-search.js` around lines 18 - 29, The bootstrap code prematurely reads/writes micIconVisible from the local Storage cache and mixes boolean vs string types; change the startup flow in the script so you await window.storageReady before calling Storage.getItem("micIconVisible") or Storage.setItem("micIconVisible"), and when storing the default or toggled value (in both the initial block using isSupportedBrowser() and in toggleMicIconVisibility()), serialize the value consistently (e.g., store "true"/"false" strings) so the existing read comparison savedState === "true" works reliably and a synced preference arriving via chrome.storage.sync won’t be overwritten.scripts/widgets-transparency.js (1)
16-22:⚠️ Potential issue | 🟠 MajorDefer opacity initialization until storage is ready to avoid overwriting synced values.
Storage.getItem("bgOpacity")at module scope runs beforewindow.storageReadycompletes. IfbgOpacitydoesn't exist locally, the fallback to90is persisted immediately viasetSliderPosition(), overwriting the user's synced opacity setting.Suggested fix
-function setSliderPosition(percentage) { +function setSliderPosition(percentage, { persist = true } = {}) { const posPercent = Math.min(100, Math.max(20, percentage)); slider.style.width = `${posPercent}%`; opacityLevel.textContent = `${localizeNumbers(Math.round(posPercent).toString(), currentLanguage)}%`; document.documentElement.style.setProperty("--transparency", `${Math.round(posPercent)}%`); - Storage.setItem("bgOpacity", posPercent); + if (persist) { + Storage.setItem("bgOpacity", posPercent); + } } @@ -const savedOpacity = Storage.getItem("bgOpacity") || 90; -setSliderPosition(Number(savedOpacity)); +window.storageReady.then(() => { + const savedOpacity = Storage.getItem("bgOpacity"); + setSliderPosition(Number(savedOpacity ?? 90), { persist: false }); +});Also applies to: lines 57-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/widgets-transparency.js` around lines 16 - 22, The module currently reads Storage.getItem("bgOpacity") at load time and calls setSliderPosition(…) which always persists a default (90) before window.storageReady completes, overwriting synced values; fix by deferring opacity initialization until window.storageReady resolves: move the Storage.getItem("bgOpacity") call and the calls at the other init site (lines 57-59) into an async handler for window.storageReady (or await window.storageReady), read Storage.getItem("bgOpacity") there, and only call setSliderPosition(value) if a stored value exists; if no stored value, set the UI slider visually without persisting (either add an optional parameter to setSliderPosition like persist=false or create a lightweight setSliderUI function) so you don't call Storage.setItem("bgOpacity", ...) until the user actually changes the slider.scripts/weather.js (1)
9-17:⚠️ Potential issue | 🟠 MajorGate the weather bootstrap on
window.storageReady.These startup reads now target synced storage, but the file still consumes them before
scripts/storage.jshas finished loadingchrome.storageinto_cache. After the migration has clearedlocalStorage, the widget can come up visible when it should be hidden, ignore the saved API key/location/GPS setting, and miss the cached forecast on load. Mirror theawait window.storageReadypattern already used inscripts/theme.jsbefore the firstStorage.getItem(...).Also applies to: 66-76, 303-305, 359-362
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/weather.js` around lines 9 - 17, Make the DOMContentLoaded handler wait for storage to be initialized before reading synced values: turn the document.addEventListener("DOMContentLoaded", ...) callback into an async function and await window.storageReady before any Storage.getItem(...) calls (e.g., the hideWeatherVisible read that sets hideWeatherCheckbox.checked), and apply the same pattern before the other Storage.getItem usages in this file (the blocks that touch weatherHeader/weatherOptions and the later reads around the API key/location/GPS and cached forecast). This ensures Storage.getItem is only called after scripts/storage.js has populated its _cache.scripts/search.js (1)
301-314:⚠️ Potential issue | 🟡 MinorPersist Enter-key selection through the same path as click selection.
This branch still writes the deprecated
selectedSearchEnginekey and never updatesactiveSearchMode. BecauseradioButton.checked = truedoes not trigger thechangelistener, keyboard selection is lost on reload and can reopen in the wrong mode.♿ Proposed fix
const engine = selectedItem.getAttribute("data-engine"); const radioButton = document.querySelector(`input[type="radio"][value="engine${engine}"]`); radioButton.checked = true; + radioButton.dispatchEvent(new Event("change")); // Swap the dropdown and sort them swapDropdown(`*[data-engine="${engine}"]`); sortDropdown(); - - Storage.setItem("selectedSearchEngine", radioButton.value);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/search.js` around lines 301 - 314, The Enter-key handler currently sets radioButton.checked and writes the deprecated "selectedSearchEngine" key but does not update the canonical activeSearchMode nor fire the radio change handler; fix it by (1) dispatching the radio change event after setting radioButton.checked (e.g., radioButton.dispatchEvent(new Event('change')) or similar) so the existing change listener runs, and (2) persist the selection to the current Storage key ("activeSearchMode") (or set both keys if backward compatibility is required) instead of only writing "selectedSearchEngine"; keep the existing swapDropdown and sortDropdown calls and reference the existing symbols selectedItem, engine, radioButton, swapDropdown, sortDropdown, and Storage.setItem to locate where to apply these changes.scripts/theme.js (1)
276-311:⚠️ Potential issue | 🟠 MajorUse
changeevent or much slower debounce for color picker persistence.
chrome.storage.syncenforces a 2 writes/second limit. A 10ms throttle during color picker dragging will attempt ~100 writes/second, consistently exceeding this and causing operations to fail. Switch to thechangeevent (fires only when the user releases the picker) or debounce with at least 500ms to stay within limits.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/theme.js` around lines 276 - 311, The color picker currently uses a 10ms throttle (throttle, handleColorPickerChange) which will exceed chrome.storage.sync write limits; replace this by either binding the handler to the "change" event on colorPicker (instead of "input") or implement a debounce with at least ~500ms before calling handleColorPickerChange so Storage.setItem(customThemeStorageKey, ...) and Storage.removeItem(themeStorageKey) are only invoked infrequently; keep the existing resetDarkTheme() and applyCustomTheme(...) calls inside handleColorPickerChange and ensure you still remove any prior listener (removeEventListener) before adding the new "change" or debounced listener.
🧹 Nitpick comments (2)
scripts/todo-list.js (1)
207-228: Consider awaitingstorageReadyfor cross-device sync accuracy.This code reads
todoLastUpdateDateandtodoListat module parse time using the synchronous cache. For existing users (localStorage data migrated), this works correctly. However, for cross-device sync scenarios wherechrome.storagemay have newer data than the pre-populated cache, the initial render could show stale todos untilstorage.onChangedfires.If todo list consistency across devices is important, consider wrapping this initialization in an async pattern:
💡 Optional refactor for sync accuracy
-let todoLastUpdateDate = Storage.getItem("todoLastUpdateDate"); -let todoCurrentDate = new Date().toLocaleDateString(); - -if (todoLastUpdateDate === todoCurrentDate) { - ShowToDoList(); -} else { - // ...existing logic -} +(async function initTodoList() { + await window.storageReady; + + let todoLastUpdateDate = Storage.getItem("todoLastUpdateDate"); + let todoCurrentDate = new Date().toLocaleDateString(); + + if (todoLastUpdateDate === todoCurrentDate) { + ShowToDoList(); + } else { + // ...existing logic + } +})();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/todo-list.js` around lines 207 - 228, This initialization reads Storage.getItem at module parse time and can show stale data; update the block that uses todoLastUpdateDate, todoCurrentDate, todoList, ShowToDoList and SaveToDoData so it runs only after the storageReady Promise resolves (e.g., make the surrounding initializer async or use an async IIFE), await storageReady before calling Storage.getItem(...) and then perform the date check, list mutation and calls to SaveToDoData/ShowToDoList to ensure cross-device chrome.storage sync has populated the cache first.scripts/clock.js (1)
48-50: Normalize boolean serialization for the clock toggles.
hideClockVisibleandgreetingEnabledare now written as booleans, but every read still compares against the string"true". IfStoragereturns native booleans after sync/migration, both toggles reload as false.Suggested fix
-const isClockHidden = Storage.getItem("hideClockVisible") === "true"; +const isClockHidden = String(Storage.getItem("hideClockVisible")) === "true"; @@ -Storage.setItem("hideClockVisible", isChecked); +Storage.setItem("hideClockVisible", String(isChecked)); @@ -const isGreetingEnabled = Storage.getItem("greetingEnabled") === "true"; +const isGreetingEnabled = String(Storage.getItem("greetingEnabled")) === "true"; @@ -greetingCheckbox.checked = Storage.getItem("greetingEnabled") === "true"; +greetingCheckbox.checked = String(Storage.getItem("greetingEnabled")) === "true"; @@ -Storage.setItem("greetingEnabled", greetingCheckbox.checked); +Storage.setItem("greetingEnabled", String(greetingCheckbox.checked));Also applies to: 60-63, 264-265, 503-504, 545-546
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/clock.js` around lines 48 - 50, Storage keys "hideClockVisible" and "greetingEnabled" are being compared only to the string "true", which fails if Storage returns native booleans; update reads that use Storage.getItem("hideClockVisible") and Storage.getItem("greetingEnabled") to normalize both string and boolean values (e.g., treat a value as true if it === true or === "true") and then assign those normalized booleans to hideClockCheckbox.checked, the greetingEnabled variable, and any other checkbox.checked assignments that reference those keys (ensure you change all occurrences that currently compare to "true").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/backup-restore.js`:
- Around line 235-242: The current localStorage cleanup after Storage.reset()
uses a predicate that excludes "quotes_*", "LoadingScreenColor" and weather keys
so those local-only entries survive; update the cleanup to use the same
predicate logic as in scripts/storage.js (the predicate that identifies synced
vs local-only keys) so local-only keys are removed. Concretely: replace the
localKeys = Object.keys(localStorage).filter(...) block with the same predicate
used in storage.js (or import/centralize that predicate) and then call
localKeys.forEach(k => localStorage.removeItem(k)); keep references to
Storage.reset(), localKeys, and localStorage.removeItem to locate and apply the
change.
In `@scripts/bookmarks.js`:
- Line 30: currentSortMethod is being read at module load via Storage.getItem
which may return a cached fallback before chrome.storage sync finishes; defer or
re-read the saved sort when storage is ready or when bookmarks are first
populated. Update the initialization so currentSortMethod is not set at
top-level: remove the eager let currentSortMethod = Storage.getItem(...) and
instead set currentSortMethod inside the storageReady promise or inside the
function that loads bookmarks (e.g., after bookmarks are fetched), calling
Storage.getItem("bookmarkSortMethod") and applying the sort; ensure any code
that depends on currentSortMethod waits for that initialization or reacts to
changes from Storage.getItem.
In `@scripts/clock.js`:
- Around line 115-119: initializeClockType currently reads/writes clocktype to
Storage immediately and the DOMContentLoaded handler also persists defaults
before window.storageReady resolves, which can overwrite synced settings; change
initializeClockType and the DOMContentLoaded startup code to await
window.storageReady before calling Storage.getItem or Storage.setItem (use
async/await or return a promise) so persistence happens only after sync
completes. Also fix the greetingEnabled and hideClockVisible serialization
mismatch by choosing a single representation: either always store booleans via
Storage.setItem(key, JSON.stringify(value)) and parse with JSON.parse on read,
or always store strings and compare to "true"/"false" when reading; update all
reads/writes (references: greetingEnabled, hideClockVisible, Storage.getItem,
Storage.setItem) to use the chosen consistent serialization.
In `@scripts/languages.js`:
- Around line 404-411: The startup language read runs before storage has synced;
modify the startup handler that calls getLanguageStatus() (currently invoked
inside the window.onload flow) to await window.storageReady first (use an async
IIFE or make the handler async) so getLanguageStatus() reads the synced value
rather than the empty cache; ensure applyLanguage() is only called after
awaiting window.storageReady and after retrieving the stored preference, and
keep saveLanguageStatus/getLanguageStatus unchanged except that callers now
await storage readiness before calling them.
In `@scripts/search-suggestions.js`:
- Around line 11-15: The DOMContentLoaded handler reads savedProxy too early;
modify the listener to await window.storageReady before calling
Storage.getItem("proxy") so the cache is populated first — make the callback
async, await window.storageReady, then call Storage.getItem("proxy") to
initialize savedProxy (and any use of proxyurl) and only then proceed to use
userProxyInput and saveProxyButton; reference the DOMContentLoaded callback,
window.storageReady, Storage.getItem("proxy"), and the savedProxy/proxyurl
initialization when implementing the change.
In `@scripts/search.js`:
- Line 12: The file reads persisted values from Storage.getItem (e.g., the
languageCode declaration and the reads around the initial engine/mode and
shortcut/theme toggles) before scripts/storage.js has finished hydrating, so
change the initialization to wait for window.storageReady first: make the module
initialization async (or wrap startup in an async IIFE) and await
window.storageReady before calling Storage.getItem, then compute languageCode
and any other persisted defaults (the blocks currently using Storage.getItem at
the top and the groups of reads you flagged) so they come from the
fully-hydrated cache rather than the in-memory placeholder.
In `@scripts/storage.js`:
- Around line 104-107: The Storage.getItem wrapper returns raw booleans/numbers
from _cache which breaks the localStorage string contract; in getItem (the
function that checks _isLocalOnly and uses _cache) ensure values returned from
_cache are normalized to strings (e.g., return null if missing, otherwise
String(_cache[key])) so callers doing string comparisons (like
scripts/weather.js) continue to work; keep localStorage.getItem behavior
unchanged for local-only keys.
---
Outside diff comments:
In `@scripts/save-load-states.js`:
- Around line 17-29: loadCheckboxState (and the other loader functions that call
Storage.getItem during DOMContentLoaded) run before async chrome.storage
finishes, so they must be re-run after window.storageReady resolves; change
initialization to call loadCheckboxState (and the loaders referenced at lines
36–44 and 51–59) once during DOMContentLoaded as now, and then attach a .then or
await on window.storageReady to re-invoke those same loader functions (using the
same keys and checkbox elements like bookmarkGridCheckbox) so the cached
localStorage values are overwritten with synced chrome.storage values when
available; ensure you do not double-bind click handlers—just call the loader
functions again after storageReady resolves.
In `@scripts/search.js`:
- Around line 301-314: The Enter-key handler currently sets radioButton.checked
and writes the deprecated "selectedSearchEngine" key but does not update the
canonical activeSearchMode nor fire the radio change handler; fix it by (1)
dispatching the radio change event after setting radioButton.checked (e.g.,
radioButton.dispatchEvent(new Event('change')) or similar) so the existing
change listener runs, and (2) persist the selection to the current Storage key
("activeSearchMode") (or set both keys if backward compatibility is required)
instead of only writing "selectedSearchEngine"; keep the existing swapDropdown
and sortDropdown calls and reference the existing symbols selectedItem, engine,
radioButton, swapDropdown, sortDropdown, and Storage.setItem to locate where to
apply these changes.
In `@scripts/shortcuts.js`:
- Around line 136-160: loadShortcuts is running before sync-backed Storage is
initialized, causing Storage.getItem to read stale local values; ensure callers
await window.storageReady before invoking loadShortcuts (or make loadShortcuts
async and await window.storageReady at its start) so the Storage cache is
populated from sync prior to any calls to Storage.getItem; update the
DOMContentLoaded initialization that currently calls loadShortcuts to await
window.storageReady first (or await loadShortcuts if you made it handle waiting
itself) so user shortcuts from chrome.storage.sync are available and edits won't
overwrite them.
In `@scripts/theme.js`:
- Around line 276-311: The color picker currently uses a 10ms throttle
(throttle, handleColorPickerChange) which will exceed chrome.storage.sync write
limits; replace this by either binding the handler to the "change" event on
colorPicker (instead of "input") or implement a debounce with at least ~500ms
before calling handleColorPickerChange so Storage.setItem(customThemeStorageKey,
...) and Storage.removeItem(themeStorageKey) are only invoked infrequently; keep
the existing resetDarkTheme() and applyCustomTheme(...) calls inside
handleColorPickerChange and ensure you still remove any prior listener
(removeEventListener) before adding the new "change" or debounced listener.
In `@scripts/voice-search.js`:
- Around line 18-29: The bootstrap code prematurely reads/writes micIconVisible
from the local Storage cache and mixes boolean vs string types; change the
startup flow in the script so you await window.storageReady before calling
Storage.getItem("micIconVisible") or Storage.setItem("micIconVisible"), and when
storing the default or toggled value (in both the initial block using
isSupportedBrowser() and in toggleMicIconVisibility()), serialize the value
consistently (e.g., store "true"/"false" strings) so the existing read
comparison savedState === "true" works reliably and a synced preference arriving
via chrome.storage.sync won’t be overwritten.
In `@scripts/weather.js`:
- Around line 9-17: Make the DOMContentLoaded handler wait for storage to be
initialized before reading synced values: turn the
document.addEventListener("DOMContentLoaded", ...) callback into an async
function and await window.storageReady before any Storage.getItem(...) calls
(e.g., the hideWeatherVisible read that sets hideWeatherCheckbox.checked), and
apply the same pattern before the other Storage.getItem usages in this file (the
blocks that touch weatherHeader/weatherOptions and the later reads around the
API key/location/GPS and cached forecast). This ensures Storage.getItem is only
called after scripts/storage.js has populated its _cache.
In `@scripts/widgets-transparency.js`:
- Around line 16-22: The module currently reads Storage.getItem("bgOpacity") at
load time and calls setSliderPosition(…) which always persists a default (90)
before window.storageReady completes, overwriting synced values; fix by
deferring opacity initialization until window.storageReady resolves: move the
Storage.getItem("bgOpacity") call and the calls at the other init site (lines
57-59) into an async handler for window.storageReady (or await
window.storageReady), read Storage.getItem("bgOpacity") there, and only call
setSliderPosition(value) if a stored value exists; if no stored value, set the
UI slider visually without persisting (either add an optional parameter to
setSliderPosition like persist=false or create a lightweight setSliderUI
function) so you don't call Storage.setItem("bgOpacity", ...) until the user
actually changes the slider.
---
Nitpick comments:
In `@scripts/clock.js`:
- Around line 48-50: Storage keys "hideClockVisible" and "greetingEnabled" are
being compared only to the string "true", which fails if Storage returns native
booleans; update reads that use Storage.getItem("hideClockVisible") and
Storage.getItem("greetingEnabled") to normalize both string and boolean values
(e.g., treat a value as true if it === true or === "true") and then assign those
normalized booleans to hideClockCheckbox.checked, the greetingEnabled variable,
and any other checkbox.checked assignments that reference those keys (ensure you
change all occurrences that currently compare to "true").
In `@scripts/todo-list.js`:
- Around line 207-228: This initialization reads Storage.getItem at module parse
time and can show stale data; update the block that uses todoLastUpdateDate,
todoCurrentDate, todoList, ShowToDoList and SaveToDoData so it runs only after
the storageReady Promise resolves (e.g., make the surrounding initializer async
or use an async IIFE), await storageReady before calling Storage.getItem(...)
and then perform the date check, list mutation and calls to
SaveToDoData/ShowToDoList to ensure cross-device chrome.storage sync has
populated the cache first.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 60326c3f-124e-4454-8c12-0fd93fe865b4
📒 Files selected for processing (22)
CHANGELOG.mdindex.htmlmanifest(firefox).jsonmanifest.jsonscripts/ai-tools.jsscripts/backup-restore.jsscripts/bookmarks.jsscripts/clock.jsscripts/custom-text.jsscripts/languages.jsscripts/menu-shortcut-page.jsscripts/save-load-states.jsscripts/script.jsscripts/search-suggestions.jsscripts/search.jsscripts/shortcuts.jsscripts/storage.jsscripts/theme.jsscripts/todo-list.jsscripts/voice-search.jsscripts/weather.jsscripts/widgets-transparency.js
| await Storage.reset(); | ||
| // Also clear local-only localStorage data | ||
| const localKeys = Object.keys(localStorage).filter(k => | ||
| !k.startsWith("quotes_") && k !== "LoadingScreenColor" && | ||
| k !== "weatherParsedData" && k !== "weatherParsedTime" && | ||
| k !== "weatherParsedLocation" && k !== "weatherParsedLang" | ||
| ); | ||
| localKeys.forEach(k => localStorage.removeItem(k)); |
There was a problem hiding this comment.
Reset leaves the local-only data behind.
Storage.reset() already clears synced keys. This filter then removes only non-local-only localStorage entries, so quotes_*, LoadingScreenColor, and the weather cache survive reset even though the comment says the opposite. Reusing the same predicate as scripts/storage.js:18-24 here would also prevent this from drifting again.
🧹 Proposed fix
- const localKeys = Object.keys(localStorage).filter(k =>
- !k.startsWith("quotes_") && k !== "LoadingScreenColor" &&
- k !== "weatherParsedData" && k !== "weatherParsedTime" &&
- k !== "weatherParsedLocation" && k !== "weatherParsedLang"
- );
+ const localKeys = Object.keys(localStorage).filter(k =>
+ k.startsWith("quotes_") || k === "LoadingScreenColor" ||
+ k === "weatherParsedData" || k === "weatherParsedTime" ||
+ k === "weatherParsedLocation" || k === "weatherParsedLang"
+ );
localKeys.forEach(k => localStorage.removeItem(k));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await Storage.reset(); | |
| // Also clear local-only localStorage data | |
| const localKeys = Object.keys(localStorage).filter(k => | |
| !k.startsWith("quotes_") && k !== "LoadingScreenColor" && | |
| k !== "weatherParsedData" && k !== "weatherParsedTime" && | |
| k !== "weatherParsedLocation" && k !== "weatherParsedLang" | |
| ); | |
| localKeys.forEach(k => localStorage.removeItem(k)); | |
| await Storage.reset(); | |
| // Also clear local-only localStorage data | |
| const localKeys = Object.keys(localStorage).filter(k => | |
| k.startsWith("quotes_") || k === "LoadingScreenColor" || | |
| k === "weatherParsedData" || k === "weatherParsedTime" || | |
| k === "weatherParsedLocation" || k === "weatherParsedLang" | |
| ); | |
| localKeys.forEach(k => localStorage.removeItem(k)); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/backup-restore.js` around lines 235 - 242, The current localStorage
cleanup after Storage.reset() uses a predicate that excludes "quotes_*",
"LoadingScreenColor" and weather keys so those local-only entries survive;
update the cleanup to use the same predicate logic as in scripts/storage.js (the
predicate that identifies synced vs local-only keys) so local-only keys are
removed. Concretely: replace the localKeys =
Object.keys(localStorage).filter(...) block with the same predicate used in
storage.js (or import/centralize that predicate) and then call
localKeys.forEach(k => localStorage.removeItem(k)); keep references to
Storage.reset(), localKeys, and localStorage.removeItem to locate and apply the
change.
| const sortAlphabetical = document.getElementById("sortAlphabetical"); | ||
| const sortTimeAdded = document.getElementById("sortTimeAdded"); | ||
| let currentSortMethod = localStorage.getItem("bookmarkSortMethod") || 'title'; | ||
| let currentSortMethod = Storage.getItem("bookmarkSortMethod") || 'title'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, get an overview of the bookmarks.js file
wc -l scripts/bookmarks.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 94
🏁 Script executed:
# View the area around line 30 and surrounding context
sed -n '1,50p' scripts/bookmarks.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 2122
🏁 Script executed:
# Search for storageReady event handling and how it's used
rg -A 5 "storageReady" scripts/bookmarks.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 52
🏁 Script executed:
# Search for all assignments to currentSortMethod to see if it's updated later
rg "currentSortMethod\s*=" scripts/bookmarks.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 413
🏁 Script executed:
# Search for Storage.getItem calls related to bookmarkSortMethod
rg "bookmarkSortMethod" scripts/bookmarks.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 254
🏁 Script executed:
# Search for where currentSortMethod is assigned (context around those assignments)
rg -B 3 -A 3 "currentSortMethod\s*=" scripts/bookmarks.js | head -40Repository: prem-k-r/MaterialYouNewTab
Length of output: 1360
🏁 Script executed:
# Check if there's a Storage module/object definition
fd -type f "Storage" . --ignore-caseRepository: prem-k-r/MaterialYouNewTab
Length of output: 240
🏁 Script executed:
# Search for storageReady or storage sync patterns in the entire codebase
rg "storageReady" --type jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 225
🏁 Script executed:
# Look for event listeners on storage changes
rg "addEventListener.*storage|storage.*addEventListener" --type jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 52
🏁 Script executed:
# Get context around lines where currentSortMethod is set - appears to be around 138, 147 based on the original comment
sed -n '130,160p' scripts/bookmarks.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 1044
🏁 Script executed:
# Check the storage.js to understand how storageReady works
cat -n scripts/storage.js | head -50Repository: prem-k-r/MaterialYouNewTab
Length of output: 2601
🏁 Script executed:
# Look for when bookmarks.js is loaded/initialized - check if it's loaded after storageReady
rg "bookmarks" index.html --type html -A 2 -B 2Repository: prem-k-r/MaterialYouNewTab
Length of output: 1430
🏁 Script executed:
# Check if there are any other uses of await storageReady in the codebase to see the pattern
rg "await.*storageReady" --type-listRepository: prem-k-r/MaterialYouNewTab
Length of output: 459
🏁 Script executed:
# Get full context of how bookmarks.js initializes and whether there's any storageReady handling
sed -n '1,80p' scripts/bookmarks.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 3171
🏁 Script executed:
# Check the HTML to see script loading order
head -100 index.html | tail -50Repository: prem-k-r/MaterialYouNewTab
Length of output: 2672
🏁 Script executed:
# Check if bookmarks.js has any storageReady handling
rg "storageReady" scripts/bookmarks.js -A 3 -B 3Repository: prem-k-r/MaterialYouNewTab
Length of output: 52
🏁 Script executed:
# Check the complete Storage wrapper implementation to understand how it syncs
sed -n '50,150p' scripts/storage.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 4563
🏁 Script executed:
# Look for any listener or callback that might update currentSortMethod after storageReady
rg "currentSortMethod|bookmarkSortMethod" scripts/bookmarks.js -B 2 -A 2Repository: prem-k-r/MaterialYouNewTab
Length of output: 1236
🏁 Script executed:
# Check if there are any onChanged listeners for storage
rg "onChanged|addListener" scripts/bookmarks.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 52
🏁 Script executed:
# Check if updateSortButtons is called at initialization (and if updateSortButtons runs before storageReady)
sed -n '30,45p' scripts/bookmarks.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 463
🏁 Script executed:
# Check the complete sequence of when bookmarks functionality becomes available
# Look for when loadBookmarks is called
rg "loadBookmarks()" scripts/bookmarks.js -B 3 -A 3 | head -40Repository: prem-k-r/MaterialYouNewTab
Length of output: 818
🏁 Script executed:
# Check if there's any coordination between storageReady and bookmarks initialization
rg "DOMContentLoaded|addEventListener.*load" scripts/bookmarks.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 129
Read bookmarkSortMethod lazily or after storageReady completes.
currentSortMethod is captured at script load time from the _cache before chrome.storage sync has loaded. If the synced value arrives later, the session will stay on the fallback 'title' sort until the user manually changes it. Consider deferring the initialization or re-reading from Storage when bookmarks are first loaded.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/bookmarks.js` at line 30, currentSortMethod is being read at module
load via Storage.getItem which may return a cached fallback before
chrome.storage sync finishes; defer or re-read the saved sort when storage is
ready or when bookmarks are first populated. Update the initialization so
currentSortMethod is not set at top-level: remove the eager let
currentSortMethod = Storage.getItem(...) and instead set currentSortMethod
inside the storageReady promise or inside the function that loads bookmarks
(e.g., after bookmarks are fetched), calling
Storage.getItem("bookmarkSortMethod") and applying the sort; ensure any code
that depends on currentSortMethod waits for that initialization or reacts to
changes from Storage.getItem.
| function initializeClockType() { | ||
| const savedClockType = localStorage.getItem("clocktype"); | ||
| const savedClockType = Storage.getItem("clocktype"); | ||
| clocktype = savedClockType ? savedClockType : "analog"; // Default to "analog" if nothing is saved | ||
| localStorage.setItem("clocktype", clocktype); // Ensure it's set in local storage | ||
| Storage.setItem("clocktype", clocktype); // Ensure it's set in local storage | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -E "^scripts/(clock|storage)" | head -20Repository: prem-k-r/MaterialYouNewTab
Length of output: 105
🏁 Script executed:
cd scripts && ls -la | head -20Repository: prem-k-r/MaterialYouNewTab
Length of output: 1299
🏁 Script executed:
wc -l scripts/clock.js scripts/storage.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 129
🏁 Script executed:
sed -n '115,125p' scripts/clock.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 525
🏁 Script executed:
sed -n '495,505p' scripts/clock.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 595
🏁 Script executed:
cat -n scripts/storage.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 9045
🏁 Script executed:
sed -n '1,30p' scripts/clock.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 1207
🏁 Script executed:
rg -n "initializeClockType|greetingEnabled" scripts/clock.js -A 2 -B 2Repository: prem-k-r/MaterialYouNewTab
Length of output: 1473
🏁 Script executed:
rg -n "DOMContentLoaded" scripts/clock.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 137
🏁 Script executed:
sed -n '490,510p' scripts/clock.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 1154
🏁 Script executed:
rg -n "window.storageReady" scripts/ -A 2 -B 2Repository: prem-k-r/MaterialYouNewTab
Length of output: 1009
🏁 Script executed:
rg -n "Storage.setItem.*checked|setItem.*greetingEnabled" scripts/clock.js -A 1 -B 1Repository: prem-k-r/MaterialYouNewTab
Length of output: 391
🏁 Script executed:
rg -n "Storage.setItem.*checked|Storage.setItem\(.*\)" scripts/clock.js | head -20Repository: prem-k-r/MaterialYouNewTab
Length of output: 606
🏁 Script executed:
rg -n "Storage.getItem.*===" scripts/clock.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 387
🏁 Script executed:
sed -n '540,550p' scripts/clock.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 429
🏁 Script executed:
cat > /tmp/check_serialization.py << 'EOF'
# When greetingCheckbox.checked (boolean true/false) is passed to Storage.setItem,
# it gets stored in _cache as a boolean, then sent to chrome.storage.
# Chrome storage serializes to JSON, converting boolean true to string "true".
# But reads from _cache might get the boolean first, before sync completes.
print("Scenario 1: Write boolean before sync")
print(" Storage.setItem('greetingEnabled', true)")
print(" -> _cache['greetingEnabled'] = true (boolean)")
print(" -> storageArea.set({greetingEnabled: true})")
print("")
print("Scenario 2: Read immediately after write")
print(" Storage.getItem('greetingEnabled') === 'true'")
print(" -> Returns true (boolean) from cache !== 'true' (string) MISMATCH!")
print("")
print("Scenario 3: Read after chrome.storage syncs")
print(" -> _cache['greetingEnabled'] = 'true' (string from sync)")
print(" -> Returns true (boolean) === 'true' (string) OK")
EOF
python /tmp/check_serialization.pyRepository: prem-k-r/MaterialYouNewTab
Length of output: 558
🏁 Script executed:
sed -n '110,125p' scripts/clock.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 877
🏁 Script executed:
rg -n "document.addEventListener|defer" scripts/clock.js | head -10Repository: prem-k-r/MaterialYouNewTab
Length of output: 206
🏁 Script executed:
sed -n '60,65p' scripts/clock.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 339
🏁 Script executed:
sed -n '45,55p' scripts/clock.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 444
🏁 Script executed:
rg -n "Storage.getItem.*hideClockVisible" scripts/clock.js -A 1 -B 1Repository: prem-k-r/MaterialYouNewTab
Length of output: 254
Await storage sync before seeding clock defaults.
Both startup paths (initializeClockType() at line 122 and the DOMContentLoaded handler at lines 499–500) persist fallback values before window.storageReady completes. On a fresh synced install, these writes replace the user's existing synced settings with defaults.
Additionally, greetingEnabled and hideClockVisible are written as booleans (lines 546, 62) but read with string comparisons (lines 503, 264, 49), creating a serialization mismatch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/clock.js` around lines 115 - 119, initializeClockType currently
reads/writes clocktype to Storage immediately and the DOMContentLoaded handler
also persists defaults before window.storageReady resolves, which can overwrite
synced settings; change initializeClockType and the DOMContentLoaded startup
code to await window.storageReady before calling Storage.getItem or
Storage.setItem (use async/await or return a promise) so persistence happens
only after sync completes. Also fix the greetingEnabled and hideClockVisible
serialization mismatch by choosing a single representation: either always store
booleans via Storage.setItem(key, JSON.stringify(value)) and parse with
JSON.parse on read, or always store strings and compare to "true"/"false" when
reading; update all reads/writes (references: greetingEnabled, hideClockVisible,
Storage.getItem, Storage.setItem) to use the chosen consistent serialization.
| // Function to save the language status to Storage | ||
| function saveLanguageStatus(key, languageStatus) { | ||
| localStorage.setItem(key, languageStatus); | ||
| Storage.setItem(key, languageStatus); | ||
| } | ||
|
|
||
| // Function to get the language status from localStorage | ||
| // Function to get the language status from Storage | ||
| function getLanguageStatus(key) { | ||
| return localStorage.getItem(key); | ||
| return Storage.getItem(key); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l scripts/languages.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 94
🏁 Script executed:
cat -n scripts/languages.js | head -50Repository: prem-k-r/MaterialYouNewTab
Length of output: 1691
🏁 Script executed:
cat -n scripts/languages.js | sed -n '300,320p'Repository: prem-k-r/MaterialYouNewTab
Length of output: 1158
🏁 Script executed:
cat -n scripts/languages.js | sed -n '400,415p'Repository: prem-k-r/MaterialYouNewTab
Length of output: 537
🏁 Script executed:
# Search for window.onload, window.storageReady, and applyLanguage usage
rg -n "window\.onload|window\.storageReady|applyLanguage|getLanguageStatus|saveLanguageStatus" scripts/languages.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 474
🏁 Script executed:
rg -n "storageReady" scripts/Repository: prem-k-r/MaterialYouNewTab
Length of output: 234
🏁 Script executed:
# Search for Storage definition and initialization
rg -n "^Storage\s*=|window\.Storage|function Storage"Repository: prem-k-r/MaterialYouNewTab
Length of output: 115
🏁 Script executed:
# Search for where Storage is defined and how it's initialized
rg -n "Storage\." scripts/ | head -20Repository: prem-k-r/MaterialYouNewTab
Length of output: 1872
🏁 Script executed:
# Look for Storage initialization in manifest or main files
fd -t f -e js | xargs grep -l "storageReady\|Storage.*ready\|Storage.*init" | head -10Repository: prem-k-r/MaterialYouNewTab
Length of output: 126
🏁 Script executed:
# Check the entire context around window.onload to see full initialization flow
cat -n scripts/languages.js | sed -n '380,412p'Repository: prem-k-r/MaterialYouNewTab
Length of output: 1457
🏁 Script executed:
cat -n scripts/storage.js | sed -n '65,110p'Repository: prem-k-r/MaterialYouNewTab
Length of output: 2348
🏁 Script executed:
# Check if window.onload in languages.js is synchronous and if it races with storageReady
rg -n "window\.onload|DOMContentLoaded" scripts/Repository: prem-k-r/MaterialYouNewTab
Length of output: 1315
🏁 Script executed:
# Check the order of script loading in HTML
fd -t f -e html | xargs grep -n "storage\.js\|languages\.js" | head -20Repository: prem-k-r/MaterialYouNewTab
Length of output: 199
🏁 Script executed:
cat -n scripts/theme.js | sed -n '20,30p'Repository: prem-k-r/MaterialYouNewTab
Length of output: 500
🏁 Script executed:
cat -n scripts/theme.js | sed -n '85,95p'Repository: prem-k-r/MaterialYouNewTab
Length of output: 496
🏁 Script executed:
cat -n scripts/theme.js | sed -n '20,30p'Repository: prem-k-r/MaterialYouNewTab
Length of output: 500
🏁 Script executed:
cat -n scripts/theme.js | sed -n '85,95p'Repository: prem-k-r/MaterialYouNewTab
Length of output: 496
Await window.storageReady before reading language preference on startup.
The window.onload handler at line 398 calls getLanguageStatus() without awaiting window.storageReady, so it reads from an empty cache on startup. The fallback to "en" is then immediately saved back via applyLanguage(), overwriting the user's synced language preference on fresh profiles. Refactor to use an async IIFE or DOMContentLoaded with await window.storageReady (as demonstrated in theme.js).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/languages.js` around lines 404 - 411, The startup language read runs
before storage has synced; modify the startup handler that calls
getLanguageStatus() (currently invoked inside the window.onload flow) to await
window.storageReady first (use an async IIFE or make the handler async) so
getLanguageStatus() reads the synced value rather than the empty cache; ensure
applyLanguage() is only called after awaiting window.storageReady and after
retrieving the stored preference, and keep saveLanguageStatus/getLanguageStatus
unchanged except that callers now await storage readiness before calling them.
| document.addEventListener("DOMContentLoaded", () => { | ||
| const userProxyInput = document.getElementById("userproxy"); | ||
| const saveProxyButton = document.getElementById("saveproxy"); | ||
| const savedProxy = localStorage.getItem("proxy"); | ||
| const savedProxy = Storage.getItem("proxy"); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n scripts/search-suggestions.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 17127
🏁 Script executed:
rg -l "Storage\s*=" --type js | head -20Repository: prem-k-r/MaterialYouNewTab
Length of output: 114
🏁 Script executed:
rg "window\.storageReady" --type jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 225
🏁 Script executed:
rg "getItem|setItem" --type js -B2 -A2 | head -50Repository: prem-k-r/MaterialYouNewTab
Length of output: 2663
🏁 Script executed:
cat scripts/storage.jsRepository: prem-k-r/MaterialYouNewTab
Length of output: 7806
Await window.storageReady before initializing proxyurl.
On a fresh synced profile, the custom proxy is loaded into the cache by window.storageReady after migration and sync completion. Reading savedProxy during DOMContentLoaded may return null if chrome.storage.sync has not yet populated the cache, causing proxyurl to default to the fallback endpoint for the entire session. Update line 11 to await window.storageReady before line 14's Storage.getItem("proxy") call.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/search-suggestions.js` around lines 11 - 15, The DOMContentLoaded
handler reads savedProxy too early; modify the listener to await
window.storageReady before calling Storage.getItem("proxy") so the cache is
populated first — make the callback async, await window.storageReady, then call
Storage.getItem("proxy") to initialize savedProxy (and any use of proxyurl) and
only then proceed to use userProxyInput and saveProxyButton; reference the
DOMContentLoaded callback, window.storageReady, Storage.getItem("proxy"), and
the savedProxy/proxyurl initialization when implementing the change.
| const searchInput = document.getElementById("searchQ"); | ||
|
|
||
| const languageCode = (localStorage.getItem("selectedLanguage") || "en").slice(0, 2); | ||
| const languageCode = (Storage.getItem("selectedLanguage") || "en").slice(0, 2); |
There was a problem hiding this comment.
Await window.storageReady before initializing persisted search state.
These reads now happen against the in-memory cache before scripts/storage.js has finished hydrating it from extension storage. After the migration has cleared localStorage, this can bootstrap the tab with defaults for the whole session: Line 12 can pin Wikipedia to en, the initial engine/mode can be wrong, and Line 395 can even overwrite a previously saved showShortcutSwitch=true with false. scripts/theme.js already uses the correct pattern.
Also applies to: 46-47, 235-237, 385-396
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/search.js` at line 12, The file reads persisted values from
Storage.getItem (e.g., the languageCode declaration and the reads around the
initial engine/mode and shortcut/theme toggles) before scripts/storage.js has
finished hydrating, so change the initialization to wait for window.storageReady
first: make the module initialization async (or wrap startup in an async IIFE)
and await window.storageReady before calling Storage.getItem, then compute
languageCode and any other persisted defaults (the blocks currently using
Storage.getItem at the top and the groups of reads you flagged) so they come
from the fully-hydrated cache rather than the in-memory placeholder.
| getItem(key) { | ||
| if (_isLocalOnly(key)) return localStorage.getItem(key); | ||
| return Object.prototype.hasOwnProperty.call(_cache, key) ? _cache[key] : null; | ||
| }, |
There was a problem hiding this comment.
Keep Storage.getItem() compatible with the old localStorage contract.
This wrapper is replacing localStorage, but it now returns raw booleans/numbers from _cache. Existing consumers still do string comparisons — scripts/weather.js Line 16, Line 75, and Line 541 all check against "true" — so synced settings can read back as off after a storage round-trip. Normalize getItem() output to strings, or finish auditing every caller before release.
🔁 Proposed fix
getItem(key) {
- if (_isLocalOnly(key)) return localStorage.getItem(key);
- return Object.prototype.hasOwnProperty.call(_cache, key) ? _cache[key] : null;
+ const value = _isLocalOnly(key)
+ ? localStorage.getItem(key)
+ : Object.prototype.hasOwnProperty.call(_cache, key) ? _cache[key] : null;
+ return value == null ? null : String(value);
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/storage.js` around lines 104 - 107, The Storage.getItem wrapper
returns raw booleans/numbers from _cache which breaks the localStorage string
contract; in getItem (the function that checks _isLocalOnly and uses _cache)
ensure values returned from _cache are normalized to strings (e.g., return null
if missing, otherwise String(_cache[key])) so callers doing string comparisons
(like scripts/weather.js) continue to work; keep localStorage.getItem behavior
unchanged for local-only keys.
Replaces direct localStorage usage with a centralized Storage wrapper (scripts/storage.js) that uses chrome.storage.sync on Chrome and browser.storage.local on Firefox.
Key changes:
Closes #108
📌 Description
Replaces direct
localStorageusage across the entire extension with a centralizedStoragewrapper (scripts/storage.js) that useschrome.storage.syncon Chrome andbrowser.storage.localon Firefox.🎨 Visual Changes (Screenshots / Videos)
No visual changes — this is an internal storage layer refactor. User-facing behavior remains identical.
🔗 Related Issues
✅ Checklist
Storage API Migration with Sync Support
Overview
Replaces direct
localStorageusage with a centralizedStoragewrapper (scripts/storage.js) that provides unified access to browser storage APIs, enabling settings synchronization across devices on Chrome while maintaining offline support and respecting sync quotas.Key Technical Changes
New Storage Wrapper (
scripts/storage.js)window.Storageandwindow.storageReadyPromise) that abstracts storage backend selectionchrome.storage.syncon Chrome andbrowser.storage.localon Firefox; falls back tolocalStoragewhen extension storage is unavailablequotes_*,LoadingScreenColor,weatherParsed*) to avoid syncing large or quota-sensitive datalocalStoragedata to the appropriate storage backend on first load (tracked by_storage_migratedflag)localStoragefor fast reads by defer-loaded scriptsonChangedlisteners for cross-device/cross-context updatesgetItem(),setItem(),removeItem(),reset(),clearForRestore(),getAll(),setAll()Bootstrap and Theme Initialization
scripts/storage.jsadded toindex.htmlto ensurewindow.Storageandwindow.storageReadyare available earlyscripts/theme.jsupdated toawait window.storageReadybefore applying persisted theme/color preferences, eliminating flash of incorrect theme on page loadBackup/Restore Compatibility
scripts/backup-restore.jsto include synced settings fromchromeStoragein backups alongside local-onlylocalStoragekeysStorage.clearForRestore()and selectively removes local-only keysStorage Permissions
"storage"permission to bothmanifest.jsonandmanifest(firefox).jsonto enable extension storage API accesslocalStorage Replacement Across 16 Scripts
localStorage.getItem/setItemcalls withStorage.getItem/setItemin:scripts/bookmarks.js,scripts/clock.js,scripts/custom-text.js,scripts/menu-shortcut-page.js,scripts/save-load-states.js,scripts/script.js,scripts/shortcuts.js,scripts/voice-search.js,scripts/widgets-transparency.jsscripts/ai-tools.js,scripts/languages.js,scripts/search.js,scripts/search-suggestions.js,scripts/todo-list.js,scripts/weather.jsBenefits
chrome.storage.syncFiles Modified
CHANGELOG.md: Added changelog entriesindex.html: Added storage.js scriptmanifest.json,manifest(firefox).json: Added storage permissionscripts/storage.js: New centralized wrapper (177 lines)scripts/backup-restore.js: Updated to include chromeStorage and support both backup formatsscripts/theme.js: Added await storageReady gatesUser-Facing Changes
None—behavior remains the same. Existing user data is automatically migrated; synced settings are transparent to users on Chrome.