fix: load oversight URL thumbnails only after successful probe

[davidnuescheler]

Summary

• Load URL facet thumbnails asynchronously with an Image() probe so failed _ogimage responses never render broken <img> placeholders
• Skip thumbnail loading entirely when the 404 checkpoint filter is active, since those pages are expected to have no og:image
• Preserve favicon fallback for facets that set favicon="true" (e.g. Enter Source)

Test plan

• Open Oversight Explorer preview with a domain that previously showed broken thumbnails
• Confirm URL facet rows show thumbnails only when _ogimage succeeds — no broken icons or "thumb" alt text
• Filter by the 404 checkpoint and confirm the URL facet shows links only (no thumbnail attempts)
• Verify Enter Source facet still falls back to favicons when og:image is unavailable
• npm test -- tests/tools/optel/oversight/link-facet.test.js
• npm run lint:js

Made with Cursor

[aem-code-sync]

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

• Re-run all PSI checks
• Re-run failed PSI checks
• Re-sync branch

Commits

• 9176a99 ✅ (latest)

[aem-code-sync]

	Page	Scores	Audits	Google
📱	/tools/optel/oversight/explorer.html
🖥️	/tools/optel/oversight/explorer.html

[claude]

BLOCKING (likely unintended scope creep): This fallback branch handles every link-facet that renders an http(s):// value without thumbnail="true" (e.g. click.target, error.source, redirect.source, and any thumbnail="false" facet such as enter.source on list.html/table.html/share.html/single.html, or url/redirect.source on redirectperf.html/cwvperf.html). Previously it rendered the plain escaped URL as anchor text; this change switches it to labelURLParts(...), which decomposes the URL into <span class="protocol">/<span class="hostname">/<span class="pathname">/etc.

That's a behavior change unrelated to the PR's stated scope (async thumbnail probing / 404-checkpoint skip / favicon fallback), and it isn't covered by the new tests (which only exercise the extracted helper functions, not createLabelHTML). It also interacts with existing CSS such as #facets link-facet[facet="enter.source"] a .hostname { display: inline } / .pathname { display: none } in rum-slicer.css, which previously only applied when thumbnail="true" — after this change it will also silently truncate enter.source URLs to hostname-only wherever thumbnail="false" is set.

If this was unintentional, please revert to the original plain-text rendering:

Suggested change

	}
	return `<a href="${escapeHTML(labelText)}" target="_new">${escapeHTML(labelText)}</a>`;

If it's intentional, please call it out in the PR description and add a test/manual check for the affected non-thumbnail facets (enter.source on list/table/share/single, click.target, error.source, redirect.source).

[claude]

Code Review

Reviewed the async thumbnail-probe rewrite in tools/optel/oversight/elements/link-facet.js and its new tests in tests/tools/optel/oversight/link-facet.test.js.

What's good

• Probing with Image() before inserting an <img> correctly avoids broken og:image placeholders — a real fix.
• Consolidating the two near-duplicate thumbnailAtt branches also fixes a pre-existing bug: the old https://-only branch always ran ahead of the favicon-fallback branch and never honored favicon="true", so enter.source (https) never actually got its favicon fallback despite the attribute. The unified isThumbnailUrl/appendThumbnail path fixes that.
• Good new unit test coverage for the extracted helpers (isThumbnailUrl, getOgImageUrl, getFaviconUrl, is404CheckpointActive, appendThumbnail), including the favicon-fallback and probe-failure paths.
• Using DOM property assignment (img.src, img.title) instead of string-built HTML avoids any injection risk from labelText.

Issues Found

BLOCKING:

• tools/optel/oversight/elements/link-facet.js (inline comment) — the non-thumbnail http(s):// fallback in createLabelHTML was changed from plain escaped URL text to labelURLParts(...) (hostname/pathname decomposition). This affects every link-facet that renders URLs without thumbnail="true" — click.target, error.source, redirect.source, and any thumbnail="false" facet (enter.source on list.html/table.html/share.html/single.html, url/redirect.source on redirectperf.html/cwvperf.html). It's unrelated to this PR's stated purpose, isn't covered by the new tests, and combined with the existing enter.source CSS (.hostname { display: inline } / .pathname { display: none }) could silently truncate those URLs to hostname-only wherever thumbnails are off. Please confirm whether this is intentional; if not, revert per the inline suggestion.

Verdict

REQUEST CHANGES — pending confirmation on the scope-creep item above.