Skip to content

fix(cli): capture images declared only through srcset - #5298

Merged
jrusso1020 merged 4 commits into
heygen-com:mainfrom
user-github-me:fix/capture-srcset-only-images
Oct 24, 2026
Merged

jrusso1020 merged 4 commits into
heygen-com:mainfrom
user-github-me:fix/capture-srcset-only-images

Conversation

@user-github-me

@user-github-me user-github-me commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Website capture skips <img> elements whose images are declared only through srcset, leaving valid image candidates out of the asset catalog. Include images with either src or srcset in the existing discovery pass. Preserve DOM context, image descriptions, optimized-variant deduplication, and <picture> source handling.

The merge with current main retains its validated network transport for GIF metadata, public redirect handling, and refusal of private-address redirects.

Validation

  • 102 capture, downloader, network, HTTP/TLS integration, and extraction tests pass. Coverage includes srcset-only images, largest optimized candidates, merged src/srcset references, picture sources, GIF metadata, and blocked redirects.
  • Full workspace build, CLI typecheck, lint, formatting, test reachability, comment checks, and normal commit hooks pass.

The published tree matches the tested tree, and all newly published commits have verified GitHub signatures.

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verified the bug is still live on main: the image pass in assetCataloger.ts still selects only img[src], so an <img> declared purely through srcset never reaches the existing srcset split. Widening the selector to img[src], img[srcset] is the minimal fix, and it is safe for srcset-only elements because img.src is the empty string there and add() already drops empty URLs, so no bogus page-URL asset appears. The new happy-dom tests drive the real catalogAssets script (made possible by narrowing the parameter to Pick<Page, "evaluate">, which every caller still satisfies) and cover srcset-only candidates, Next.js variant dedup keeping the largest width, src/srcset merging, and the no-source <picture> img case. Need: yes · Size: ok · Reuse: uses the existing srcset loop and dedup · Simpler: already minimal.

— Rames

@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 24, 2026
@jrusso1020
jrusso1020 removed this pull request from the merge queue due to a manual request Oct 24, 2026
@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 24, 2026
Merged via the queue into heygen-com:main with commit 1001a79 Oct 24, 2026
107 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants