diff --git a/packages/dom/src/serialize-pseudo-classes.js b/packages/dom/src/serialize-pseudo-classes.js index 023804afa..56fababa4 100644 --- a/packages/dom/src/serialize-pseudo-classes.js +++ b/packages/dom/src/serialize-pseudo-classes.js @@ -321,15 +321,89 @@ export function cleanupInteractiveStateMarkers(ctx) { ctx._liveMutations = []; } +// Layout / box-model / positioning properties are deliberately NOT baked into +// the pseudo-class computed-style block. serializePseudoClasses captures a +// single getComputedStyle snapshot taken at the SDK capture viewport; baking +// sizing/position as `!important` would freeze that viewport's geometry and +// override the page's responsive rules at every OTHER render width (PER-9836: +// a CTA anchor baked at `width: 134px !important` rendered content-width at +// Percy's 375px width instead of the site's mobile full-width). We only want +// the *paint* delta of the forced :hover/:active state (color, background, +// border, shadow, etc.); layout stays governed by the page's own stylesheets +// (which are serialized separately and re-evaluate media queries per width). +const NON_BAKED_STYLE_PROPS = new Set([ + // intrinsic sizing + 'width', 'height', 'min-width', 'max-width', 'min-height', 'max-height', + 'inline-size', 'block-size', 'min-inline-size', 'max-inline-size', + 'min-block-size', 'max-block-size', 'aspect-ratio', + // padding + 'padding', 'padding-top', 'padding-right', 'padding-bottom', 'padding-left', + 'padding-block', 'padding-block-start', 'padding-block-end', + 'padding-inline', 'padding-inline-start', 'padding-inline-end', + // margin + 'margin', 'margin-top', 'margin-right', 'margin-bottom', 'margin-left', + 'margin-block', 'margin-block-start', 'margin-block-end', + 'margin-inline', 'margin-inline-start', 'margin-inline-end', + // positioning + 'position', 'top', 'right', 'bottom', 'left', + 'inset', 'inset-block', 'inset-block-start', 'inset-block-end', + 'inset-inline', 'inset-inline-start', 'inset-inline-end', 'float', 'clear', + // box / display model + 'display', 'box-sizing', + // flexbox + 'flex', 'flex-basis', 'flex-grow', 'flex-shrink', 'flex-direction', + 'flex-flow', 'flex-wrap', + // grid + 'grid', 'grid-template', 'grid-template-columns', 'grid-template-rows', + 'grid-template-areas', 'grid-auto-columns', 'grid-auto-rows', + 'grid-auto-flow', 'grid-column', 'grid-row', 'grid-area', + // alignment / gaps / multicol + 'gap', 'row-gap', 'column-gap', 'order', + 'justify-content', 'justify-items', 'justify-self', + 'align-content', 'align-items', 'align-self', + 'place-content', 'place-items', 'place-self', + 'columns', 'column-count', 'column-width', + // transition timing — a static clone never animates; baking it only risks + // a load-time tween in the renderer, never a benefit + 'transition', 'transition-property', 'transition-duration', + 'transition-timing-function', 'transition-delay', 'transition-behavior' +]); + function stylesToCSSText(styles) { const decls = []; for (let i = 0; i < styles.length; i++) { const property = styles[i]; + if (NON_BAKED_STYLE_PROPS.has(property)) continue; decls.push(`${property}: ${styles.getPropertyValue(property)} !important;`); } return decls.join(' '); } +// Read an element's computed style with CSS transitions momentarily disabled, so +// the baked snapshot reflects the element's FINAL forced-state style rather than +// a mid-transition frame. The SDK typically forces :hover/:active (e.g. via a +// Playwright `.hover()`) and serializes immediately, which can catch a running +// transition partway — PER-9836: a hover snapshot baked a half-faded background +// (rgb(197,230,249), the mid-point of the rest→hover fade) instead of the settled +// hover colour. Setting `transition: none` makes getComputedStyle return the +// after-change value; the element's original inline transition is restored in a +// `finally` so the live page (SDK mode runs in the customer's tab) is untouched +// whether or not getComputedStyle throws. +function computedStyleTextSettled(win, element) { + // Element nodes always expose `.style`; if reading/mutating it ever throws + // (exotic node), the caller's try/catch downgrades to a warning as before. + const style = element.style; + const prevValue = style.getPropertyValue('transition'); + const prevPriority = style.getPropertyPriority('transition'); + style.setProperty('transition', 'none', 'important'); + try { + return stylesToCSSText(win.getComputedStyle(element)); + } finally { + if (prevValue) style.setProperty('transition', prevValue, prevPriority); + else style.removeProperty('transition'); + } +} + // Resolve a nested rule's selector against its parent per CSS Nesting. // The CSSOM serializes nested selectors with the implicit nesting selector // made explicit, so every nested selector — including each item of a @@ -530,8 +604,7 @@ export function serializePseudoClasses(ctx) { // fall back to the global window when ctx.dom is the top document or a // synthetic root that doesn't expose defaultView (e.g. tests). const win = ctx.dom.defaultView || window; - const computedStyles = win.getComputedStyle(element); - const cssText = stylesToCSSText(computedStyles); + const cssText = computedStyleTextSettled(win, element); cssRules.push(`[${PSEUDO_ELEMENT_MARKER_ATTR}="${percyElementId}"] { ${cssText} }`); } catch (err) { console.warn('Could not get computed styles for element', element, err); diff --git a/packages/dom/test/serialize-pseudo-classes.test.js b/packages/dom/test/serialize-pseudo-classes.test.js index ad8cded93..96a4cada2 100644 --- a/packages/dom/test/serialize-pseudo-classes.test.js +++ b/packages/dom/test/serialize-pseudo-classes.test.js @@ -95,6 +95,63 @@ describe('serialize-pseudo-classes', () => { expect(style.textContent).toContain('color: rgb(255, 0, 0) !important'); }); + // PER-9836: baking layout/box-model props at the capture viewport froze + // a CTA's width and overrode the page's responsive rules at other Percy + // render widths. Only the paint delta of the forced state should be baked. + it('bakes paint properties but excludes layout/box-model/transition properties', () => { + const foo = ctx.dom.getElementById('foo'); + foo.style.width = '234px'; + foo.style.padding = '11px'; + foo.style.margin = '7px'; + foo.style.transition = 'all 0.3s ease'; + // nosemgrep: javascript.browser.security.insecure-document-method.insecure-document-method + ctx.clone.head.innerHTML = ''; + serializePseudoClasses(ctx); + const css = ctx.clone.head.querySelector('style[data-percy-pseudo-class-styles="true"]').textContent; + + // paint/state props are still baked + expect(css).toContain('color: rgb(255, 0, 0) !important'); + + // layout / box-model / positioning / transition props are NOT baked + // (boundary `{ ` or `; ` before the property avoids matching longhands + // like `border-top-width`) + expect(css).not.toMatch(/(?:\{ |; )width:/); + expect(css).not.toMatch(/(?:\{ |; )height:/); + expect(css).not.toMatch(/(?:\{ |; )padding(?:-\w+)?:/); + expect(css).not.toMatch(/(?:\{ |; )margin(?:-\w+)?:/); + expect(css).not.toMatch(/(?:\{ |; )position:/); + expect(css).not.toMatch(/(?:\{ |; )display:/); + expect(css).not.toMatch(/(?:\{ |; )flex(?:-\w+)?:/); + expect(css).not.toMatch(/(?:\{ |; )transition(?:-\w+)?:/); + }); + + // PER-9836 Fix B: the SDK forces :hover then serializes immediately, which + // can catch a running CSS transition partway. We disable transitions while + // reading computed style (so the settled forced-state value is baked) and + // restore the element's inline transition afterward. + it('disables transitions while reading computed style and restores them after', () => { + const foo = ctx.dom.getElementById('foo'); + foo.style.setProperty('transition', 'all 1s ease'); + let transitionDuringRead = 'NOT_CALLED'; + const origGCS = window.getComputedStyle; + window.getComputedStyle = (el) => { + if (el === foo) transitionDuringRead = foo.style.getPropertyValue('transition'); + return { length: 1, 0: 'color', getPropertyValue: () => 'red' }; + }; + try { + // nosemgrep: javascript.browser.security.insecure-document-method.insecure-document-method + ctx.clone.head.innerHTML = ''; + serializePseudoClasses(ctx); + } finally { + window.getComputedStyle = origGCS; + } + // transitions were OFF during the computed-style read... + expect(transitionDuringRead).toBe('none'); + // ...and the element's original inline transition is restored afterward + expect(foo.style.getPropertyValue('transition')).toContain('1s'); + expect(foo.style.getPropertyValue('transition')).not.toBe('none'); + }); + it('adds attributes in cloned dom as well', () => { let orginalBody = ctx.dom.body.innerHTML; let originalClonedBody = ctx.clone.body.innerHTML;