Skip to content

fix: prevent popup crash by ensuring DOM is loaded before accessing elements #81

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Khushi5623 wants to merge 6 commits into
base: master
Choose a base branch
from
+59 −39
Open

fix: prevent popup crash by ensuring DOM is loaded before accessing elements #81

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
997ece4
Improved scan function by removing dependency on runtime messaging an…
Khushi5623 Apr 2, 2026
683024a
Improve error message for invalid extension input
Khushi5623 Apr 2, 2026
ce4d1fc
Fix esc() function to handle null/undefined safely and prevent UI issues
Khushi5623 Apr 2, 2026
c947196
Enhance loading UX by disabling submit button during scan
Khushi5623 Apr 2, 2026
2faa5da
fix: prevent popup crash by ensuring DOM is loaded before accessing e…
Khushi5623 Apr 3, 2026
2fdf479
Merge branch 'master' into fix-popup-crash
Khushi5623 Apr 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 59 additions & 39 deletions packages/extension/src/popup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* ExtensionShield – popup (self-contained, no service worker needed) */
(function () {
'use strict';
document.addEventListener("DOMContentLoaded", () => {

var API = 'https://extensionshield.com';
var CACHE_TTL = 6 * 3600 * 1000;
Expand Down Expand Up @@ -181,13 +182,15 @@
}

var extId = extractExtensionIdFromInput(raw);
if (!extId) {
setScanUrlMessage('Enter a Chrome Web Store URL.', 'error');
return;
}

if (!extId) {
setScanUrlMessage('Please enter a valid Chrome Web Store URL or Extension ID.', 'error');
return;
}
setScanUrlMessage('Scanning…', '');
setScanSearchLoading(true);
if (scanUrlSubmit) {
scanUrlSubmit.disabled = true;
}
Comment on lines +191 to +193
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Guard handleScanUrlSubmit() itself against re-entry.

Disabling the button only blocks pointer clicks. The Enter-key handler still calls handleScanUrlSubmit() directly at Lines 315-318, so users can start overlapping scans while the first one is still in flight. Add an early return at the top of the handler (or a dedicated isScanning flag) so every entry path shares the same lock.

Suggested fix 
 function handleScanUrlSubmit() {
+    if (scanUrlSubmit && scanUrlSubmit.disabled) return;
     var raw = scanUrlInput && scanUrlInput.value ? scanUrlInput.value.trim() : '';
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/extension/src/popup.js` around lines 191 - 193, The handler
handleScanUrlSubmit is not guarded against re-entry so Enter-key calls can start
overlapping scans; add a shared lock (e.g., a module-level boolean isScanning)
or an early-return guard at the top of handleScanUrlSubmit that returns
immediately when scanning is true, set isScanning = true when a scan begins and
set it back to false in all completion/error paths, ensure the existing
scanUrlSubmit.disabled = true/false code is kept in sync with isScanning so both
button clicks and Enter-key invocations use the same lock.

if (scanResultsContent) scanResultsContent.hidden = true;

renderScanResult({
Expand All @@ -205,6 +208,9 @@
renderScanResult(result);
setScanUrlMessage('', '');
setScanSearchLoading(false);
if (scanUrlSubmit) {
scanUrlSubmit.disabled = false;
}
return;
}

Expand All @@ -214,6 +220,9 @@
if (triggerResult.status === 'error') {
setScanUrlMessage('Could not start scan. Try on the website.', 'error');
setScanSearchLoading(false);
if (scanUrlSubmit) {
scanUrlSubmit.disabled = false;
}
return;
}
if (triggerResult.status === 'completed' || triggerResult.already_scanned) {
Expand All @@ -222,11 +231,17 @@
renderScanResult(result);
setScanUrlMessage('', '');
setScanSearchLoading(false);
if (scanUrlSubmit) {
scanUrlSubmit.disabled = false;
}
});
}
setScanUrlMessage('Scan in progress…', '');
return waitForScan(extId).then(function (p2) {
setScanSearchLoading(false);
if (scanUrlSubmit) {
scanUrlSubmit.disabled = false;
}
if (p2._st === 'timeout') {
setScanUrlMessage('Scan is taking longer. Check again soon.', 'error');
renderScanResult({
Expand All @@ -249,9 +264,15 @@

setScanUrlMessage('Could not fetch results.', 'error');
setScanSearchLoading(false);
if (scanUrlSubmit) {
scanUrlSubmit.disabled = false;
}
}).catch(function () {
setScanUrlMessage('Network error. Check your connection.', 'error');
setScanSearchLoading(false);
if (scanUrlSubmit) {
scanUrlSubmit.disabled = false;
}
});
}

Expand Down Expand Up @@ -303,11 +324,16 @@
return new Promise(function (resolve) { setTimeout(resolve, ms); });
}

function esc(s) {
function esc(s) {
if (s === null || s === undefined) return '';
try {
var d = document.createElement('div');
d.textContent = s;
d.textContent = String(s);
return d.innerHTML;
} catch (e) {
return '';
}
}

function getIconUrl(ext) {
var icons = ext && ext.icons;
Expand Down Expand Up @@ -647,44 +673,37 @@
}
}

function scan(force) {
chrome.permissions.contains({ permissions: ["management"] }, function(hasPerm) {
if (!hasPerm) {
showError("Management permission required. Click Extensions tab to grant.");
render([]);
function scan(force) {
showStatus('Getting extensions…');

try {
chrome.management.getAll(function (all) {
if (chrome.runtime.lastError || !all) {
hideStatus();
showError('Cannot access extensions');
return;
}

showStatus('Getting extensions…');
var selfId = chrome.runtime.id;
var exts = [];

chrome.runtime.sendMessage({ action: 'getAllExtensions' }, function (all) {
if (chrome.runtime.lastError || !all) {
chrome.management.getAll(function (fallbackAll) {
if (chrome.runtime.lastError) {
hideStatus();
showError('Cannot access extensions: ' + (chrome.runtime.lastError.message || 'unknown'));
return;
}
var selfId = chrome.runtime.id;
var filtered = [];
for (var j = 0; j < fallbackAll.length; j++) {
if (!fallbackAll[j].permissions) fallbackAll[j].permissions = [];
if (fallbackAll[j].type === 'extension' && fallbackAll[j].id !== selfId && fallbackAll[j].enabled) filtered.push(fallbackAll[j]);
}
runScanWithExtensions(filtered, force);
});
return;
for (var i = 0; i < all.length; i++) {
if (
all[i].type === 'extension' &&
all[i].id !== selfId &&
all[i].enabled
) {
exts.push(all[i]);
}
}

var exts = [];
for (var i = 0; i < all.length; i++) {
if (!all[i].permissions) all[i].permissions = [];
if (all[i].enabled) exts.push(all[i]);
}
runScanWithExtensions(exts, force);
});
runScanWithExtensions(exts, force);
});
} catch (e) {
hideStatus();
showError('Unexpected error while scanning');
}
}
Comment on lines +676 to +706
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -e

echo "== manifest permission model =="
sed -n '1,40p' packages/extension/src/manifest.json

echo
echo "== popup startup + management API usage =="
rg -n -C3 'scan\(false\)|chrome\.management\.getAll|chrome\.permissions\.(contains|request)' \
  packages/extension/src/popup.js \
  packages/extension/src/background.js

Repository: Stanzin7/ExtensionShield

Length of output: 3415

🏁 Script executed:

sed -n '65,80p' packages/extension/src/popup.js

Repository: Stanzin7/ExtensionShield

Length of output: 573

Gate the initial scan on the optional management permission.

scan(false) runs unconditionally on popup load (line 792), but packages/extension/src/manifest.json declares "management" under optional_permissions, not required permissions. On a fresh install, chrome.management.getAll() at line 680 will fail and the popup opens directly to "Cannot access extensions" instead of prompting for permission. The button handler at lines 70–76 correctly checks chrome.permissions.contains() before requesting the permission, but the startup scan bypasses this entirely. Apply the same permission-gating pattern to the initial scan(false) call and render a permission-request state instead of treating missing permission as a scan error.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/extension/src/popup.js` around lines 676 - 706, The initial scan
should be gated by the optional "management" permission: inside scan(force)
before calling chrome.management.getAll, call
chrome.permissions.contains({permissions: ['management']}, callback) and if the
permission is not granted hideStatus() and render the permission-request state
(reuse the same UI/handler used by the existing permission button flow that
requests management) instead of calling chrome.management.getAll and showing
"Cannot access extensions"; if permissions.contains reports granted, proceed to
call chrome.management.getAll and then runScanWithExtensions(exts, force); keep
the existing try/catch for unexpected errors.

Copy link
Copy Markdown
Collaborator

@sapnilbiswas sapnilbiswas Apr 24, 2026

Choose a reason for hiding this comment

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

@Khushi5623 Please have a check on this

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 24, 2026

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


function runScanWithExtensions(exts, force) {
if (!exts || exts.length === 0) {
Expand Down Expand Up @@ -769,6 +788,7 @@
nextExt();
}

initTheme();
scan(false);
initTheme();
scan(false);
});
})();
Loading