Skip to content

fix dark mode reset#40

Open
Utkarsh-rwt wants to merge 2 commits intoShantanugupta43:mainfrom
Utkarsh-rwt:fix/dark-mode-reset
Open

fix dark mode reset#40
Utkarsh-rwt wants to merge 2 commits intoShantanugupta43:mainfrom
Utkarsh-rwt:fix/dark-mode-reset

Conversation

@Utkarsh-rwt
Copy link
Copy Markdown
Contributor

Fix popup dark mode reset by persisting theme state in chrome.storage.local and restoring it on initialization.

Copy link
Copy Markdown
Owner

@Shantanugupta43 Shantanugupta43 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey great work need a few changes and PR is good to merge.

setupEventListeners();


async function loadDarkMode() {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the function is nested inside initialize(). Cut it out and paste it above initialize() as its own function because No other part of your popup script can reuse it and It becomes harder to debug or extend later

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there may be an XSS risk the browser may interpret the text as HTML code. Would be better to make it

const strong = document.createElement('strong');
strong.textContent = 'Why: ';
derivationEl.appendChild(strong);
derivationEl.appendChild(document.createTextNode(derivation));

setupEventListeners();


async function loadDarkMode() {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the function is nested inside initialize(). Cut it out and paste it above initialize() as its own function so that because no other part of your popup script can reuse it and It becomes harder to debug or extend later.

…ne top-level async function loadDarkMode() declared before initialize(). The dark mode DOM variable declarations (sign, outer, inner, text) were also moved up alongside it so they're in scope when the function runs.

Fix 2 — XSS-safe derivation rendering: The derivationEl.innerHTML = \<strong>Why:</strong> ${derivation}`line is replaced with explicit DOM construction —createElement('strong')withtextContent, then createTextNode(derivation)` appended separately — so user-supplied derivation text is never parsed as HTML.
Copy link
Copy Markdown
Contributor Author

@Utkarsh-rwt Utkarsh-rwt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix 1 — loadDarkMode hoisted out of initialize(): It's now a standalone top-level async function loadDarkMode() declared before initialize(). The dark mode DOM variable declarations (sign, outer, inner, text) were also moved up alongside it so they're in scope when the function runs.
Fix 2 — XSS-safe derivation rendering: The derivationEl.innerHTML = <strong>Why: ${derivation}line is replaced with explicit DOM construction —createElement('strong')withtextContent, then createTextNode(derivation) appended separately — so user-supplied derivation text is never parsed as HTML.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants