Skip to content

fix(cli): preserve CSS priority during full-page capture - #5316

Open
user-github-me wants to merge 1 commit into
heygen-com:mainfrom
user-github-me:fix/capture-plate-css-priority
Open

user-github-me wants to merge 1 commit into
heygen-com:mainfrom
user-github-me:fix/capture-plate-css-priority

Conversation

@user-github-me

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

Copy link
Copy Markdown
Contributor

What

Respect CSS priority while temporarily neutralizing fixed and sticky elements for website capture's full-page screenshot, then restore their original inline declaration.

Why

With position: fixed !important in a stylesheet, assigning an ordinary inline position: static leaves the header fixed in full-page.png. An inline position: fixed !important also loses its priority when capture restores just its value. If the stylesheet contains position: relative !important, that header moves into document flow and disappears from the viewport before subsequent asset and HTML extraction.

Related work

#2892 established the full-page plate and its temporary position override. #5171 added screenshot limits and interruption reporting. Open #4949 addresses consent-triggered navigation in captureScrollScreenshots; this change is confined to captureFullPagePlate and its CSS restoration. No existing implementation of the priority correction was found.

How

Save the inline position's priority alongside its value, override with position: static !important for the plate, and restore both in the existing cleanup path. Remove both temporary markers afterward. The helper's page type lists the two methods it actually uses, allowing the tests to exercise the complete browser expressions without a page type assertion.

Test plan

  • Eight regression/control cases execute the real browser expressions against a CSS DOM: important stylesheet and inline fixed/sticky rules, restoration after screenshot failure, height omission and exhausted budget, plus ordinary-position and untouched-element controls. Seven fail on current main; 26 controls pass across the initial three-file run.
  • All 37 tests pass across screenshot capture, degradation, core extraction and post-extraction handling.
  • Chrome 152 reproduces both failures on current main. Fixed source and the built CLI function neutralize the header during capture and restore the original computed style, priority and viewport pixels afterward. An ordinary sticky control also remains unchanged.
  • Full workspace build, CLI rebuild and typecheck, root lint, changed-file lint/format and test reachability pass.
  • Fallow reports no introduced issues or duplicate groups; one inherited capture complexity finding is excluded by its standard gate.
  • Normal commit hooks and comment citation/share checks pass.

GitHub CI

All 81 latest checks have completed on signed head 7098cec367e1ba3c9daa4350e844a4ea95966d97: 68 successful and 13 skipped, including CodeQL, Windows render, build, tests, regression and all Studio edit-accuracy shards.

@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.

An inline position: static loses to a stylesheet position: fixed !important, and plain el.style.position = value drops an inline !important on restore; this saves the priority, overrides with setProperty('position', 'static', 'important'), and restores value plus priority in the existing cleanup path. I checked main: captureFullPagePlate still uses the plain assignment in both places, and the unrelated changes main made elsewhere in the file do not touch this function, so the fix is still needed and merges cleanly. Restoring an empty value via setProperty('position', '', '') removes the inline declaration, which matches the old behavior for elements that had none. The happy-dom tests run the real evaluate strings and assert computed position during the screenshot and value plus priority afterward, including the screenshot-failure cleanup path. Need: yes · Size: ok · Reuse: existing marker and cleanup path · 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
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 24, 2026

This branch has not been deployed

No deployments
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