From acaf44155c10fc5280df69ee9a4fa6bce98bb9b8 Mon Sep 17 00:00:00 2001 From: Kevin Yan Date: Sun, 28 Jun 2026 02:34:30 -0400 Subject: [PATCH] Fix 8 frontend bugs found during deep code review - Wrap async setTimeout in transitionFromSkeletonsToResults with try/catch to prevent UI stuck in SCANNING state - Fall back to startDate+1h in ICS generator when endTime parsing fails, preventing DTEND:null in .ics files - Add normalizeTimeStr() in dateTime.js to handle AM/PM time strings before ISO parsing - Remove { once: true } from settings button listener so it survives failed tab creation - Clamp end-before-start dates in calendarUrl.js resolveEndDate to startDate+1h - Rewrite ICS foldLine to use TextEncoder for byte-accurate folding per RFC 5545 - Remove racing theme IIFE from theme.js (popup.js and settings.js already handle initialization) - Track deferred poll timeouts in scanPolling.js so clearAll() can cancel them Co-Authored-By: Claude Opus 4.6 --- src/lib/calendarUrl.js | 3 +++ src/lib/icsGenerator.js | 36 +++++++++++++++++++++++++++--------- src/popup.js | 35 ++++++++++++++++++----------------- src/popup/dateTime.js | 16 +++++++++++++++- src/popup/scanPolling.js | 9 +++++++++ src/ui/theme.js | 30 ------------------------------ 6 files changed, 72 insertions(+), 57 deletions(-) diff --git a/src/lib/calendarUrl.js b/src/lib/calendarUrl.js index 3cc5bb5..5b64120 100644 --- a/src/lib/calendarUrl.js +++ b/src/lib/calendarUrl.js @@ -49,6 +49,9 @@ export function buildCalendarCreateUrl(event) { if (isAllDay) { return addDays(startDate, 1); } + if (!explicitEnd || explicitEnd <= startDate) { + return new Date(startDate.getTime() + 60 * 60 * 1000); + } return explicitEnd; } diff --git a/src/lib/icsGenerator.js b/src/lib/icsGenerator.js index 31d458b..d10c75e 100644 --- a/src/lib/icsGenerator.js +++ b/src/lib/icsGenerator.js @@ -55,18 +55,35 @@ function escapeICS(text) { .replace(/\r/g, ''); } -// Helper to fold lines longer than 75 characters (per RFC 5545) +// Helper to fold lines longer than 75 octets (per RFC 5545) function foldLine(line) { - if (line.length <= 75) return line; + const encoder = typeof TextEncoder !== 'undefined' ? new TextEncoder() : null; + + if (!encoder || encoder.encode(line).length <= 75) return line; const folded = []; - let currentLine = line; + let remaining = line; + + while (remaining.length > 0) { + const maxChars = folded.length === 0 ? 75 : 74; + let cutPoint = remaining.length; + + for (let i = Math.min(cutPoint, maxChars); i > 0; i--) { + const chunk = remaining.substring(0, i); + if (encoder.encode(chunk).length <= maxChars) { + cutPoint = i; + break; + } + } - while (currentLine.length > 75) { - folded.push(currentLine.substring(0, 75)); - currentLine = ' ' + currentLine.substring(75); + if (cutPoint === 0) cutPoint = 1; + + folded.push(remaining.substring(0, cutPoint)); + remaining = remaining.substring(cutPoint); + if (remaining.length > 0) { + remaining = ' ' + remaining; + } } - folded.push(currentLine); return folded.join(CRLF); } @@ -114,9 +131,10 @@ function generateVEVENT(event) { lines.push(`DTSTART;VALUE=DATE:${formatICSAllDayDate(startDate)}`); lines.push(`DTEND;VALUE=DATE:${formatICSAllDayDate(endDate)}`); } else { - const endDate = event.endTime + const parsedEnd = event.endTime ? toDate(event.endDate || event.startDate, event.endTime) - : addHours(startDate, 1); + : null; + const endDate = parsedEnd || addHours(startDate, 1); lines.push(`DTSTART:${formatICSDateTime(startDate)}`); lines.push(`DTEND:${formatICSDateTime(endDate)}`); } diff --git a/src/popup.js b/src/popup.js index 84382a1..e8a62b4 100644 --- a/src/popup.js +++ b/src/popup.js @@ -355,16 +355,12 @@ function showSkeletonCards(count = 3) { } // Ensure Settings button works immediately on popup load -settingsBtn?.addEventListener( - "click", - () => { - try { - const url = chrome.runtime.getURL("src/ui/settings.html"); - chrome.tabs.create({ url }); - } catch (_) { } - }, - { once: true } -); +settingsBtn?.addEventListener("click", () => { + try { + const url = chrome.runtime.getURL("src/ui/settings.html"); + chrome.tabs.create({ url }); + } catch (_) { } +}); // Quota exceeded panel - link to settings with BYOK section const quotaSettingsBtn = document.getElementById("quotaSettingsBtn"); @@ -547,14 +543,19 @@ function transitionFromSkeletonsToResults(events) { // Wait for fade out, then render event cards setTimeout(async () => { - upcomingEventsListEl.innerHTML = ""; - if (pastEventsListEl) pastEventsListEl.innerHTML = ""; - await renderEvents(events); + try { + upcomingEventsListEl.innerHTML = ""; + if (pastEventsListEl) pastEventsListEl.innerHTML = ""; + await renderEvents(events); - // Transition to results loaded state - // This removes 'scanning' class and adds 'has-results' - // Button appears smoothly via CSS transitions - setState(UI_STATE.RESULTS_LOADED); + // Transition to results loaded state + // This removes 'scanning' class and adds 'has-results' + // Button appears smoothly via CSS transitions + setState(UI_STATE.RESULTS_LOADED); + } catch (e) { + error("Failed to render events:", e); + setState(UI_STATE.IDLE); + } }, SKELETON_FADE_DURATION_MS + skeletons.length * SKELETON_STAGGER_DELAY_MS); } diff --git a/src/popup/dateTime.js b/src/popup/dateTime.js index 495a3ae..b137af7 100644 --- a/src/popup/dateTime.js +++ b/src/popup/dateTime.js @@ -67,10 +67,24 @@ export function isEventPast(event, { now = new Date(), warn = () => { }, error = } } +function normalizeTimeStr(timeStr) { + if (!timeStr) return null; + const match = timeStr.match(/(\d{1,2})(?::(\d{2}))?(?::(\d{2}))?\s*([AP]M)/i); + if (!match) return timeStr; + let hours = parseInt(match[1], 10); + const minutes = match[2] ? match[2] : "00"; + const seconds = match[3] ? match[3] : "00"; + const meridiem = match[4].toUpperCase(); + if (meridiem === "PM" && hours !== 12) hours += 12; + else if (meridiem === "AM" && hours === 12) hours = 0; + return `${String(hours).padStart(2, "0")}:${minutes}:${seconds}`; +} + export function formatDateTime(dateStr, timeStr, timeFormat = "12") { try { if (!dateStr) return ""; - const iso = `${dateStr}${timeStr ? `T${timeStr}` : ""}`; + const normalizedTime = normalizeTimeStr(timeStr); + const iso = `${dateStr}${normalizedTime ? `T${normalizedTime}` : ""}`; const d = new Date(iso); if (Number.isNaN(d.getTime())) { return `${dateStr}${timeStr ? ` ${timeStr}` : ""}`; diff --git a/src/popup/scanPolling.js b/src/popup/scanPolling.js index c3f6ae0..26672a7 100644 --- a/src/popup/scanPolling.js +++ b/src/popup/scanPolling.js @@ -27,6 +27,8 @@ export function createScanPoller({ } = {}) { const pollTimeouts = new Map(); + let deferredTimeoutId = null; + async function pollForResults(url, storageKey, onComplete, onError, options = {}) { let currentInterval = SCAN_POLL_INITIAL_INTERVAL_MS; let attempts = 0; @@ -35,6 +37,7 @@ export function createScanPoller({ const cleanup = () => { if (url) pollTimeouts.delete(url); + else deferredTimeoutId = null; }; const poll = async () => { @@ -109,6 +112,7 @@ export function createScanPoller({ const timeoutId = setTimeoutFn(poll, currentInterval); if (url) pollTimeouts.set(url, timeoutId); + else deferredTimeoutId = timeoutId; } catch (err) { error(err); cleanup(); @@ -118,6 +122,7 @@ export function createScanPoller({ const initialTimeoutId = setTimeoutFn(poll, currentInterval); if (url) pollTimeouts.set(url, initialTimeoutId); + else deferredTimeoutId = initialTimeoutId; } function clearForUrl(url) { @@ -131,6 +136,10 @@ export function createScanPoller({ if (timeout) clearTimeoutFn(timeout); } pollTimeouts.clear(); + if (deferredTimeoutId) { + clearTimeoutFn(deferredTimeoutId); + deferredTimeoutId = null; + } } return { diff --git a/src/ui/theme.js b/src/ui/theme.js index 203c855..f06dd25 100644 --- a/src/ui/theme.js +++ b/src/ui/theme.js @@ -27,33 +27,3 @@ export function applyTheme(theme) { }); }); } - -// Self-executing function to load theme immediately -// This replaces the separate theme-loader.js file -(async () => { - if (typeof chrome === 'undefined' || !chrome.storage) return; - - try { - const result = await chrome.storage.sync.get("settings"); - const settings = result.settings || {}; - const theme = settings.darkModeSettings || "auto"; - - const body = document.body; - if (body) { - body.classList.remove("light-mode", "dark-mode", "auto-mode"); - body.classList.add(theme === "light" ? "light-mode" : theme === "dark" ? "dark-mode" : "auto-mode"); - } - } catch (error) { - console.error("Error loading theme:", error); - if (document.body) { - document.body.classList.add("auto-mode"); - } - } finally { - if (document.body) { - document.body.classList.remove("theme-loading"); - requestAnimationFrame(() => { - document.body.classList.add("theme-loaded"); - }); - } - } -})();