From 491260e7260ae4ed49a85c6a94b2ab81d0f1792f Mon Sep 17 00:00:00 2001 From: Junaid Ahmed Date: Sun, 9 Aug 2026 02:56:46 +0500 Subject: [PATCH 01/38] refactor: dedupe shared helpers, delete dead code, fix hot paths across codebase --- entrypoints/alfred-main-world.ts | 287 +++++---------- .../appstore-compare-tray.content/buttons.ts | 48 ++- .../appstore-compare.content/App.svelte | 16 +- .../appstore-compare.content/exporters.ts | 9 +- entrypoints/appstore-compare.content/index.ts | 6 +- .../appstore-partners.content/App.svelte | 5 +- .../appstore-partners.content/index.ts | 8 - .../appstore-partners.content/types.ts | 16 - .../appstore-partners.content/utils.ts | 141 +++----- entrypoints/appstore-search.content.ts | 62 ++-- entrypoints/background/index.ts | 58 +-- entrypoints/background/shortcuts.ts | 342 +++++++----------- .../components/ItemsTab.svelte | 11 +- .../collaborator-access.content/App.svelte | 98 ++--- .../collaborator-access.content/index.ts | 4 - .../collaborator-access.content/presets.ts | 14 +- entrypoints/dev-dashboard.content.ts | 28 +- entrypoints/main.content.ts | 161 ++++----- entrypoints/options/App.svelte | 3 +- .../components/settings/AdminSettings.svelte | 30 +- entrypoints/popup/App.svelte | 85 ++--- entrypoints/popup/Assets.svelte | 47 +-- entrypoints/popup/Headings.svelte | 40 +- entrypoints/popup/Hreflangs.svelte | 61 ++-- entrypoints/popup/Images.svelte | 53 +-- entrypoints/popup/Links.svelte | 46 +-- entrypoints/popup/Overview.svelte | 100 +---- entrypoints/popup/Robots.svelte | 46 +-- entrypoints/popup/Schema.svelte | 70 +--- entrypoints/popup/Sitemaps.svelte | 110 ++---- entrypoints/popup/Social.svelte | 44 +-- entrypoints/popup/Theme.svelte | 239 +----------- entrypoints/popup/tests/overview.test.ts | 74 +--- entrypoints/popup/utils/copy.svelte.ts | 54 +++ entrypoints/popup/utils/format.ts | 13 +- entrypoints/popup/utils/hreflang.ts | 15 +- entrypoints/popup/utils/messaging.ts | 22 +- entrypoints/popup/utils/overview.ts | 50 +-- entrypoints/popup/utils/theme.ts | 6 +- entrypoints/popup/utils/track.svelte.ts | 18 + entrypoints/popup/utils/types.ts | 11 - entrypoints/popup/utils/url.ts | 16 + .../shopify-admin.content/ToggleSidebar.ts | 51 +-- entrypoints/shopify-admin.content/index.ts | 7 +- .../tests/timeline.test.ts | 17 +- .../shopify-admin.content/timeline.logic.ts | 35 +- .../shopify-admin.content/timeline.util.ts | 4 +- .../theme-customizer.content/resizers.util.ts | 27 +- global.d.ts | 13 +- supabase/functions/track/index.ts | 10 +- utils/analytics.ts | 17 +- utils/compareTray.ts | 7 + utils/contextMenu.ts | 86 +---- utils/export.ts | 42 +++ utils/helpers.ts | 63 +--- utils/restore-right-click.ts | 4 +- utils/shopify.ts | 7 +- utils/toast.ts | 27 +- 58 files changed, 984 insertions(+), 2000 deletions(-) create mode 100644 entrypoints/popup/utils/copy.svelte.ts create mode 100644 entrypoints/popup/utils/track.svelte.ts create mode 100644 entrypoints/popup/utils/url.ts create mode 100644 utils/export.ts diff --git a/entrypoints/alfred-main-world.ts b/entrypoints/alfred-main-world.ts index 248e41b..e0fc746 100644 --- a/entrypoints/alfred-main-world.ts +++ b/entrypoints/alfred-main-world.ts @@ -6,6 +6,36 @@ export default defineUnlistedScript(() => { // Settings will be received via postMessage let settings: AlfredSettings = {}; + const alfredWin = () => window as unknown as WindowWithAlfred; + + /** Dispatches an analytics event for the content script to relay. */ + const track = (action: string, metadata?: Record) => { + window.dispatchEvent(new CustomEvent('alfred:track', { detail: metadata ? { action, metadata } : { action } })); + }; + + /** Metadata shared by every tracked action. */ + const pageMetadata = (pageType?: string) => ({ + page_url: window.location.href, + page_type: pageType ?? alfredWin().__st?.p ?? 'other', + shop_domain: window.location.hostname + }); + + /** Guard preamble shared by every action: toast + false when not on a Shopify store. */ + const requireShopify = (): boolean => { + if (!alfredWin().Alfred.isShopify()) { + Toast.error('Not a Shopify store'); + return false; + } + return true; + }; + + /** Shared catch tail: log the error, toast the user, signal failure. */ + const handleError = (error: unknown, logMsg: string, toastMsg: string): false => { + console.error(logMsg, error); + Toast.error(toastMsg); + return false; + }; + // Define alfred utils in the global scope (window as unknown as WindowWithAlfred).Alfred = { // Settings @@ -206,11 +236,7 @@ export default defineUnlistedScript(() => { openInAdmin: (): boolean => { try { const win = window as unknown as WindowWithAlfred; - // Check if this is a Shopify store - if (!win.Alfred.isShopify()) { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Not a Shopify store'); - return false; - } + if (!requireShopify()) return false; const shopName = win.Alfred.getShopName(); const { p, rid } = win.__st ?? {}; @@ -221,33 +247,18 @@ export default defineUnlistedScript(() => { } else if (p && rid && ['product', 'collection', 'page', 'article'].includes(p)) { url = `https://admin.shopify.com/store/${shopName}/${p}s/${rid}`; } else { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Page type not supported'); + Toast.error('Page type not supported'); return false; } - (win.Alfred.Toast as { success: (msg: string) => void }).success('Opening admin...'); + Toast.success('Opening admin...'); window.open(url, '_blank'); - // Track the action - window.dispatchEvent( - new CustomEvent('alfred:track', { - detail: { - action: 'open_in_admin', - metadata: { - page_url: window.location.href, - page_type: p || 'other', - shop_domain: window.location.hostname - } - } - }) - ); + track('open_in_admin', pageMetadata(p || 'other')); return true; } catch (error) { - console.error('Error opening in admin:', error); - const win = window as unknown as WindowWithAlfred; - (win.Alfred.Toast as { error: (msg: string) => void }).error('Failed to open admin'); - return false; + return handleError(error, 'Error opening in admin:', 'Failed to open admin'); } }, @@ -258,11 +269,7 @@ export default defineUnlistedScript(() => { openInCustomizer: (): boolean => { try { const win = window as unknown as WindowWithAlfred; - // Check if this is a Shopify store - if (!win.Alfred.isShopify()) { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Not a Shopify store'); - return false; - } + if (!requireShopify()) return false; const themeId = win.Shopify?.theme?.id; const shopName = win.Alfred.getShopName(); @@ -274,29 +281,14 @@ export default defineUnlistedScript(() => { ? `https://admin.shopify.com/store/${shopName}/themes/${themeId}/editor?previewPath=${previewPath}` : `https://admin.shopify.com/store/${shopName}/themes/${themeId}/editor`; - (win.Alfred.Toast as { success: (msg: string) => void }).success('Opening customizer...'); + Toast.success('Opening customizer...'); window.open(customizerUrl, '_blank'); - // Track the action - window.dispatchEvent( - new CustomEvent('alfred:track', { - detail: { - action: 'open_in_customizer', - metadata: { - page_url: window.location.href, - page_type: win.__st?.p ?? 'other', - shop_domain: window.location.hostname - } - } - }) - ); + track('open_in_customizer', pageMetadata()); return true; } catch (error) { - console.error('Error opening in customizer:', error); - const win = window as unknown as WindowWithAlfred; - (win.Alfred.Toast as { error: (msg: string) => void }).error('Failed to open in customizer'); - return false; + return handleError(error, 'Error opening in customizer:', 'Failed to open in customizer'); } }, @@ -307,15 +299,11 @@ export default defineUnlistedScript(() => { copyProductJson: async (): Promise => { try { const win = window as unknown as WindowWithAlfred; - // Check if this is a Shopify store - if (!win.Alfred.isShopify()) { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Not a Shopify store'); - return false; - } + if (!requireShopify()) return false; // Check if this is a product page if (!window.location.pathname.includes('/products/')) { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Not a product page'); + Toast.error('Not a product page'); return false; } @@ -326,7 +314,7 @@ export default defineUnlistedScript(() => { const response = await fetch(jsonUrl); if (!response.ok) { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Failed to fetch product data'); + Toast.error('Failed to fetch product data'); return false; } @@ -335,30 +323,15 @@ export default defineUnlistedScript(() => { // If successful, show toast and dispatch event for tracking if (copiedToClipboard) { - (win.Alfred.Toast as { success: (msg: string) => void }).success('Product JSON copied'); - - window.dispatchEvent( - new CustomEvent('alfred:track', { - detail: { - action: 'copy_product_json', - metadata: { - page_url: window.location.href, - page_type: 'product', - shop_domain: window.location.hostname - } - } - }) - ); + Toast.success('Product JSON copied'); + track('copy_product_json', pageMetadata('product')); } else { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Failed to copy product JSON'); + Toast.error('Failed to copy product JSON'); } return copiedToClipboard; } catch (error) { - console.error('Error copying product JSON:', error); - const win = window as unknown as WindowWithAlfred; - (win.Alfred.Toast as { error: (msg: string) => void }).error('Failed to copy product JSON'); - return false; + return handleError(error, 'Error copying product JSON:', 'Failed to copy product JSON'); } }, @@ -369,17 +342,13 @@ export default defineUnlistedScript(() => { copyCartJson: async (): Promise => { try { const win = window as unknown as WindowWithAlfred; - // Check if this is a Shopify store - if (!win.Alfred.isShopify()) { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Not a Shopify store'); - return false; - } + if (!requireShopify()) return false; // Fetch cart data using Shopify's cart.js API const response = await fetch('/cart.js'); if (!response.ok) { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Failed to fetch cart data'); + Toast.error('Failed to fetch cart data'); return false; } @@ -388,30 +357,15 @@ export default defineUnlistedScript(() => { // If successful, show toast and dispatch event for tracking if (copiedToClipboard) { - (win.Alfred.Toast as { success: (msg: string) => void }).success('Cart JSON copied'); - - window.dispatchEvent( - new CustomEvent('alfred:track', { - detail: { - action: 'copy_cart_json', - metadata: { - page_url: window.location.href, - page_type: (window as unknown as WindowWithAlfred).__st?.p ?? 'other', - shop_domain: window.location.hostname - } - } - }) - ); + Toast.success('Cart JSON copied'); + track('copy_cart_json', pageMetadata()); } else { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Failed to copy cart JSON'); + Toast.error('Failed to copy cart JSON'); } return copiedToClipboard; } catch (error) { - console.error('Error copying cart JSON:', error); - const win = window as unknown as WindowWithAlfred; - (win.Alfred.Toast as { error: (msg: string) => void }).error('Failed to copy cart JSON'); - return false; + return handleError(error, 'Error copying cart JSON:', 'Failed to copy cart JSON'); } }, @@ -423,11 +377,7 @@ export default defineUnlistedScript(() => { copyThemePreviewUrl: async (disablePreviewBar = false): Promise => { try { const win = window as unknown as WindowWithAlfred; - // Check if this is a Shopify store - if (!win.Alfred.isShopify()) { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Not a Shopify store'); - return false; - } + if (!requireShopify()) return false; const themeId = win.Shopify?.theme?.id ?? ''; const url = new URL(window.location.href); @@ -444,32 +394,19 @@ export default defineUnlistedScript(() => { // If successful, show toast and dispatch event for tracking if (copiedToClipboard) { - (win.Alfred.Toast as { success: (msg: string) => void }).success('Preview URL copied!'); - - window.dispatchEvent( - new CustomEvent('alfred:track', { - detail: { - action: 'copy_theme_preview_url', - metadata: { - page_url: window.location.href, - page_type: win.__st?.p ?? 'other', - shop_domain: window.location.hostname, - disable_preview_bar: disablePreviewBar, - source: 'context_menu' - } - } - }) - ); + Toast.success('Preview URL copied!'); + track('copy_theme_preview_url', { + ...pageMetadata(), + disable_preview_bar: disablePreviewBar, + source: 'context_menu' + }); } else { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Failed to copy theme preview URL'); + Toast.error('Failed to copy theme preview URL'); } return copiedToClipboard; } catch (error) { - console.error('Error copying preview URL:', error); - const win = window as unknown as WindowWithAlfred; - (win.Alfred.Toast as { error: (msg: string) => void }).error('Failed to copy theme preview URL'); - return false; + return handleError(error, 'Error copying preview URL:', 'Failed to copy theme preview URL'); } }, @@ -480,29 +417,19 @@ export default defineUnlistedScript(() => { exitThemePreview: (): boolean => { try { const win = window as unknown as WindowWithAlfred; - // Check if this is a Shopify store - if (!win.Alfred.isShopify()) { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Not a Shopify store'); - return false; - } + if (!requireShopify()) return false; // Check if the current theme is not the main/published theme const role = win.Shopify?.theme?.role; if (role === 'main') { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Not previewing a theme'); + Toast.error('Not previewing a theme'); return false; } - (win.Alfred.Toast as { success: (msg: string) => void }).success('Exiting theme preview...'); + Toast.success('Exiting theme preview...'); // Track the action before navigation - window.dispatchEvent( - new CustomEvent('alfred:track', { - detail: { - action: 'exit_theme_preview' - } - }) - ); + track('exit_theme_preview'); // Navigate to the current URL with an empty preview_theme_id to clear the preview const url = new URL(window.location.href); @@ -511,10 +438,7 @@ export default defineUnlistedScript(() => { return true; } catch (error) { - console.error('Error exiting theme preview:', error); - const win = window as unknown as WindowWithAlfred; - (win.Alfred.Toast as { error: (msg: string) => void }).error('Failed to exit theme preview'); - return false; + return handleError(error, 'Error exiting theme preview:', 'Failed to exit theme preview'); } }, @@ -524,47 +448,28 @@ export default defineUnlistedScript(() => { */ clearCart: async (): Promise => { try { - const win = window as unknown as WindowWithAlfred; - // Check if this is a Shopify store - if (!win.Alfred.isShopify()) { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Not a Shopify store'); - return false; - } + if (!requireShopify()) return false; - (win.Alfred.Toast as { success: (msg: string) => void }).success('Clearing cart...'); + Toast.success('Clearing cart...'); // Clear cart const response = await fetch('/cart/clear'); if (!response.ok) { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Failed to clear cart'); + Toast.error('Failed to clear cart'); return false; } - (win.Alfred.Toast as { success: (msg: string) => void }).success('Cart cleared'); + Toast.success('Cart cleared'); // Track the action before reload - window.dispatchEvent( - new CustomEvent('alfred:track', { - detail: { - action: 'clear_cart', - metadata: { - page_url: window.location.href, - page_type: win.__st?.p ?? 'other', - shop_domain: window.location.hostname - } - } - }) - ); + track('clear_cart', pageMetadata()); // Reload the page to reflect the empty cart window.location.reload(); return true; } catch (error) { - console.error('Error clearing cart:', error); - const win = window as unknown as WindowWithAlfred; - (win.Alfred.Toast as { error: (msg: string) => void }).error('Failed to clear cart'); - return false; + return handleError(error, 'Error clearing cart:', 'Failed to clear cart'); } }, @@ -575,32 +480,28 @@ export default defineUnlistedScript(() => { openSectionInCodeEditor: (): boolean => { try { const win = window as unknown as WindowWithAlfred; - // Check if this is a Shopify store - if (!win.Alfred.isShopify()) { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Not a Shopify store'); - return false; - } + if (!requireShopify()) return false; // Use the last right-clicked element const target = win.Alfred._lastRightClickedElement; if (!target) { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Invalid section'); + Toast.error('Invalid section'); return false; } // Find the parent section element - const sectionElement = target.closest('.shopify-section')!; + const sectionElement = target.closest('.shopify-section'); if (!sectionElement) { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Invalid section'); + Toast.error('Invalid section'); return false; } // Extract section ID const sectionId = sectionElement.id; if (!sectionId?.includes('__')) { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Invalid section'); + Toast.error('Invalid section'); return false; } @@ -610,7 +511,7 @@ export default defineUnlistedScript(() => { // 2. "shopify-section-template--__image_banner_zBNR7B" -> "image_banner" const parts = sectionId.split('__'); if (parts.length < 2) { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Section not recognized'); + Toast.error('Section not recognized'); return false; } @@ -618,12 +519,12 @@ export default defineUnlistedScript(() => { let sectionName = parts[1]; if (!sectionName) { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Section not recognized'); + Toast.error('Section not recognized'); return false; } // Check if there are multiple sections with the same base pattern - const basePattern = sectionId.split('__')[0] + '__'; + const basePattern = parts[0] + '__'; const allSections = document.querySelectorAll(`[id^="${basePattern}"]`); if (allSections.length > 1) { @@ -634,11 +535,6 @@ export default defineUnlistedScript(() => { } } - if (!sectionName) { - (win.Alfred.Toast as { error: (msg: string) => void }).error('Section not recognized'); - return false; - } - sectionName = sectionName.replace(/_/g, '-'); // If section name is "main", concatenate with the current page's ptype @@ -657,28 +553,13 @@ export default defineUnlistedScript(() => { // Open in new tab window.open(editorUrl, '_blank'); - (win.Alfred.Toast as { success: (msg: string) => void }).success(`Opening ${sectionName}.liquid`); - - // Track the action - window.dispatchEvent( - new CustomEvent('alfred:track', { - detail: { - action: 'open_section_in_code_editor', - metadata: { - page_url: window.location.href, - page_type: win.__st?.p ?? 'other', - shop_domain: window.location.hostname - } - } - }) - ); + Toast.success(`Opening ${sectionName}.liquid`); + + track('open_section_in_code_editor', pageMetadata()); return true; } catch (error) { - console.error('Error opening section in editor:', error); - const win = window as unknown as WindowWithAlfred; - (win.Alfred.Toast as { error: (msg: string) => void }).error('Failed to open section in editor'); - return false; + return handleError(error, 'Error opening section in editor:', 'Failed to open section in editor'); } } }; diff --git a/entrypoints/appstore-compare-tray.content/buttons.ts b/entrypoints/appstore-compare-tray.content/buttons.ts index 5ab46ad..7579dec 100644 --- a/entrypoints/appstore-compare-tray.content/buttons.ts +++ b/entrypoints/appstore-compare-tray.content/buttons.ts @@ -1,5 +1,5 @@ import { sendTrackEvent } from '@/utils/analytics'; -import { addToTray, removeFromTray, getTray, watchTray, COMPARE_TRAY_LIMIT } from '~/utils/compareTray'; +import { addToTray, removeFromTray, getTray, watchTray, COMPARE_TRAY_LIMIT, isAppHandle } from '~/utils/compareTray'; import { Toast } from '~/utils/toast'; const BUTTON_CLASS = 'alfred-compare-button'; @@ -237,17 +237,18 @@ function injectListingButton() { } // Only individual listings live at a single-segment path like /judgeme - const match = window.location.pathname.match(/^\/([a-z0-9][a-z0-9_-]*)$/); + const segment = window.location.pathname.replace(/^\//, ''); + const handle = !segment.includes('/') && isAppHandle(segment) ? segment : null; const h1 = hero.querySelector('h1'); - if (!match?.[1] || !h1) { + if (!handle || !h1) { return; } const button = createButton( { - handle: match[1], - name: h1.textContent?.trim() ?? match[1], + handle, + name: h1.textContent?.trim() ?? handle, iconUrl: getListingIconUrl() }, () => hero.querySelector('img') @@ -266,6 +267,15 @@ export function initCompareButtons(): () => void { }; let observer: MutationObserver | undefined; + // One pending pass at a time: a mutation burst coalesces into a single run. + let pendingInjectTimeout: number | undefined; + const scheduleInjectAll = () => { + clearTimeout(pendingInjectTimeout); + pendingInjectTimeout = window.setTimeout(() => { + pendingInjectTimeout = undefined; + injectAll(); + }, 100); + }; const unwatch = watchTray((items) => { trayHandles = new Set(items.map((item) => item.handle)); @@ -278,19 +288,20 @@ export function initCompareButtons(): () => void { refreshButtons(); observer = new MutationObserver((mutations) => { - const hasNewCards = mutations.some( - (mutation) => - mutation.type === 'childList' && - Array.from(mutation.addedNodes).some( - (node) => - node.nodeType === Node.ELEMENT_NODE && - ((node as Element).matches('[data-controller="app-card"]') || - (node as Element).querySelector('[data-controller="app-card"]') !== null) - ) - ); - - if (hasNewCards) { - setTimeout(injectAll, 100); + for (const mutation of mutations) { + if (mutation.type !== 'childList') { + continue; + } + for (const node of mutation.addedNodes) { + if ( + node.nodeType === Node.ELEMENT_NODE && + ((node as Element).matches('[data-controller="app-card"]') || + (node as Element).querySelector('[data-controller="app-card"]') !== null) + ) { + scheduleInjectAll(); + return; + } + } } }); @@ -298,6 +309,7 @@ export function initCompareButtons(): () => void { }); return () => { + clearTimeout(pendingInjectTimeout); observer?.disconnect(); unwatch(); document.getElementById(STYLE_ID)?.remove(); diff --git a/entrypoints/appstore-compare.content/App.svelte b/entrypoints/appstore-compare.content/App.svelte index 5468975..ea9ee73 100644 --- a/entrypoints/appstore-compare.content/App.svelte +++ b/entrypoints/appstore-compare.content/App.svelte @@ -7,6 +7,7 @@ import { cornerStack } from '~/utils/cornerStack'; import { withCredit } from '~/utils/credit'; import { Toast } from '~/utils/toast'; + import { downloadFile } from '~/utils/export'; import { buildComparisonCsv, buildComparisonJson, buildComparisonMarkdown, COMPARISON_ROWS } from './exporters'; type Column = { @@ -113,31 +114,20 @@ } } - function downloadFile(filename: string, content: string, type: string) { - const url = URL.createObjectURL(new Blob([content], { type })); - const link = document.createElement('a'); - link.href = url; - link.download = filename; - document.body.appendChild(link); - link.click(); - link.remove(); - URL.revokeObjectURL(url); - } - function exportFilename(extension: string): string { return `shopify-alfred-compare-${new Date().toISOString().split('T')[0]}.${extension}`; } function downloadCsv() { closeExportMenu(); - downloadFile(exportFilename('csv'), withCsvCredit(buildComparisonCsv(loadedListings)), 'text/csv;charset=utf-8;'); + downloadFile(withCsvCredit(buildComparisonCsv(loadedListings)), exportFilename('csv'), 'text/csv;charset=utf-8;'); Toast.success('Comparison downloaded as CSV'); sendTrackEvent('compare_export_csv', { app_count: loadedListings.length }); } function downloadJson() { closeExportMenu(); - downloadFile(exportFilename('json'), buildComparisonJson(loadedListings), 'application/json'); + downloadFile(buildComparisonJson(loadedListings), exportFilename('json'), 'application/json'); Toast.success('Comparison downloaded as JSON'); sendTrackEvent('compare_export_json', { app_count: loadedListings.length }); } diff --git a/entrypoints/appstore-compare.content/exporters.ts b/entrypoints/appstore-compare.content/exporters.ts index c4bba2a..9e8d0a9 100644 --- a/entrypoints/appstore-compare.content/exporters.ts +++ b/entrypoints/appstore-compare.content/exporters.ts @@ -1,4 +1,5 @@ import { formatAppAge } from '~/utils/appListing'; +import { csvField } from '~/utils/export'; function escapeCell(value: string): string { return value.replace(/\|/g, '\\|').replace(/\n/g, ' '); @@ -88,10 +89,6 @@ export function buildComparisonMarkdown(listings: AppListing[]): string { ].join('\n'); } -function csvCell(value: string): string { - return `"${value.replace(/"/g, '""')}"`; -} - /** Flatten the renderers' markdown links for plain-text formats: [label](url) -> label (url) */ function plainText(value: string): string { return value.replace(/\[([^\]]*)\]\(([^)]*)\)/g, '$1 ($2)'); @@ -103,10 +100,10 @@ function plainText(value: string): string { */ export function buildComparisonCsv(listings: AppListing[]): string { const header = ['Attribute', ...listings.map((l) => l.name ?? l.handle)]; - const lines = [header.map(csvCell).join(',')]; + const lines = [header.map(csvField).join(',')]; for (const [label, render] of COMPARISON_ROWS) { - lines.push([label, ...listings.map((l) => plainText(render(l)))].map(csvCell).join(',')); + lines.push([label, ...listings.map((l) => plainText(render(l)))].map(csvField).join(',')); } return lines.join('\n'); diff --git a/entrypoints/appstore-compare.content/index.ts b/entrypoints/appstore-compare.content/index.ts index 8113473..0011f39 100644 --- a/entrypoints/appstore-compare.content/index.ts +++ b/entrypoints/appstore-compare.content/index.ts @@ -1,12 +1,10 @@ import { createIntegratedUi } from '#imports'; import { mount, unmount } from 'svelte'; import { getItem } from '~/utils/storage'; -import { COMPARE_TRAY_LIMIT } from '~/utils/compareTray'; +import { COMPARE_TRAY_LIMIT, isAppHandle } from '~/utils/compareTray'; import App from './App.svelte'; import './style.css'; -const HANDLE_PATTERN = /^[a-z0-9][a-z0-9_-]*$/; - /** Lift the pre-paint curtain from style.css (shows whatever main holds). */ function revealMain() { document.documentElement.classList.add('alfred-compare-ready'); @@ -30,7 +28,7 @@ export default defineContentScript({ .replace(/^\/compare\//, '') .split(',') .map((handle) => decodeURIComponent(handle).trim().toLowerCase()) - .filter((handle) => HANDLE_PATTERN.test(handle)) + .filter(isAppHandle) ) ].slice(0, COMPARE_TRAY_LIMIT); diff --git a/entrypoints/appstore-partners.content/App.svelte b/entrypoints/appstore-partners.content/App.svelte index 50a4ec8..098e00f 100644 --- a/entrypoints/appstore-partners.content/App.svelte +++ b/entrypoints/appstore-partners.content/App.svelte @@ -1,6 +1,7 @@ +{#snippet checkmark(checked: boolean, onchange: (e: Event) => void, extraClass: string = '')} + + + + +{/snippet} + {#if showHotlinkModal}
Note: Renaming the preset changes the handle and breaks existing hotlinks.

@@ -498,7 +473,7 @@ - {formatBytes(img.size)} + {img.size ? formatSize(img.size) : '—'} {img.format ? img.format.toUpperCase() : '—'} {dimsLabel(img)} diff --git a/entrypoints/popup/Links.svelte b/entrypoints/popup/Links.svelte index 93a0e16..2244a09 100644 --- a/entrypoints/popup/Links.svelte +++ b/entrypoints/popup/Links.svelte @@ -1,27 +1,25 @@ {#if schema.length === 0} @@ -161,8 +129,8 @@
JSON-LD structured data
-
- - - - - - - - - diff --git a/entrypoints/popup/ReviewPrompt.svelte b/entrypoints/popup/components/ReviewPrompt.svelte similarity index 100% rename from entrypoints/popup/ReviewPrompt.svelte rename to entrypoints/popup/components/ReviewPrompt.svelte diff --git a/entrypoints/popup/components/SearchField.svelte b/entrypoints/popup/components/SearchField.svelte new file mode 100644 index 0000000..70eace1 --- /dev/null +++ b/entrypoints/popup/components/SearchField.svelte @@ -0,0 +1,44 @@ + + +
+ + + {#if value} + + {/if} + {@render trailing?.()} +
+ + diff --git a/entrypoints/popup/SegmentedControl.svelte b/entrypoints/popup/components/SegmentedControl.svelte similarity index 100% rename from entrypoints/popup/SegmentedControl.svelte rename to entrypoints/popup/components/SegmentedControl.svelte diff --git a/entrypoints/popup/SerpPreview.svelte b/entrypoints/popup/components/SerpPreview.svelte similarity index 96% rename from entrypoints/popup/SerpPreview.svelte rename to entrypoints/popup/components/SerpPreview.svelte index a51c927..e9b0df4 100644 --- a/entrypoints/popup/SerpPreview.svelte +++ b/entrypoints/popup/components/SerpPreview.svelte @@ -1,6 +1,6 @@ + + + + diff --git a/entrypoints/popup/SuccessNudge.svelte b/entrypoints/popup/components/SuccessNudge.svelte similarity index 100% rename from entrypoints/popup/SuccessNudge.svelte rename to entrypoints/popup/components/SuccessNudge.svelte diff --git a/entrypoints/popup/SummaryBar.svelte b/entrypoints/popup/components/SummaryBar.svelte similarity index 100% rename from entrypoints/popup/SummaryBar.svelte rename to entrypoints/popup/components/SummaryBar.svelte diff --git a/entrypoints/popup/components/TableToolbar.svelte b/entrypoints/popup/components/TableToolbar.svelte new file mode 100644 index 0000000..c3eb218 --- /dev/null +++ b/entrypoints/popup/components/TableToolbar.svelte @@ -0,0 +1,151 @@ + + + + + { if (e.key === 'Escape') openMenu = null; }} /> + +
+
+
+ {#each facets as facet (facet.key)} +
+ + {#if openMenu === facet.key} + + {/if} +
+ {/each} + {#if anyFilterActive && onReset} + + {/if} +
+
+ {@render actions?.()} + + + + {#if exportItems.length > 0} + { openMenu = openMenu === 'export' ? null : 'export'; }} + onClose={() => { openMenu = null; }} + /> + {/if} +
+
+ {#if searchOpen} + + {/if} +
+ + diff --git a/entrypoints/popup/components/ToolbarButton.svelte b/entrypoints/popup/components/ToolbarButton.svelte new file mode 100644 index 0000000..965aef5 --- /dev/null +++ b/entrypoints/popup/components/ToolbarButton.svelte @@ -0,0 +1,43 @@ + + + + + diff --git a/entrypoints/popup/Tooltip.svelte b/entrypoints/popup/components/Tooltip.svelte similarity index 100% rename from entrypoints/popup/Tooltip.svelte rename to entrypoints/popup/components/Tooltip.svelte diff --git a/entrypoints/popup/index.html b/entrypoints/popup/index.html index 48aa004..f59bed5 100644 --- a/entrypoints/popup/index.html +++ b/entrypoints/popup/index.html @@ -207,7 +207,8 @@ /* Dark: toggled filter/toolbar buttons read as accent-selected (GitHub toggled-filter style) — legible on/off state without spending green, which stays reserved for the single primary action per view. Global on - purpose — these classes live in scoped Svelte styles across tabs. */ + purpose — these classes live in scoped Svelte styles (.toolbar-btn--active + in ToolbarButton.svelte, the rest across tabs). */ @media (prefers-color-scheme: dark) { :root:not([data-theme='light']) :is(.toolbar-btn--active, .filter--active, .wrap-btn--active) { background: var(--accent-tint); diff --git a/entrypoints/popup/index.ts b/entrypoints/popup/index.ts index ab42940..4a2c0a8 100644 --- a/entrypoints/popup/index.ts +++ b/entrypoints/popup/index.ts @@ -1,6 +1,7 @@ import { mount } from 'svelte'; import App from './App.svelte'; import './stores/theme.svelte'; +import './styles/table.css'; document.addEventListener('DOMContentLoaded', () => { const root = document.getElementById('app'); diff --git a/entrypoints/popup/styles/table.css b/entrypoints/popup/styles/table.css new file mode 100644 index 0000000..2f89414 --- /dev/null +++ b/entrypoints/popup/styles/table.css @@ -0,0 +1,84 @@ +/** + * Shared chrome for the popup's data tables (Links, Images, Assets, Hreflangs, + * Sitemaps). These style DOM that lives in each tab's own , so no + * component can own them. + * + * Everything sits in a cascade layer on purpose: layered rules always lose to + * unlayered ones, so a tab's scoped Svelte styles override any of this without + * specificity games (Images needs `vertical-align: top`, Assets gates the row + * cursor behind .row--clickable, and so on). + * + * Cell rules are scoped under .table so generic names can't leak — Overview has + * an unrelated .row that must not pick up table-row behaviour. + */ +@layer table { + .table-wrap { + flex: 1; + overflow-y: auto; + overflow-x: hidden; + } + .table-wrap::-webkit-scrollbar { + width: 3px; + } + .table-wrap::-webkit-scrollbar-thumb { + background: var(--scrollbar); + border-radius: 3px; + } + + .table { + width: 100%; + border-collapse: collapse; + font-size: 13px; + table-layout: fixed; + } + + /* Edge-cell gutters keep content inset while row/hover background spans full width */ + .table .th:first-child, + .table .td:first-child { + padding-left: 20px; + } + .table .th:last-child, + .table .td:last-child { + padding-right: 20px; + } + + .table .th { + text-align: left; + font-size: 10.5px; + font-weight: 700; + text-transform: uppercase; + letter-spacing: 0.04em; + color: var(--text-label); + padding: 8px 8px 8px 0; + border-bottom: 1px solid var(--border); + position: sticky; + top: 0; + background: var(--bg-canvas); + z-index: 1; + } + .table .th__count { + font-weight: 500; + color: var(--text-muted); + letter-spacing: 0; + text-transform: none; + } + + .table .td { + padding: 9px 8px 9px 0; + color: var(--text-secondary); + border-bottom: 1px solid var(--border-muted); + vertical-align: middle; + } + + /* Hover colour stays per-tab: Assets only highlights rows that do something. */ + .table .row { + transition: background 0.1s; + } + + .table-wrap .no-results { + text-align: center; + padding: 24px; + font-size: 13px; + color: var(--text-muted); + } +} From 2611a84d08548f9fb186e28f1881669e0fe2dfc8 Mon Sep 17 00:00:00 2001 From: Junaid Ahmed Date: Sun, 9 Aug 2026 07:05:48 +0500 Subject: [PATCH 25/38] refactor(popup): extract summary builders into utils and test them --- TODOS.md | 1 - entrypoints/popup/Assets.svelte | 21 +---- entrypoints/popup/Hreflangs.svelte | 23 +---- entrypoints/popup/Images.svelte | 19 +--- entrypoints/popup/Links.svelte | 30 +------ entrypoints/popup/Schema.svelte | 12 +-- .../popup/components/SummaryBar.svelte | 6 +- entrypoints/popup/tests/assets.test.ts | 32 +++++++ entrypoints/popup/tests/hreflang.test.ts | 46 +++++++++- entrypoints/popup/tests/images.test.ts | 53 ++++++++++- entrypoints/popup/tests/links.test.ts | 89 ++++++++++++++++++- entrypoints/popup/tests/schema.test.ts | 32 ++++++- entrypoints/popup/utils/assets.ts | 30 ++++++- entrypoints/popup/utils/hreflang.ts | 34 ++++++- entrypoints/popup/utils/images.ts | 34 ++++++- entrypoints/popup/utils/links.ts | 48 +++++++++- entrypoints/popup/utils/schema.ts | 16 +++- entrypoints/popup/utils/types.ts | 9 ++ 18 files changed, 424 insertions(+), 111 deletions(-) diff --git a/TODOS.md b/TODOS.md index f617164..76a54c2 100644 --- a/TODOS.md +++ b/TODOS.md @@ -340,7 +340,6 @@ Inline Core Web Vitals summary in the Theme tab. Show LCP, FID/INP, CLS, and TTF Non-blocking findings from the popup-improvements pre-landing review, deferred to keep the ship moving: -- **summaryItems aggregation untested** — five tabs now build `summaryItems` as an inline `$derived.by` (Links, Images, Assets, Schema, Hreflangs). Extract pure `summarize*` helpers returning SummaryItem[] and pin singular/plural labels, zero-suppression, and warn/err tones with bun tests - **Broken-anchor predicate unexercised** — `samePageFragment` is extracted and tested in `popup/utils/links.ts`, but the `isBrokenAnchor` predicate around it is still inline in `main.content.ts` and untested. Extract it with tests for the `''`/`top`/named-anchor branches, and add an `` row to test-pages/links/mixed.html (the hasNamedAnchor Set path has no fixture) - **Popup renders non-http(s) hrefs as live anchors** — javascript:/data: links rely on MV3 CSP to stay inert; render `other`-kind schemes as plain text in Links (and the same pattern in Images/Assets open actions) - **Link index stamping is page-visible** — `data-alfred-link-index` lets pages fingerprint the extension; accepted tradeoff for mutation-safe scroll-to-link, revisit with a WeakRef snapshot if it ever matters diff --git a/entrypoints/popup/Assets.svelte b/entrypoints/popup/Assets.svelte index 4965c29..6e0e1c4 100644 --- a/entrypoints/popup/Assets.svelte +++ b/entrypoints/popup/Assets.svelte @@ -1,12 +1,11 @@ {#if assets.length === 0} diff --git a/entrypoints/popup/Hreflangs.svelte b/entrypoints/popup/Hreflangs.svelte index e818c59..d3dc726 100644 --- a/entrypoints/popup/Hreflangs.svelte +++ b/entrypoints/popup/Hreflangs.svelte @@ -1,12 +1,12 @@ - diff --git a/entrypoints/popup/tests/assets.test.ts b/entrypoints/popup/tests/assets.test.ts index b268a3d..31cca38 100644 --- a/entrypoints/popup/tests/assets.test.ts +++ b/entrypoints/popup/tests/assets.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from 'bun:test'; +import type { RawAsset } from '../utils/types'; import { displaySource, hasOwnLoad, @@ -8,6 +9,7 @@ import { matchesAssetFlag, scriptLoad, scriptSubtype, + summarizeAssets, typeLabel } from '../utils/assets'; @@ -268,3 +270,33 @@ describe('displaySource', () => { expect(displaySource('not a url', 'example.com')).toBe('not a url'); }); }); + +describe('summarizeAssets', () => { + const asset = (over: Partial): RawAsset => + ({ kind: 'script', size: 0, renderBlocking: false, ...over }) as RawAsset; + + const texts = (items: { text: string }[]) => items.map((i) => i.text); + + test('always shows both kind counts, singular for one', () => { + expect(texts(summarizeAssets([asset({}), asset({ kind: 'style' })]))).toEqual(['1 script', '1 style']); + }); + + test('shows zeroed kind counts for an empty list', () => { + expect(texts(summarizeAssets([]))).toEqual(['0 scripts', '0 styles']); + }); + + test('suppresses size and render-blocking at zero', () => { + expect(summarizeAssets([asset({})])).toHaveLength(2); + }); + + test('totals known sizes and omits the unknown ones', () => { + expect(summarizeAssets([asset({ size: 2048 }), asset({ size: 0 })])[2]?.text).toBe('2.0 KB'); + }); + + test('warns on render-blocking assets', () => { + expect(summarizeAssets([asset({ renderBlocking: true })]).at(-1)).toEqual({ + text: '1 render-blocking', + tone: 'warn' + }); + }); +}); diff --git a/entrypoints/popup/tests/hreflang.test.ts b/entrypoints/popup/tests/hreflang.test.ts index 9b97349..9d76e54 100644 --- a/entrypoints/popup/tests/hreflang.test.ts +++ b/entrypoints/popup/tests/hreflang.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from 'bun:test'; -import { analyzeHreflangs, isValidHreflangCode, normalizeUrl } from '../utils/hreflang'; +import { analyzeHreflangs, isValidHreflangCode, normalizeUrl, summarizeHreflangs } from '../utils/hreflang'; +import type { HreflangAnalysis, HreflangEntry } from '../utils/hreflang'; import type { RawHreflang } from '../utils/types'; const tag = (overrides: Partial): RawHreflang => ({ @@ -116,3 +117,46 @@ describe('analyzeHreflangs', () => { expect(a.hasSelf).toBe(true); }); }); + +describe('summarizeHreflangs', () => { + const analysis = (over: Partial): HreflangAnalysis => ({ + entries: [], + issues: [], + hasXDefault: false, + hasSelf: false, + errorCount: 0, + warningCount: 0, + ...over + }); + + const entry = (over: Partial = {}): HreflangEntry => + ({ ...tag({}), isSelf: false, ...over }) as HreflangEntry; + + const texts = (items: { text: string }[]) => items.map((i) => i.text); + + test('leads with the alternate count, singular for one', () => { + expect(summarizeHreflangs(analysis({ entries: [entry()] }), PAGE)[0]?.text).toBe('1 alternate'); + expect(summarizeHreflangs(analysis({ entries: [entry(), entry()] }), PAGE)[0]?.text).toBe('2 alternates'); + }); + + test('states the healthy x-default and self-reference cases rather than suppressing them', () => { + const items = summarizeHreflangs(analysis({ hasXDefault: true, hasSelf: true }), PAGE); + expect(texts(items)).toEqual(['0 alternates', 'x-default', 'self-referencing']); + expect(items.every((i) => i.tone === undefined)).toBe(true); + }); + + test('warns on a missing x-default and errors on a missing self-reference', () => { + const items = summarizeHreflangs(analysis({}), PAGE); + expect(items.find((i) => i.text === 'no x-default')?.tone).toBe('warn'); + expect(items.find((i) => i.text === 'no self-reference')?.tone).toBe('err'); + }); + + test('omits the self-reference item entirely without a page URL', () => { + expect(texts(summarizeHreflangs(analysis({ hasSelf: false }), null))).toEqual(['0 alternates', 'no x-default']); + }); + + test('appends the error count, singular for one', () => { + expect(summarizeHreflangs(analysis({ errorCount: 1 }), PAGE).at(-1)).toEqual({ text: '1 error', tone: 'err' }); + expect(summarizeHreflangs(analysis({ errorCount: 2 }), PAGE).at(-1)?.text).toBe('2 errors'); + }); +}); diff --git a/entrypoints/popup/tests/images.test.ts b/entrypoints/popup/tests/images.test.ts index 3cbc1f4..d476e0f 100644 --- a/entrypoints/popup/tests/images.test.ts +++ b/entrypoints/popup/tests/images.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from 'bun:test'; -import type { RawImage } from '../utils/types'; +import type { ImageStatus, RawImage } from '../utils/types'; import { altState, analyzeImages, @@ -9,7 +9,8 @@ import { imageStatus, isBrokenImage, isOversized, - parseBackgroundUrls + parseBackgroundUrls, + summarizeImages } from '../utils/images'; describe('isBrokenImage', () => { @@ -253,3 +254,51 @@ describe('isOversized', () => { expect(isOversized(1600, 1000, 160, 100, 0)).toBe(true); }); }); + +describe('summarizeImages', () => { + const img = (over: Partial): RawImage => ({ index: 0, size: 0, oversized: false, ...over }) as RawImage; + + const NO_STATUS = new Map(); + const texts = (items: { text: string }[]) => items.map((i) => i.text); + + test('leads with the row count, singular for one', () => { + expect(summarizeImages([img({})], NO_STATUS)[0]?.text).toBe('1 image'); + expect(summarizeImages([img({ index: 0 }), img({ index: 1 })], NO_STATUS)[0]?.text).toBe('2 images'); + }); + + test('keeps the plural noun for an empty list', () => { + expect(texts(summarizeImages([], NO_STATUS))).toEqual(['0 images']); + }); + + test('suppresses size and every defect count at zero', () => { + expect(summarizeImages([img({})], NO_STATUS)).toHaveLength(1); + }); + + test('totals known sizes and omits the unknown ones', () => { + const images = [img({ index: 0, size: 1024 }), img({ index: 1, size: 0 })]; + expect(summarizeImages(images, NO_STATUS)[1]?.text).toBe('1.0 KB'); + }); + + test('splits the status map into missing-alt and broken counts', () => { + const images = [img({ index: 0 }), img({ index: 1 }), img({ index: 2 })]; + const statuses = new Map([ + [0, 'missing-alt'], + [1, 'broken'], + [2, 'ok'] + ]); + const items = summarizeImages(images, statuses); + expect(items.find((i) => i.text === '1 missing alt')?.tone).toBe('warn'); + expect(items.find((i) => i.text === '1 broken')?.tone).toBe('err'); + }); + + test('treats an image missing from the status map as ok', () => { + expect(summarizeImages([img({ index: 7 })], NO_STATUS)).toHaveLength(1); + }); + + test('warns on oversized images', () => { + expect(summarizeImages([img({ oversized: true })], NO_STATUS).at(-1)).toEqual({ + text: '1 oversized', + tone: 'warn' + }); + }); +}); diff --git a/entrypoints/popup/tests/links.test.ts b/entrypoints/popup/tests/links.test.ts index 6eee3d3..b0a5abd 100644 --- a/entrypoints/popup/tests/links.test.ts +++ b/entrypoints/popup/tests/links.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from 'bun:test'; +import type { LinkStatusBucket, LinkStatusResult, RawLink } from '../utils/types'; import { classifyLink, followRank, @@ -6,7 +7,8 @@ import { isInsecureHttp, linkText, relFlags, - samePageFragment + samePageFragment, + summarizeLinks } from '../utils/links'; describe('isDofollow', () => { @@ -239,3 +241,88 @@ describe('isInsecureHttp', () => { expect(isInsecureHttp('not a url')).toBe(false); }); }); + +describe('summarizeLinks', () => { + const link = (over: Partial): RawLink => + ({ + index: 0, + href: 'https://example.com/a', + text: 'a', + rel: '', + kind: 'internal', + isNofollow: false, + isSponsored: false, + isUgc: false, + isImage: false, + isHidden: false, + isInsecure: false, + isBrokenAnchor: false, + ...over + }) as RawLink; + + const status = (bucket: LinkStatusBucket): LinkStatusResult => ({ status: 0, bucket }); + const NO_STATUS = new Map(); + + const texts = (items: { text: string }[]) => items.map((i) => i.text); + + it('always leads with the row count and external total', () => { + expect(texts(summarizeLinks([link({}), link({ kind: 'external' })], NO_STATUS))).toEqual(['2 links', '1 external']); + }); + + it('uses the singular noun for one link', () => { + expect(summarizeLinks([link({})], NO_STATUS)[0]?.text).toBe('1 link'); + }); + + it('keeps the plural noun for an empty list', () => { + expect(texts(summarizeLinks([], NO_STATUS))).toEqual(['0 links', '0 external']); + }); + + it('suppresses every defect count at zero', () => { + expect(summarizeLinks([link({})], NO_STATUS)).toHaveLength(2); + }); + + it('counts sponsored and ugc as nofollow-class hints', () => { + const links = [link({ isNofollow: true }), link({ isSponsored: true }), link({ isUgc: true }), link({})]; + expect(texts(summarizeLinks(links, NO_STATUS))).toContain('3 nofollow'); + }); + + it('leaves the nofollow item untoned but titled', () => { + const item = summarizeLinks([link({ isNofollow: true })], NO_STATUS).find((i) => i.text === '1 nofollow'); + expect(item?.tone).toBeUndefined(); + expect(item?.title).toBe('Links carrying nofollow, sponsored, or ugc hints'); + }); + + it('warns on insecure http and errors on broken fragments', () => { + const items = summarizeLinks([link({ isInsecure: true, isBrokenAnchor: true })], NO_STATUS); + expect(items.find((i) => i.text === '1 insecure http')?.tone).toBe('warn'); + expect(items.find((i) => i.text === '1 broken #')?.tone).toBe('err'); + }); + + it('warns on redirects', () => { + const statuses = new Map([['https://example.com/a', status('redirect')]]); + expect(summarizeLinks([link({})], statuses).find((i) => i.text === '1 redirect')?.tone).toBe('warn'); + }); + + it('folds 4xx, 5xx, and unreachable into one failing count', () => { + const links = [ + link({ href: 'https://example.com/1' }), + link({ href: 'https://example.com/2' }), + link({ href: 'https://example.com/3' }) + ]; + const statuses = new Map([ + ['https://example.com/1', status('client-error')], + ['https://example.com/2', status('server-error')], + ['https://example.com/3', status('error')] + ]); + expect(summarizeLinks(links, statuses).find((i) => i.text === '3 failing')?.tone).toBe('err'); + }); + + it('ignores links that came back ok', () => { + const statuses = new Map([['https://example.com/a', status('ok')]]); + expect(summarizeLinks([link({})], statuses)).toHaveLength(2); + }); + + it('ignores links that were never checked', () => { + expect(summarizeLinks([link({ href: 'https://example.com/unchecked' })], NO_STATUS)).toHaveLength(2); + }); +}); diff --git a/entrypoints/popup/tests/schema.test.ts b/entrypoints/popup/tests/schema.test.ts index f77baac..03e5fa5 100644 --- a/entrypoints/popup/tests/schema.test.ts +++ b/entrypoints/popup/tests/schema.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'bun:test'; -import { analyzeSchema, schemaTypeName } from '../utils/schema'; -import type { RawSchemaBlock } from '../utils/types'; +import { analyzeSchema, schemaTypeName, summarizeSchema } from '../utils/schema'; +import type { RawSchemaBlock, SchemaAnalysis, SchemaEntity } from '../utils/types'; function block(value: unknown, index = 0, placement: 'head' | 'body' = 'head'): RawSchemaBlock { return { index, raw: JSON.stringify(value), parseError: null, placement }; @@ -129,3 +129,31 @@ describe('schemaTypeName — @type normalization', () => { expect(schemaTypeName({ name: 'x' })).toBe('Unknown'); }); }); + +describe('summarizeSchema', () => { + const analysis = (over: Partial): SchemaAnalysis => ({ + entities: [], + invalidBlocks: [], + ...over + }); + + const entity = (type: string): SchemaEntity => ({ type, blockIndex: 0, data: {} }); + + it('leads with the entity-type count, singular for one', () => { + expect(summarizeSchema(analysis({ entities: [entity('Product')] }))[0]?.text).toBe('1 type'); + expect(summarizeSchema(analysis({ entities: [entity('Product'), entity('FAQPage')] }))[0]?.text).toBe('2 types'); + }); + + it('keeps the plural noun when nothing parsed', () => { + expect(summarizeSchema(analysis({}))).toEqual([{ text: '0 types' }]); + }); + + it('errors on blocks that failed to parse', () => { + const items = summarizeSchema(analysis({ invalidBlocks: [{ blockIndex: 0, error: 'Unexpected token' }] })); + expect(items.at(-1)).toEqual({ + text: '1 invalid', + tone: 'err', + title: 'Blocks that failed to parse as JSON' + }); + }); +}); diff --git a/entrypoints/popup/utils/assets.ts b/entrypoints/popup/utils/assets.ts index 7d1de06..fc14f77 100644 --- a/entrypoints/popup/utils/assets.ts +++ b/entrypoints/popup/utils/assets.ts @@ -1,4 +1,5 @@ -import type { AssetKind, AssetLoad, AssetPlacement, AssetSubtype, RawAsset } from './types'; +import type { AssetKind, AssetLoad, AssetPlacement, AssetSubtype, RawAsset, SummaryItem } from './types'; +import { formatSize } from './format'; import { queryActiveTab } from './messaging'; import { normalizeHost } from './links'; @@ -199,3 +200,30 @@ export function displaySource(src: string, pageHost: string | null): string { if (pageHost && host === normalizeHost(pageHost)) return rest; return host + rest; } + +/** + * Builds the Assets footer summary. Script and style counts always show; total + * size and the render-blocking count are suppressed at zero. + * @param assets - The rows currently visible (post-filter), not the full set. + */ +export function summarizeAssets(assets: RawAsset[]): SummaryItem[] { + let scripts = 0, + styles = 0, + bytes = 0, + renderBlocking = 0; + for (const a of assets) { + if (a.kind === 'script') scripts++; + else styles++; + bytes += a.size; + if (a.renderBlocking) renderBlocking++; + } + const items: SummaryItem[] = [ + { text: `${scripts} ${scripts === 1 ? 'script' : 'scripts'}` }, + { text: `${styles} ${styles === 1 ? 'style' : 'styles'}` } + ]; + if (bytes > 0) { + items.push({ text: formatSize(bytes), title: 'Sum of known sizes; opaque cross-origin assets are not included' }); + } + if (renderBlocking > 0) items.push({ text: `${renderBlocking} render-blocking`, tone: 'warn' }); + return items; +} diff --git a/entrypoints/popup/utils/hreflang.ts b/entrypoints/popup/utils/hreflang.ts index 020b5c7..d743528 100644 --- a/entrypoints/popup/utils/hreflang.ts +++ b/entrypoints/popup/utils/hreflang.ts @@ -1,4 +1,4 @@ -import type { RawHreflang } from './types'; +import type { RawHreflang, SummaryItem } from './types'; import { queryActiveTab } from './messaging'; import { normalizeUrl } from './url'; @@ -167,3 +167,35 @@ export function analyzeHreflangs(tags: RawHreflang[], pageUrl: string | null): H warningCount: issues.filter((i) => i.severity === 'warning').length }; } + +/** + * Builds the Hreflangs footer summary. Unlike the other tabs the x-default and + * self-reference items always render, stating the healthy case too, because + * their absence is the defect. + * @param analysis - Result of analyzeHreflangs for the current page. + * @param pageUrl - URL of the page; null omits the self-reference item entirely, + * since analyzeHreflangs cannot judge it without one. + */ +export function summarizeHreflangs(analysis: HreflangAnalysis, pageUrl: string | null): SummaryItem[] { + const n = analysis.entries.length; + const items: SummaryItem[] = [{ text: `${n} ${n === 1 ? 'alternate' : 'alternates'}` }]; + items.push( + analysis.hasXDefault + ? { text: 'x-default' } + : { text: 'no x-default', tone: 'warn', title: 'No fallback page for unmatched languages' } + ); + if (pageUrl) { + items.push( + analysis.hasSelf + ? { text: 'self-referencing' } + : { text: 'no self-reference', tone: 'err', title: "The set does not include this page's own URL" } + ); + } + if (analysis.errorCount > 0) { + items.push({ + text: `${analysis.errorCount} ${analysis.errorCount === 1 ? 'error' : 'errors'}`, + tone: 'err' + }); + } + return items; +} diff --git a/entrypoints/popup/utils/images.ts b/entrypoints/popup/utils/images.ts index f15f6b1..7e3545f 100644 --- a/entrypoints/popup/utils/images.ts +++ b/entrypoints/popup/utils/images.ts @@ -1,4 +1,5 @@ -import type { ImageSource, ImageStatus, RawImage } from './types'; +import type { ImageSource, ImageStatus, RawImage, SummaryItem } from './types'; +import { formatSize } from './format'; import { queryActiveTab, sendToActiveTab } from './messaging'; /** @@ -195,3 +196,34 @@ export function isOversized( const needed = displayWidth * displayHeight * ratio * ratio; return natural > needed * 4 && natural - needed > OVERSIZED_WASTE_FLOOR; } + +/** + * Builds the Images footer summary. Total always shows; size and every defect + * count are suppressed at zero. + * @param images - The rows currently visible (post-filter), not the full set. + * @param statuses - Resolved status keyed by RawImage.index; missing reads as 'ok'. + */ +export function summarizeImages(images: RawImage[], statuses: ReadonlyMap): SummaryItem[] { + let bytes = 0, + missingAlt = 0, + broken = 0, + oversized = 0; + for (const img of images) { + bytes += img.size; + const st = statuses.get(img.index) ?? 'ok'; + if (st === 'broken') broken++; + else if (st === 'missing-alt') missingAlt++; + if (img.oversized) oversized++; + } + const items: SummaryItem[] = [{ text: `${images.length} ${images.length === 1 ? 'image' : 'images'}` }]; + if (bytes > 0) { + items.push({ + text: formatSize(bytes), + title: 'Sum of known sizes; opaque cross-origin images are not included' + }); + } + if (missingAlt > 0) items.push({ text: `${missingAlt} missing alt`, tone: 'warn' }); + if (broken > 0) items.push({ text: `${broken} broken`, tone: 'err' }); + if (oversized > 0) items.push({ text: `${oversized} oversized`, tone: 'warn' }); + return items; +} diff --git a/entrypoints/popup/utils/links.ts b/entrypoints/popup/utils/links.ts index a48fc5f..3ec37e6 100644 --- a/entrypoints/popup/utils/links.ts +++ b/entrypoints/popup/utils/links.ts @@ -1,4 +1,4 @@ -import type { LinkKind, RawLink, LinkStatusResult } from './types'; +import type { LinkKind, RawLink, LinkStatusResult, SummaryItem } from './types'; import { queryActiveTab, sendToActiveTab } from './messaging'; import { sendRuntimeMessage } from '@/utils/messages'; import type { TextSourceElement } from './dom-text'; @@ -157,3 +157,49 @@ export const isDofollow = (link: FollowFlags): boolean => !link.isNofollow && !l /** Sort rank for the Dofollow column: dofollow, then ugc, sponsored, nofollow. */ export const followRank = (link: FollowFlags): number => link.isNofollow ? 3 : link.isSponsored ? 2 : link.isUgc ? 1 : 0; + +/** + * Builds the Links footer summary. Count and external total always show; every + * defect count is suppressed at zero so a clean page reads as a short bar. + * @param links - The rows currently visible (post-filter), not the full set. + * @param statuses - Checked HTTP statuses keyed by href; unchecked links are absent. + */ +export function summarizeLinks(links: RawLink[], statuses: ReadonlyMap): SummaryItem[] { + let external = 0, + nofollowish = 0, + insecure = 0, + broken = 0; + let httpRedirect = 0, + httpDead = 0; + for (const l of links) { + if (l.kind === 'external') external++; + if (!isDofollow(l)) nofollowish++; + if (l.isInsecure) insecure++; + if (l.isBrokenAnchor) broken++; + const st = statuses.get(l.href); + if (st) { + if (st.bucket === 'redirect') httpRedirect++; + else if (st.bucket === 'client-error' || st.bucket === 'server-error' || st.bucket === 'error') httpDead++; + } + } + const items: SummaryItem[] = [ + { text: `${links.length} ${links.length === 1 ? 'link' : 'links'}` }, + { text: `${external} external` } + ]; + if (nofollowish > 0) { + items.push({ text: `${nofollowish} nofollow`, title: 'Links carrying nofollow, sponsored, or ugc hints' }); + } + if (insecure > 0) items.push({ text: `${insecure} insecure http`, tone: 'warn' }); + if (broken > 0) items.push({ text: `${broken} broken #`, tone: 'err' }); + if (httpRedirect > 0) { + items.push({ text: `${httpRedirect} redirect`, tone: 'warn', title: 'Links that respond with a 3xx redirect' }); + } + if (httpDead > 0) { + items.push({ + text: `${httpDead} failing`, + tone: 'err', + title: 'Links returning 4xx/5xx or unreachable (advisory)' + }); + } + return items; +} diff --git a/entrypoints/popup/utils/schema.ts b/entrypoints/popup/utils/schema.ts index 7cfeb27..a4a8c18 100644 --- a/entrypoints/popup/utils/schema.ts +++ b/entrypoints/popup/utils/schema.ts @@ -1,4 +1,4 @@ -import type { RawSchemaBlock, SchemaAnalysis, SchemaEntity } from './types'; +import type { RawSchemaBlock, SchemaAnalysis, SchemaEntity, SummaryItem } from './types'; import { queryActiveTab } from './messaging'; /** @@ -97,3 +97,17 @@ export function analyzeSchema(blocks: RawSchemaBlock[]): SchemaAnalysis { return { entities, invalidBlocks }; } + +/** Builds the Schema footer summary: entity-type count, plus invalid blocks when any failed to parse. */ +export function summarizeSchema(analysis: SchemaAnalysis): SummaryItem[] { + const n = analysis.entities.length; + const items: SummaryItem[] = [{ text: `${n} ${n === 1 ? 'type' : 'types'}` }]; + if (analysis.invalidBlocks.length > 0) { + items.push({ + text: `${analysis.invalidBlocks.length} invalid`, + tone: 'err', + title: 'Blocks that failed to parse as JSON' + }); + } + return items; +} diff --git a/entrypoints/popup/utils/types.ts b/entrypoints/popup/utils/types.ts index e3f7ced..ad29d09 100644 --- a/entrypoints/popup/utils/types.ts +++ b/entrypoints/popup/utils/types.ts @@ -1,3 +1,12 @@ +// One segment of a tab's SummaryBar footer, e.g. `{ text: '3 broken', tone: 'err' }`. +// Lives here rather than beside the component so the per-tab summarize* builders +// in utils/ don't have to import a type from a .svelte module. +export interface SummaryItem { + text: string; + tone?: 'warn' | 'err'; + title?: string; +} + export interface RawHeading { level: number; text: string; From 7af747de0311bd8e42a7ebd4180c5c7aebfaf1a5 Mon Sep 17 00:00:00 2001 From: Junaid Ahmed Date: Sun, 9 Aug 2026 07:12:19 +0500 Subject: [PATCH 26/38] fix(links): only treat as a fragment target, test the predicate --- TODOS.md | 1 - entrypoints/main.content.ts | 21 ++++----- entrypoints/popup/tests/links.test.ts | 65 +++++++++++++++++++++++++++ entrypoints/popup/utils/links.ts | 24 ++++++++++ test-pages/TESTING.md | 17 ++++--- test-pages/links/mixed.html | 25 +++++++---- 6 files changed, 126 insertions(+), 27 deletions(-) diff --git a/TODOS.md b/TODOS.md index 76a54c2..16117da 100644 --- a/TODOS.md +++ b/TODOS.md @@ -340,7 +340,6 @@ Inline Core Web Vitals summary in the Theme tab. Show LCP, FID/INP, CLS, and TTF Non-blocking findings from the popup-improvements pre-landing review, deferred to keep the ship moving: -- **Broken-anchor predicate unexercised** — `samePageFragment` is extracted and tested in `popup/utils/links.ts`, but the `isBrokenAnchor` predicate around it is still inline in `main.content.ts` and untested. Extract it with tests for the `''`/`top`/named-anchor branches, and add an `` row to test-pages/links/mixed.html (the hasNamedAnchor Set path has no fixture) - **Popup renders non-http(s) hrefs as live anchors** — javascript:/data: links rely on MV3 CSP to stay inert; render `other`-kind schemes as plain text in Links (and the same pattern in Images/Assets open actions) - **Link index stamping is page-visible** — `data-alfred-link-index` lets pages fingerprint the extension; accepted tradeoff for mutation-safe scroll-to-link, revisit with a WeakRef snapshot if it ever matters diff --git a/entrypoints/main.content.ts b/entrypoints/main.content.ts index 5c23730..142aea5 100644 --- a/entrypoints/main.content.ts +++ b/entrypoints/main.content.ts @@ -7,7 +7,8 @@ import { Toast } from '@/utils/toast'; import type { TabMessage } from '@/utils/messages'; import { createBridgeClient } from '@/utils/mainWorldBridge'; import { headingText } from './popup/utils/headings'; -import { classifyLink, isDofollow, isInsecureHttp, linkText, relFlags, samePageFragment } from './popup/utils/links'; +import type { FragmentTargets } from './popup/utils/links'; +import { classifyLink, isBrokenAnchor, isDofollow, isInsecureHttp, linkText, relFlags } from './popup/utils/links'; import { looksLikeHtml } from './popup/utils/robots'; import { classifySitemap, @@ -352,22 +353,18 @@ export default defineContentScript({ // Built lazily on the first getElementById miss; getElementsByName walks the // whole document per call, which is O(links x DOM) on hash-router pages. let anchorNames: Set | null = null; - const hasNamedAnchor = (fragment: string): boolean => { - anchorNames ??= new Set(Array.from(document.querySelectorAll('[name]'), (el) => el.getAttribute('name'))); - return anchorNames.has(fragment); + const targets: FragmentTargets = { + hasId: (id) => document.getElementById(id) !== null, + hasNamedAnchor: (name) => { + anchorNames ??= new Set(Array.from(document.querySelectorAll('a[name]'), (el) => el.getAttribute('name'))); + return anchorNames.has(name); + } }; const pageUrl = location.href; const links = anchors.map((anchor, i) => { const href = anchor.href; // serializing getter; read once per anchor const rel = anchor.getAttribute('rel') ?? ''; const { nofollow, sponsored, ugc } = relFlags(rel); - const fragment = samePageFragment(href, pageUrl); - const isBrokenAnchor = - fragment !== null && - fragment !== '' && - fragment !== 'top' && // #top scrolls to the document top even without a target - !document.getElementById(fragment) && - !hasNamedAnchor(fragment); return { index: i, href, @@ -380,7 +377,7 @@ export default defineContentScript({ isImage: anchor.querySelector('img, svg, picture') !== null, isHidden: !anchor.checkVisibility(), isInsecure: isInsecureHttp(href), - isBrokenAnchor + isBrokenAnchor: isBrokenAnchor(href, pageUrl, targets) }; }); sendResponse(links); diff --git a/entrypoints/popup/tests/links.test.ts b/entrypoints/popup/tests/links.test.ts index b0a5abd..c3c0aa6 100644 --- a/entrypoints/popup/tests/links.test.ts +++ b/entrypoints/popup/tests/links.test.ts @@ -3,6 +3,7 @@ import type { LinkStatusBucket, LinkStatusResult, RawLink } from '../utils/types import { classifyLink, followRank, + isBrokenAnchor, isDofollow, isInsecureHttp, linkText, @@ -211,6 +212,70 @@ describe('samePageFragment', () => { }); }); +describe('isBrokenAnchor', () => { + const PAGE = 'https://shop.com/products/tee'; + + const targets = (ids: string[] = [], names: string[] = []) => ({ + hasId: (id: string) => ids.includes(id), + hasNamedAnchor: (name: string) => names.includes(name) + }); + + it('flags a same-page fragment with no matching target', () => { + expect(isBrokenAnchor(`${PAGE}#nowhere`, PAGE, targets())).toBe(true); + }); + + it('accepts a fragment matching an element id', () => { + expect(isBrokenAnchor(`${PAGE}#reviews`, PAGE, targets(['reviews']))).toBe(false); + }); + + it('accepts a fragment matching a legacy named anchor', () => { + expect(isBrokenAnchor(`${PAGE}#reviews`, PAGE, targets([], ['reviews']))).toBe(false); + }); + + it('exempts #top, which scrolls to the document top with no target', () => { + expect(isBrokenAnchor(`${PAGE}#top`, PAGE, targets())).toBe(false); + }); + + it('still resolves #top through a real target when one exists', () => { + expect(isBrokenAnchor(`${PAGE}#top`, PAGE, targets(['top']))).toBe(false); + }); + + it('does not exempt other casings of top', () => { + expect(isBrokenAnchor(`${PAGE}#Top`, PAGE, targets())).toBe(true); + }); + + it('never flags a bare href="#"', () => { + expect(isBrokenAnchor(`${PAGE}#`, PAGE, targets())).toBe(false); + }); + + it('never flags a link that points off this page', () => { + expect(isBrokenAnchor('https://shop.com/other#nowhere', PAGE, targets())).toBe(false); + expect(isBrokenAnchor('https://example.com/#nowhere', PAGE, targets())).toBe(false); + expect(isBrokenAnchor(`${PAGE}?variant=2#nowhere`, PAGE, targets())).toBe(false); + }); + + it('never flags a link with no fragment at all', () => { + expect(isBrokenAnchor(PAGE, PAGE, targets())).toBe(false); + }); + + it('matches the decoded fragment, not the percent-encoded one', () => { + expect(isBrokenAnchor(`${PAGE}#size%20guide`, PAGE, targets(['size guide']))).toBe(false); + expect(isBrokenAnchor(`${PAGE}#size%20guide`, PAGE, targets(['size%20guide']))).toBe(true); + }); + + it('does not consult the name index when an id already matched', () => { + let consulted = false; + isBrokenAnchor(`${PAGE}#reviews`, PAGE, { + hasId: () => true, + hasNamedAnchor: () => { + consulted = true; + return false; + } + }); + expect(consulted).toBe(false); + }); +}); + describe('isInsecureHttp', () => { it('flags plain-http links', () => { expect(isInsecureHttp('http://example.com/old-page')).toBe(true); diff --git a/entrypoints/popup/utils/links.ts b/entrypoints/popup/utils/links.ts index 3ec37e6..b73b7b3 100644 --- a/entrypoints/popup/utils/links.ts +++ b/entrypoints/popup/utils/links.ts @@ -124,6 +124,30 @@ export function samePageFragment(href: string, pageUrl: string): string | null { } } +/** + * Fragment-target lookups, injected so the predicate below stays DOM-free and + * testable while the content script keeps its lazily-built name index. + */ +export interface FragmentTargets { + hasId(id: string): boolean; + /** Matches `` only — a name attribute on any other element is not a fragment target. */ + hasNamedAnchor(name: string): boolean; +} + +/** + * Flags a same-page `#fragment` link whose target does not exist. Links that + * point elsewhere are never broken by this measure. `#top` is exempt: browsers + * scroll to the document top for it whether or not a matching element exists. + * @param {string} href - Absolute href of the link. + * @param {string} pageUrl - URL of the page being analyzed. + * @param targets - Lookups against the page's ids and named anchors. + */ +export function isBrokenAnchor(href: string, pageUrl: string, targets: FragmentTargets): boolean { + const fragment = samePageFragment(href, pageUrl); + if (fragment === null || fragment === 'top') return false; + return !targets.hasId(fragment) && !targets.hasNamedAnchor(fragment); +} + /** Hosts browsers treat as potentially-trustworthy origins even over http. */ const LOOPBACK_HOST = /^(localhost|.+\.localhost|127\.0\.0\.1|\[::1\])$/i; diff --git a/test-pages/TESTING.md b/test-pages/TESTING.md index c72cfa9..8305e0d 100644 --- a/test-pages/TESTING.md +++ b/test-pages/TESTING.md @@ -92,8 +92,8 @@ numbered links on that page. ### Counts and classification -- [ ] Header shows (18/18); the nav link is the 18th -- [ ] Type filter: Internal 10, External 5, **Other 3** (Other is a new option) +- [ ] Header shows (21/21); the nav link is the 21st +- [ ] Type filter: Internal 13, External 5, **Other 3** (Other is a new option) - [ ] Row 9 (`www.localhost`) is **Internal** and has no `http` badge - [ ] Rows 15, 16, 17 show Type **Mailto**, **Tel**, **Other** and their full href in the URL cell @@ -103,7 +103,7 @@ numbered links on that page. - [ ] Row 5 (nofollow): red "No" pill - [ ] Row 6 (sponsored): amber "Sponsored" pill - [ ] Row 7 (ugc): amber "UGC" pill -- [ ] Follow filter shows Dofollow 15, Nofollow 1, Sponsored 1, UGC 1 +- [ ] Follow filter shows Dofollow 18, Nofollow 1, Sponsored 1, UGC 1 - [ ] Sort by Dofollow ascending groups Yes → UGC → Sponsored → No ### Anchor text (alt and aria-label fallback) @@ -114,13 +114,18 @@ numbered links on that page. with the "img" tag; the text wins over the image - [ ] Row 12: red italic "(image without alt text)" - [ ] Row 13: "Theme fixture" (from aria-label) -- [ ] Anchor filter: Text 17, Image 3, None 1 +- [ ] Anchor filter: Text 20, Image 3, None 1 ### Badges - [ ] Row 8: amber "http" badge (plain-http external link) -- [ ] Row 3: red "broken #" badge (`#nowhere` has no target) -- [ ] Row 2 (`#top`): no broken badge (target exists) +- [ ] Rows 3 (`#nowhere`) and 20 (`#viewport`): red "broken #" badge +- [ ] Row 2 (`#content`): no broken badge (an element carries that id) +- [ ] Row 18 (`#top`): no broken badge, even though nothing on the page has + `id="top"` — browsers scroll to the document top for it +- [ ] Row 19 (`#legacy`): no broken badge (`` is a target); + row 20 proves `name` on a non-anchor element (``) + is not - [ ] The three links to `/` (rows 10, 12, nav) each show a ×3 dup badge ### Hidden links (new surfacing) diff --git a/test-pages/links/mixed.html b/test-pages/links/mixed.html index 2c1e042..a466544 100644 --- a/test-pages/links/mixed.html +++ b/test-pages/links/mixed.html @@ -2,6 +2,8 @@ + + Internal, external, rel hints, image, hidden, broken-anchor, and non-web links diff --git a/entrypoints/popup/tests/hreflang.test.ts b/entrypoints/popup/tests/hreflang.test.ts index 9d76e54..25ef288 100644 --- a/entrypoints/popup/tests/hreflang.test.ts +++ b/entrypoints/popup/tests/hreflang.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from 'bun:test'; -import { analyzeHreflangs, isValidHreflangCode, normalizeUrl, summarizeHreflangs } from '../utils/hreflang'; +import { analyzeHreflangs, isValidHreflangCode, summarizeHreflangs } from '../utils/hreflang'; import type { HreflangAnalysis, HreflangEntry } from '../utils/hreflang'; import type { RawHreflang } from '../utils/types'; @@ -28,17 +28,6 @@ describe('isValidHreflangCode', () => { }); }); -describe('normalizeUrl', () => { - test('drops hash, lowercases host, strips trailing slash beyond root', () => { - expect(normalizeUrl('https://Example.com/en/#top')).toBe('https://example.com/en'); - expect(normalizeUrl('https://example.com/')).toBe('https://example.com/'); - }); - - test('returns null for unparseable input', () => { - expect(normalizeUrl('/en/')).toBeNull(); - }); -}); - describe('analyzeHreflangs', () => { test('empty input yields no entries and no issues', () => { const a = analyzeHreflangs([], PAGE); From 62ac16a61b32654f6b1e81d52406173e9292e6bc Mon Sep 17 00:00:00 2001 From: Junaid Ahmed Date: Sun, 9 Aug 2026 08:03:02 +0500 Subject: [PATCH 32/38] refactor(robots): normalize user-agent tokens at parse time --- entrypoints/popup/tests/robots.test.ts | 9 ++++-- entrypoints/popup/utils/robots.ts | 42 +++++++++++++------------- 2 files changed, 28 insertions(+), 23 deletions(-) diff --git a/entrypoints/popup/tests/robots.test.ts b/entrypoints/popup/tests/robots.test.ts index d54b5ec..fcd4484 100644 --- a/entrypoints/popup/tests/robots.test.ts +++ b/entrypoints/popup/tests/robots.test.ts @@ -26,7 +26,7 @@ describe('parseRobots', () => { const p = parse('User-agent: *\nDisallow: /admin\nAllow: /admin/public\n'); expect(p.groups.length).toBe(1); const g = p.groups[0]!; - expect(g.userAgents).toEqual([{ token: '*', line: 1 }]); + expect(g.userAgents).toEqual([{ raw: '*', token: '*', line: 1 }]); expect(g.rules).toEqual([ { type: 'disallow', path: '/admin', line: 2 }, { type: 'allow', path: '/admin/public', line: 3 } @@ -36,7 +36,12 @@ describe('parseRobots', () => { it('stacks consecutive User-agent lines into one group', () => { const p = parse('User-agent: GPTBot\nUser-agent: CCBot\nDisallow: /\n'); expect(p.groups.length).toBe(1); - expect(p.groups[0]!.userAgents.map((u) => u.token)).toEqual(['GPTBot', 'CCBot']); + expect(p.groups[0]!.userAgents.map((u) => u.raw)).toEqual(['GPTBot', 'CCBot']); + }); + + it('normalizes each declared user-agent to its product token at parse time', () => { + const p = parse('User-agent: Googlebot/2.1\nUser-agent:\nDisallow: /\n'); + expect(p.groups[0]!.userAgents.map((u) => u.token)).toEqual(['googlebot', '']); }); it('starts a new group when User-agent follows rules', () => { diff --git a/entrypoints/popup/utils/robots.ts b/entrypoints/popup/utils/robots.ts index 26ddb39..614b725 100644 --- a/entrypoints/popup/utils/robots.ts +++ b/entrypoints/popup/utils/robots.ts @@ -24,7 +24,8 @@ export interface RobotsRule { } export interface RobotsGroup { - userAgents: { token: string; line: number }[]; + /** `token` is the normalized product token used for matching; `raw` is the line as written. */ + userAgents: { raw: string; token: string; line: number }[]; rules: RobotsRule[]; crawlDelay: { value: string; line: number } | null; } @@ -151,7 +152,7 @@ export function parseRobots(text: string): ParsedRobots { groupHasRules = false; parsed.groups.push(group); } - group.userAgents.push({ token: value, line }); + group.userAgents.push({ raw: value, token: value === '*' ? '*' : userAgentToken(value), line }); break; case 'allow': case 'disallow': { @@ -272,14 +273,13 @@ function selectGroups(parsed: ParsedRobots, botToken: string): { groups: RobotsG let best = ''; for (const g of parsed.groups) { for (const ua of g.userAgents) { - if (ua.token === '*') continue; - // An empty token would prefix-match every bot; it declares no group at all. - const t = userAgentToken(ua.token); - if (t !== '' && bot.startsWith(t) && t.length > best.length) best = t; + // '' would prefix-match every bot; such a line declares no group at all. + if (ua.token === '*' || ua.token === '') continue; + if (bot.startsWith(ua.token) && ua.token.length > best.length) best = ua.token; } } if (best) { - const groups = parsed.groups.filter((g) => g.userAgents.some((ua) => userAgentToken(ua.token) === best)); + const groups = parsed.groups.filter((g) => g.userAgents.some((ua) => ua.token === best)); return { groups, token: best }; } const wildcard = parsed.groups.filter((g) => g.userAgents.some((ua) => ua.token === '*')); @@ -442,14 +442,14 @@ export function lintRobots(parsed: ParsedRobots, meta: { size: number }): LintFi for (const g of parsed.groups) { for (const ua of g.userAgents) { - // Reduces to '' when the value is blank or has no token character at all. - // Such a group matches no crawler, so its rules never apply to anyone. - if (ua.token !== '*' && userAgentToken(ua.token) === '') { + // '' when the value is blank or has no token character at all. Such a + // group matches no crawler, so its rules never apply to anyone. + if (ua.token === '') { findings.push({ severity: 'info', code: 'empty-user-agent', - message: ua.token - ? `User-agent \`${ua.token}\` has no product token: this group matches no crawler` + message: ua.raw + ? `User-agent \`${ua.raw}\` has no product token: this group matches no crawler` : 'User-agent has no value: this group matches no crawler', line: ua.line }); @@ -472,7 +472,7 @@ export function lintRobots(parsed: ParsedRobots, meta: { size: number }): LintFi findings.push({ severity: 'info', code: 'empty-group', - message: `Group \`${first.token}\` has no rules: that allows everything, which may be unintended`, + message: `Group \`${first.raw}\` has no rules: that allows everything, which may be unintended`, line: first.line }); } @@ -482,17 +482,17 @@ export function lintRobots(parsed: ParsedRobots, meta: { size: number }): LintFi const seenTokens = new Map(); for (const g of parsed.groups) { for (const ua of g.userAgents) { - // Normalized, so `Googlebot` and `Googlebot/2.1` count as the one group - // they actually merge into. Tokenless values are reported separately. - const t = ua.token === '*' ? '*' : userAgentToken(ua.token); - if (t === '') continue; - const count = (seenTokens.get(t) ?? 0) + 1; - seenTokens.set(t, count); + // Keyed on the normalized token, so `Googlebot` and `Googlebot/2.1` count + // as the one group they actually merge into. Tokenless values are + // reported separately by the empty-user-agent lint. + if (ua.token === '') continue; + const count = (seenTokens.get(ua.token) ?? 0) + 1; + seenTokens.set(ua.token, count); if (count === 2) { findings.push({ severity: 'info', code: 'duplicate-group', - message: `\`${ua.token}\` appears in multiple groups: crawlers merge them, but it is easy to misread`, + message: `\`${ua.raw}\` appears in multiple groups: crawlers merge them, but it is easy to misread`, line: ua.line }); } @@ -667,7 +667,7 @@ export function detectShopifyDefault(parsed: ParsedRobots, pageHost: string | nu for (const g of parsed.groups) { for (const ua of g.userAgents) { - live.push({ entry: normalizeEntry('user-agent', ua.token, pageHost), line: ua.line }); + live.push({ entry: normalizeEntry('user-agent', ua.raw, pageHost), line: ua.line }); } for (const r of g.rules) live.push({ entry: normalizeEntry(r.type, r.path, pageHost), line: r.line }); if (g.crawlDelay) { From ede2b7ed62ecc73df5edfa3c1fe5d8d022964d2e Mon Sep 17 00:00:00 2001 From: Junaid Ahmed Date: Sun, 9 Aug 2026 08:03:03 +0500 Subject: [PATCH 33/38] refactor(content): extract the snapshot liveness check into a helper --- entrypoints/main.content.ts | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/entrypoints/main.content.ts b/entrypoints/main.content.ts index f10b681..04be6c3 100644 --- a/entrypoints/main.content.ts +++ b/entrypoints/main.content.ts @@ -47,6 +47,17 @@ type CollectedImage = { el: Element; source: 'img' | 'picture' | 'background'; b let linkSnapshot: WeakRef[] = []; let imageSnapshot: { ref: WeakRef; source: CollectedImage['source'] }[] = []; +/** + * The snapshotted element, or undefined once the page has dropped it. The + * isConnected check matters as much as the deref: a detached node stays + * strongly reachable from any handler that still holds it, so deref alone + * would hand back an element no longer in the document. + */ +function liveRef(ref: WeakRef | undefined): T | undefined { + const el = ref?.deref(); + return el?.isConnected ? el : undefined; +} + /** * Collects all page images in a deterministic order shared by get_images, * highlight_images, and scroll_to_image so indices stay consistent across calls. @@ -1033,8 +1044,7 @@ export default defineContentScript({ sendResponse(false); return false; } - const snapped = linkSnapshot[index]?.deref(); - const target = snapped?.isConnected ? snapped : document.querySelectorAll('a[href]')[index]; + const target = liveRef(linkSnapshot[index]) ?? document.querySelectorAll('a[href]')[index]; if (target instanceof HTMLElement) flashOutline(target); sendResponse(true); return false; @@ -1091,9 +1101,9 @@ export default defineContentScript({ // Prefer the entry get_images recorded; fall back to the full walk when // that element has since been detached or garbage-collected. const snapped = imageSnapshot[index]; - const snappedEl = snapped?.ref.deref(); + const el = snapped && liveRef(snapped.ref); const item: CollectedImage | undefined = - snapped && snappedEl?.isConnected ? { el: snappedEl, source: snapped.source } : collectImageEls()[index]; + snapped && el ? { el, source: snapped.source } : collectImageEls()[index]; if (item) { const { el, source } = item; ensureImageHighlightStyle(); From cc92bd3d5af9699d7332a573e51cd998e9d1117c Mon Sep 17 00:00:00 2001 From: Junaid Ahmed Date: Sun, 9 Aug 2026 08:16:32 +0500 Subject: [PATCH 34/38] fix(cartograph): apply the 2026-03-23 adversarial review findings --- TODOS.md | 13 --- entrypoints/cartograph-world.ts | 30 ++----- entrypoints/cartograph.content/App.svelte | 1 + entrypoints/cartograph.content/cartApi.ts | 3 +- .../components/AddItemTab.svelte | 11 ++- .../components/ItemsTab.svelte | 19 ++-- .../components/KeyValueEditor.svelte | 3 +- .../components/MetadataTab.svelte | 8 +- .../components/QuantityInput.svelte | 1 + .../components/ShippingTab.svelte | 8 +- entrypoints/cartograph.content/index.ts | 14 ++- .../cartograph.content/tests/utils.test.ts | 87 +++++++++++++++++++ entrypoints/cartograph.content/types.ts | 3 +- entrypoints/cartograph.content/utils.ts | 66 ++++++++++++++ 14 files changed, 209 insertions(+), 58 deletions(-) create mode 100644 entrypoints/cartograph.content/tests/utils.test.ts diff --git a/TODOS.md b/TODOS.md index c3210bd..35f5518 100644 --- a/TODOS.md +++ b/TODOS.md @@ -179,19 +179,6 @@ Currently, adding an item requires knowing the product URL or handle upfront. Wi - The `getProductByUrl()` bridge method already exists — search just feeds handles into it - MCP integration: if the Shopify Storefront MCP server is available, could use it for richer queries (collections, metafields, inventory levels) — but the extension should work standalone without MCP -### Adversarial Review Findings (from /ship 2026-03-23) - -**Priority:** P3 - -Issues identified by 4-pass adversarial review during ship. None are blocking but improve robustness: - -- **`applicable` field missing from discount_codes type** — `MetadataTab.svelte` reads `dc.applicable` but `types.ts:15` declares only `{ code, amount, type }`, so every code displays "Not applicable". Fix: add `applicable: boolean` -- **Product URL path validation too strict** — `getProductByUrl` in `cartograph-world.ts` requires `pathname.startsWith('/products/')`, rejecting `/en/products/` and `/collections/*/products/`. Fix: normalize to extract the handle -- **Shipping rate polling timeout mismatch** — `cartograph-world.ts` polls 10 × 500ms = 5s while the client allows 30s (`SHIPPING_TIMEOUT_MS`). Fix: raise `maxAttempts` to ~20 -- **Mount failure bricks overlay** — `open()` in `cartograph.content/index.ts` sets `mounted = true` before awaiting the dynamic import, so a failed import leaves it stuck true and the overlay never opens again. Fix: try/catch that resets the flag -- **USD-only currency formatting** — prices hardcoded as `$` + `toFixed(2)`, ignores `cart.currency` -- **Quantity input capped at 99** — silent clamp on existing items with qty > 99 - ## Storefront Inspector ### Store Health Check diff --git a/entrypoints/cartograph-world.ts b/entrypoints/cartograph-world.ts index 619289e..c7cf402 100644 --- a/entrypoints/cartograph-world.ts +++ b/entrypoints/cartograph-world.ts @@ -8,6 +8,7 @@ import type { CartMethods, ProductData } from './cartograph.content/types'; +import { resolveProductPath, SHIPPING_POLL_BUDGET_MS, SHIPPING_POLL_INTERVAL_MS } from './cartograph.content/utils'; import { createBridgeServer } from '~/utils/mainWorldBridge'; export default defineUnlistedScript(() => { @@ -93,11 +94,10 @@ export default defineUnlistedScript(() => { }); if (!prepRes.ok) throw new Error(`Failed to prepare shipping rates: ${prepRes.status}`); - const maxAttempts = 10; - const pollInterval = 500; + const deadline = Date.now() + SHIPPING_POLL_BUDGET_MS; - for (let attempt = 0; attempt < maxAttempts; attempt++) { - await new Promise((resolve) => setTimeout(resolve, pollInterval)); + while (Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, SHIPPING_POLL_INTERVAL_MS)); const ratesRes = await fetchWithRetry(`/cart/async_shipping_rates.json?${params}`); if (ratesRes.status === 202) continue; @@ -111,26 +111,10 @@ export default defineUnlistedScript(() => { } async function getProductByUrl(url: string): Promise { - let pathname: string; - - // If it looks like a URL (contains / or .), parse it; otherwise treat as a plain handle - if (url.includes('/') || url.includes('.')) { - const parsed = new URL(url, window.location.origin); - pathname = parsed.pathname; - } else { - pathname = `/products/${url}`; - } - - if (!pathname.endsWith('.js')) { - pathname = pathname.replace(/\/$/, '') + '.js'; - } - - // Security: only allow fetching product endpoints - if (!pathname.startsWith('/products/')) { - throw new Error('Invalid product URL: must be a /products/ path'); - } + const path = resolveProductPath(url, window.location.origin); + if (!path) throw new Error('Invalid product URL: must be a /products/ path'); - const res = await fetchWithRetry(pathname); + const res = await fetchWithRetry(path); if (!res.ok) throw new Error(`Failed to fetch product: ${res.status}`); const data = await res.json(); diff --git a/entrypoints/cartograph.content/App.svelte b/entrypoints/cartograph.content/App.svelte index 1aa2aa7..dcea1e3 100644 --- a/entrypoints/cartograph.content/App.svelte +++ b/entrypoints/cartograph.content/App.svelte @@ -265,6 +265,7 @@ /> {:else if activeTab === 'add'} { await mutate(() => api.addItem(payload)); activeTab = 'items'; diff --git a/entrypoints/cartograph.content/cartApi.ts b/entrypoints/cartograph.content/cartApi.ts index 39b69e4..94ff52b 100644 --- a/entrypoints/cartograph.content/cartApi.ts +++ b/entrypoints/cartograph.content/cartApi.ts @@ -9,8 +9,7 @@ import type { ShippingRate, ProductData } from './types'; - -const SHIPPING_TIMEOUT_MS = 30_000; +import { SHIPPING_TIMEOUT_MS } from './utils'; const cartBridge = createBridgeClient('cart'); diff --git a/entrypoints/cartograph.content/components/AddItemTab.svelte b/entrypoints/cartograph.content/components/AddItemTab.svelte index a2bf8fb..d74f37d 100644 --- a/entrypoints/cartograph.content/components/AddItemTab.svelte +++ b/entrypoints/cartograph.content/components/AddItemTab.svelte @@ -2,11 +2,14 @@ import type { AddItemPayload, ProductData, ProductVariant } from '../types'; import QuantityInput from './QuantityInput.svelte'; import KeyValueEditor from './KeyValueEditor.svelte'; + import { formatMoney, MAX_QUANTITY } from '../utils'; let { + currency, onAddItem, onFetchProduct, }: { + currency: string; onAddItem: (payload: AddItemPayload) => Promise; onFetchProduct: (url: string) => Promise; } = $props(); @@ -157,7 +160,7 @@ onclick={() => selectVariant(variant)} > {variant.title} - ${(variant.price / 100).toFixed(2)} + {formatMoney(variant.price, currency)} {#if !variant.available} Out of stock {/if} @@ -176,7 +179,7 @@
Quantity - +
{#if selectedVariant && selectedVariant.selling_plan_allocations.length > 0} @@ -194,7 +197,7 @@ {#each availablePlans as plan} {@const allocation = selectedVariant.selling_plan_allocations.find(a => a.selling_plan_id === plan.id)} {/each} @@ -228,7 +231,7 @@ {/if} - {quantity}× {selectedVariant.title} — ${((selectedVariant.price * quantity) / 100).toFixed(2)} + {quantity}× {selectedVariant.title} — {formatMoney(selectedVariant.price * quantity, currency)}
diff --git a/entrypoints/cartograph.content/components/ItemsTab.svelte b/entrypoints/cartograph.content/components/ItemsTab.svelte index d076667..709d265 100644 --- a/entrypoints/cartograph.content/components/ItemsTab.svelte +++ b/entrypoints/cartograph.content/components/ItemsTab.svelte @@ -1,6 +1,6 @@