From 886d26875fd313baf175f7f86268770852d752d5 Mon Sep 17 00:00:00 2001 From: Matt Schile Date: Thu, 10 Feb 2022 21:29:45 -0700 Subject: [PATCH 1/4] adding support for multi-domain screenshot blackout --- .../commands/multi_domain_screenshot.spec.ts | 20 ++- packages/driver/package.json | 1 + packages/driver/src/cy/commands/screenshot.ts | 38 ++++- packages/driver/src/dom/animation.ts | 21 +++ packages/driver/src/dom/blackout.ts | 58 +++++++ packages/driver/src/dom/index.ts | 8 + packages/runner-shared/src/dimensions.js | 89 +++++++++++ packages/runner-shared/src/dom.js | 148 +----------------- .../runner-shared/src/iframe/aut-iframe.js | 34 ---- packages/runner/src/iframe/iframes.jsx | 2 - 10 files changed, 229 insertions(+), 190 deletions(-) create mode 100644 packages/driver/src/dom/animation.ts create mode 100644 packages/driver/src/dom/blackout.ts create mode 100644 packages/runner-shared/src/dimensions.js diff --git a/packages/driver/cypress/integration/e2e/multi-domain/commands/multi_domain_screenshot.spec.ts b/packages/driver/cypress/integration/e2e/multi-domain/commands/multi_domain_screenshot.spec.ts index 354cd9a22651..72ea354ebfc3 100644 --- a/packages/driver/cypress/integration/e2e/multi-domain/commands/multi_domain_screenshot.spec.ts +++ b/packages/driver/cypress/integration/e2e/multi-domain/commands/multi_domain_screenshot.spec.ts @@ -81,11 +81,21 @@ context('screenshot specs', { experimentalSessionSupport: true, experimentalMult }) }) - // FIXME: Add support for blackout option. Has cross-domain issue due to the blackout logic - // being called from top instead of the spec bridge - it.skip('supports the blackout option', () => { - cy.switchToDomain('foobar.com', () => { - cy.screenshot({ blackout: ['a'] }) + it('supports the blackout option', () => { + cy.switchToDomain('foobar.com', [this.serverResult], ([serverResult]) => { + cy.stub(Cypress, 'automation').withArgs('take:screenshot').resolves(serverResult) + + cy.screenshot({ + blackout: ['.short-element'], + onBeforeScreenshot: ($el) => { + const $blackoutElement = $el.find('.__cypress-blackout') + const $shortElement = $el.find('.short-element') + + expect($blackoutElement.outerHeight()).to.equal($shortElement.outerHeight()) + expect($blackoutElement.outerWidth()).to.equal($shortElement.outerWidth()) + expect($blackoutElement.offset()).to.deep.equal($shortElement.offset()) + }, + }) }) }) diff --git a/packages/driver/package.json b/packages/driver/package.json index 946964e6c388..22d65e902e5a 100644 --- a/packages/driver/package.json +++ b/packages/driver/package.json @@ -21,6 +21,7 @@ "@packages/network": "0.0.0-development", "@packages/resolve-dist": "0.0.0-development", "@packages/runner": "0.0.0-development", + "@packages/runner-shared": "0.0.0-development", "@packages/server": "0.0.0-development", "@packages/ts": "0.0.0-development", "@rollup/plugin-node-resolve": "^13.0.4", diff --git a/packages/driver/src/cy/commands/screenshot.ts b/packages/driver/src/cy/commands/screenshot.ts index 53987c587c0b..4d6dcc334d79 100644 --- a/packages/driver/src/cy/commands/screenshot.ts +++ b/packages/driver/src/cy/commands/screenshot.ts @@ -330,7 +330,7 @@ const takeScreenshot = (Cypress, state, screenshotConfig, options: TakeScreensho } } - const before = () => { + const before = ($el) => { return Promise.try(() => { if (disableTimersAndAnimations) { return cy.pauseTimers(true) @@ -339,11 +339,41 @@ const takeScreenshot = (Cypress, state, screenshotConfig, options: TakeScreensho return null }) .then(() => { + // could fail if iframe is cross-origin, so fail gracefully + try { + if (disableTimersAndAnimations) { + $dom.addCssAnimationDisabler($el) + } + + _.each(getBlackout(screenshotConfig), (selector) => { + $dom.addBlackouts($el, selector) + }) + } catch (err) { + /* eslint-disable no-console */ + console.error('Failed to modify app dom:') + console.error(err) + /* eslint-disable no-console */ + } + return sendAsync('before:screenshot', getOptions(true)) }) } - const after = () => { + const after = ($el) => { + // could fail if iframe is cross-origin, so fail gracefully + try { + if (disableTimersAndAnimations) { + $dom.removeCssAnimationDisabler($el) + } + + $dom.removeBlackouts($el) + } catch (err) { + /* eslint-disable no-console */ + console.error('Failed to modify app dom:') + console.error(err) + /* eslint-disable no-console */ + } + send('after:screenshot', getOptions(false)) return Promise.try(() => { @@ -380,7 +410,7 @@ const takeScreenshot = (Cypress, state, screenshotConfig, options: TakeScreensho ? subject : $dom.wrap(state('document').documentElement) - return before() + return before($el) .then(() => { if (onBeforeScreenshot) { onBeforeScreenshot.call(state('ctx'), $el) @@ -407,7 +437,7 @@ const takeScreenshot = (Cypress, state, screenshotConfig, options: TakeScreensho return props }) - .finally(after) + .finally(() => after($el)) } export default function (Commands, Cypress, cy, state, config) { diff --git a/packages/driver/src/dom/animation.ts b/packages/driver/src/dom/animation.ts new file mode 100644 index 000000000000..e18c2b2c2a31 --- /dev/null +++ b/packages/driver/src/dom/animation.ts @@ -0,0 +1,21 @@ +import $ from 'jquery' + +function addCssAnimationDisabler ($body) { + $(` + + `).appendTo($body) +} + +function removeCssAnimationDisabler ($body) { + $body.find('#__cypress-animation-disabler').remove() +} + +export default { + addCssAnimationDisabler, + removeCssAnimationDisabler, +} diff --git a/packages/driver/src/dom/blackout.ts b/packages/driver/src/dom/blackout.ts new file mode 100644 index 000000000000..8c3085ff50bc --- /dev/null +++ b/packages/driver/src/dom/blackout.ts @@ -0,0 +1,58 @@ +import $ from 'jquery' +import $dimensions from '@packages/runner-shared/src/dimensions' + +const resetStyles = ` + border: none !important; + margin: 0 !important; + padding: 0 !important; +` + +const styles = (styleString) => { + return styleString.replace(/\s*\n\s*/g, '') +} + +function addBlackoutForElement ($body, $el) { + const dimensions = $dimensions.getElementDimensions($el) + const width = dimensions.widthWithBorder + const height = dimensions.heightWithBorder + const top = dimensions.offset.top + const left = dimensions.offset.left + + const style = styles(` + ${resetStyles} + position: absolute; + top: ${top}px; + left: ${left}px; + width: ${width}px; + height: ${height}px; + background-color: black; + z-index: 2147483647; + `) + + $(`
`).appendTo($body) +} + +function addBlackouts ($body, selector) { + let $el + + try { + $el = $body.find(selector) + if (!$el.length) return + } catch (err) { + // if it's an invalid selector, just ignore it + return + } + + $el.each(function () { + addBlackoutForElement($body, $(this)) + }) +} + +function removeBlackouts ($body) { + $body.find('.__cypress-blackout').remove() +} + +export default { + addBlackouts, + removeBlackouts, +} diff --git a/packages/driver/src/dom/index.ts b/packages/driver/src/dom/index.ts index 92bbc4bd0ea8..4018e3e6ed4f 100644 --- a/packages/driver/src/dom/index.ts +++ b/packages/driver/src/dom/index.ts @@ -5,6 +5,8 @@ import $elements from './elements' import $coordinates from './coordinates' import $selection from './selection' import $visibility from './visibility' +import $blackout from './blackout' +import $animation from './animation' const { isWindow, getWindowByElement } = $window const { isDocument, getDocumentFromElement } = $document @@ -13,6 +15,8 @@ const { isVisible, isHidden, isStrictlyHidden, isHiddenByAncestors, getReasonIsH const { isInputType, isFocusable, isElement, isScrollable, isFocused, stringify, getElements, getContainsSelector, getFirstDeepestElement, getInputFromLabel, isDetached, isAttached, isTextLike, isSelector, isDescendent, getFirstFixedOrStickyPositionParent, getFirstStickyPositionParent, getFirstScrollableParent, isUndefinedOrHTMLBodyDoc, elementFromPoint, getParent, findAllShadowRoots, isWithinShadowRoot, getHostContenteditable } = $elements const { getCoordsByPosition, getElementPositioning, getElementCoordinatesByPosition, getElementAtPointFromViewport, getElementCoordinatesByPositionRelativeToXY } = $coordinates const { getSelectionBounds } = $selection +const { addBlackouts, removeBlackouts } = $blackout +const { removeCssAnimationDisabler, addCssAnimationDisabler } = $animation const isDom = (obj) => { return isElement(obj) || isWindow(obj) || isDocument(obj) @@ -24,6 +28,10 @@ const isDom = (obj) => { // purposes or for overriding. Everything else // can be tucked away behind these interfaces. export default { + removeBlackouts, + addBlackouts, + removeCssAnimationDisabler, + addCssAnimationDisabler, wrap, isW3CFocusable, isW3CRendered, diff --git a/packages/runner-shared/src/dimensions.js b/packages/runner-shared/src/dimensions.js new file mode 100644 index 000000000000..13aa4ddd7698 --- /dev/null +++ b/packages/runner-shared/src/dimensions.js @@ -0,0 +1,89 @@ +import _ from 'lodash' + +const getElementDimensions = ($el) => { + const el = $el.get(0) + + const { offsetHeight, offsetWidth } = el + + const box = { + // offset disregards margin but takes into account border + padding + offset: $el.offset(), + // dont use jquery here for width/height because it uses getBoundingClientRect() which returns scaled values. + // TODO: switch back to using jquery when upgrading to jquery 3.4+ + paddingTop: getPadding($el, 'top'), + paddingRight: getPadding($el, 'right'), + paddingBottom: getPadding($el, 'bottom'), + paddingLeft: getPadding($el, 'left'), + borderTop: getBorder($el, 'top'), + borderRight: getBorder($el, 'right'), + borderBottom: getBorder($el, 'bottom'), + borderLeft: getBorder($el, 'left'), + marginTop: getMargin($el, 'top'), + marginRight: getMargin($el, 'right'), + marginBottom: getMargin($el, 'bottom'), + marginLeft: getMargin($el, 'left'), + } + + // NOTE: offsetWidth/height always give us content + padding + border, so subtract them + // to get the true "clientHeight" and "clientWidth". + // we CANNOT just use "clientHeight" and "clientWidth" because those always return 0 + // for inline elements >_< + box.width = offsetWidth - (box.paddingLeft + box.paddingRight + box.borderLeft + box.borderRight) + box.height = offsetHeight - (box.paddingTop + box.paddingBottom + box.borderTop + box.borderBottom) + + // innerHeight: Get the current computed height for the first + // element in the set of matched elements, including padding but not border. + + // outerHeight: Get the current computed height for the first + // element in the set of matched elements, including padding, border, + // and optionally margin. Returns a number (without 'px') representation + // of the value or null if called on an empty set of elements. + box.heightWithPadding = box.height + box.paddingTop + box.paddingBottom + + box.heightWithBorder = box.heightWithPadding + box.borderTop + box.borderBottom + + box.heightWithMargin = box.heightWithBorder + box.marginTop + box.marginBottom + + box.widthWithPadding = box.width + box.paddingLeft + box.paddingRight + + box.widthWithBorder = box.widthWithPadding + box.borderLeft + box.borderRight + + box.widthWithMargin = box.widthWithBorder + box.marginLeft + box.marginRight + + return box +} + +const getNumAttrValue = ($el, attr) => { + // nuke anything thats not a number or a negative symbol + const num = _.toNumber($el.css(attr).replace(/[^0-9\.-]+/, '')) + + if (!_.isFinite(num)) { + throw new Error('Element attr did not return a valid number') + } + + return num +} + +const getPadding = ($el, dir) => { + return getNumAttrValue($el, `padding-${dir}`) +} + +const getBorder = ($el, dir) => { + return getNumAttrValue($el, `border-${dir}-width`) +} + +const getMargin = ($el, dir) => { + return getNumAttrValue($el, `margin-${dir}`) +} + +const getOuterSize = ($el) => { + return { + width: $el.outerWidth(true), + height: $el.outerHeight(true), + } +} + +export default { + getOuterSize, + getElementDimensions, +} diff --git a/packages/runner-shared/src/dom.js b/packages/runner-shared/src/dom.js index 9ec11ab05d66..547ae45a93e5 100644 --- a/packages/runner-shared/src/dom.js +++ b/packages/runner-shared/src/dom.js @@ -2,6 +2,7 @@ import _ from 'lodash' import retargetEvents from 'react-shadow-dom-retarget-events' import $Cypress from '@packages/driver' +import $dimensions from './dimensions' import { selectorPlaygroundHighlight } from './selector-playground/highlight' import { studioAssertionsMenu } from './studio/assertions-menu' // The '!' tells webpack to disable normal loaders, and keep loaders with `enforce: 'pre'` and `enforce: 'post'` @@ -72,7 +73,7 @@ function addHitBoxLayer (coords, $body) { function addElementBoxModelLayers ($el, $body) { $body = $body || $('body') - const dimensions = getElementDimensions($el) + const dimensions = $dimensions.getElementDimensions($el) const $container = $('
') .css({ @@ -306,89 +307,6 @@ function getZIndex (el) { return _.toNumber(el.css('zIndex')) } -function getElementDimensions ($el) { - const el = $el.get(0) - - const { offsetHeight, offsetWidth } = el - - const box = { - offset: $el.offset(), // offset disregards margin but takes into account border + padding - // dont use jquery here for width/height because it uses getBoundingClientRect() which returns scaled values. - // TODO: switch back to using jquery when upgrading to jquery 3.4+ - paddingTop: getPadding($el, 'top'), - paddingRight: getPadding($el, 'right'), - paddingBottom: getPadding($el, 'bottom'), - paddingLeft: getPadding($el, 'left'), - borderTop: getBorder($el, 'top'), - borderRight: getBorder($el, 'right'), - borderBottom: getBorder($el, 'bottom'), - borderLeft: getBorder($el, 'left'), - marginTop: getMargin($el, 'top'), - marginRight: getMargin($el, 'right'), - marginBottom: getMargin($el, 'bottom'), - marginLeft: getMargin($el, 'left'), - } - - // NOTE: offsetWidth/height always give us content + padding + border, so subtract them - // to get the true "clientHeight" and "clientWidth". - // we CANNOT just use "clientHeight" and "clientWidth" because those always return 0 - // for inline elements >_< - // - box.width = offsetWidth - (box.paddingLeft + box.paddingRight + box.borderLeft + box.borderRight) - box.height = offsetHeight - (box.paddingTop + box.paddingBottom + box.borderTop + box.borderBottom) - - // innerHeight: Get the current computed height for the first - // element in the set of matched elements, including padding but not border. - - // outerHeight: Get the current computed height for the first - // element in the set of matched elements, including padding, border, - // and optionally margin. Returns a number (without 'px') representation - // of the value or null if called on an empty set of elements. - box.heightWithPadding = box.height + box.paddingTop + box.paddingBottom - - box.heightWithBorder = box.heightWithPadding + box.borderTop + box.borderBottom - - box.heightWithMargin = box.heightWithBorder + box.marginTop + box.marginBottom - - box.widthWithPadding = box.width + box.paddingLeft + box.paddingRight - - box.widthWithBorder = box.widthWithPadding + box.borderLeft + box.borderRight - - box.widthWithMargin = box.widthWithBorder + box.marginLeft + box.marginRight - - return box -} - -function getNumAttrValue ($el, attr) { - // nuke anything thats not a number or a negative symbol - const num = _.toNumber($el.css(attr).replace(/[^0-9\.-]+/, '')) - - if (!_.isFinite(num)) { - throw new Error('Element attr did not return a valid number') - } - - return num -} - -function getPadding ($el, dir) { - return getNumAttrValue($el, `padding-${dir}`) -} - -function getBorder ($el, dir) { - return getNumAttrValue($el, `border-${dir}-width`) -} - -function getMargin ($el, dir) { - return getNumAttrValue($el, `margin-${dir}`) -} - -function getOuterSize ($el) { - return { - width: $el.outerWidth(true), - height: $el.outerHeight(true), - } -} - function isInViewport (win, el) { let rect = el.getBoundingClientRect() @@ -428,73 +346,13 @@ function getElementsForSelector ({ $root, selector, method, cypressDom }) { return $el } -function addCssAnimationDisabler ($body) { - $(` - - `).appendTo($body) -} - -function removeCssAnimationDisabler ($body) { - $body.find('#__cypress-animation-disabler').remove() -} - -function addBlackoutForElement ($body, $el) { - const dimensions = getElementDimensions($el) - const width = dimensions.widthWithBorder - const height = dimensions.heightWithBorder - const top = dimensions.offset.top - const left = dimensions.offset.left - - const style = styles(` - ${resetStyles} - position: absolute; - top: ${top}px; - left: ${left}px; - width: ${width}px; - height: ${height}px; - background-color: black; - z-index: 2147483647; - `) - - $(`
`).appendTo($body) -} - -function addBlackout ($body, selector) { - let $el - - try { - $el = $body.find(selector) - if (!$el.length) return - } catch (err) { - // if it's an invalid selector, just ignore it - return - } - - $el.each(function () { - addBlackoutForElement($body, $(this)) - }) -} - -function removeBlackouts ($body) { - $body.find('.__cypress-blackout').remove() -} - export const dom = { - addBlackout, - removeBlackouts, addElementBoxModelLayers, addHitBoxLayer, addOrUpdateSelectorPlaygroundHighlight, openStudioAssertionsMenu, closeStudioAssertionsMenu, - addCssAnimationDisabler, - removeCssAnimationDisabler, getElementsForSelector, - getOuterSize, + getOuterSize: $dimensions.getOuterSize, scrollIntoView, } diff --git a/packages/runner-shared/src/iframe/aut-iframe.js b/packages/runner-shared/src/iframe/aut-iframe.js index 30faf2aa19ef..96275c00221f 100644 --- a/packages/runner-shared/src/iframe/aut-iframe.js +++ b/packages/runner-shared/src/iframe/aut-iframe.js @@ -394,40 +394,6 @@ export class AutIframe { }) } - beforeScreenshot = (config) => { - // could fail if iframe is cross-origin, so fail gracefully - try { - if (config.disableTimersAndAnimations) { - dom.addCssAnimationDisabler(this._body()) - } - - _.each(config.blackout, (selector) => { - dom.addBlackout(this._body(), selector) - }) - } catch (err) { - /* eslint-disable no-console */ - console.error('Failed to modify app dom:') - console.error(err) - /* eslint-disable no-console */ - } - } - - afterScreenshot = (config) => { - // could fail if iframe is cross-origin, so fail gracefully - try { - if (config.disableTimersAndAnimations) { - dom.removeCssAnimationDisabler(this._body()) - } - - dom.removeBlackouts(this._body()) - } catch (err) { - /* eslint-disable no-console */ - console.error('Failed to modify app dom:') - console.error(err) - /* eslint-disable no-console */ - } - } - startStudio = () => { if (studioRecorder.isLoading) { studioRecorder.start(this._body()[0]) diff --git a/packages/runner/src/iframe/iframes.jsx b/packages/runner/src/iframe/iframes.jsx index 7bae51d0e4e8..4bb5be38a1bb 100644 --- a/packages/runner/src/iframe/iframes.jsx +++ b/packages/runner/src/iframe/iframes.jsx @@ -74,8 +74,6 @@ export default class Iframes extends Component { this.autIframe = new AutIframe(this.props.config) this.props.eventManager.on('visit:failed', this.autIframe.showVisitFailure) - this.props.eventManager.on('before:screenshot', this.autIframe.beforeScreenshot) - this.props.eventManager.on('after:screenshot', this.autIframe.afterScreenshot) this.props.eventManager.on('script:error', this._setScriptError) this.props.eventManager.on('visit:blank', this.autIframe.visitBlank) From 02d5c9f2726bc002133e82fcc01a78dde41a48fc Mon Sep 17 00:00:00 2001 From: Matt Schile Date: Fri, 11 Feb 2022 12:25:56 -0700 Subject: [PATCH 2/4] removing autIframe screenshot from runner-ct --- packages/runner-ct/src/iframe/iframes.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/runner-ct/src/iframe/iframes.tsx b/packages/runner-ct/src/iframe/iframes.tsx index 285ea783bdc3..2c7771e1c9e9 100644 --- a/packages/runner-ct/src/iframe/iframes.tsx +++ b/packages/runner-ct/src/iframe/iframes.tsx @@ -102,8 +102,6 @@ export const Iframes = namedObserver('Iframes', ({ useEffect(() => { eventManager.on('visit:failed', autIframe.current.showVisitFailure) - eventManager.on('before:screenshot', autIframe.current.beforeScreenshot) - eventManager.on('after:screenshot', autIframe.current.afterScreenshot) eventManager.on('script:error', _setScriptError) // TODO: need to take headless mode into account From 5062727f7fa946722214064aa297c96836da12fd Mon Sep 17 00:00:00 2001 From: Matt Schile Date: Fri, 11 Feb 2022 13:24:56 -0700 Subject: [PATCH 3/4] fixing lint issue --- packages/driver/src/dom/blackout.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/driver/src/dom/blackout.ts b/packages/driver/src/dom/blackout.ts index 8c3085ff50bc..bf8e991b475a 100644 --- a/packages/driver/src/dom/blackout.ts +++ b/packages/driver/src/dom/blackout.ts @@ -43,7 +43,7 @@ function addBlackouts ($body, selector) { return } - $el.each(function () { + $el.each(function (this: HTMLElement) { addBlackoutForElement($body, $(this)) }) } From c92000be166a32493b365b29f5c8e016dcc0928c Mon Sep 17 00:00:00 2001 From: Matt Schile Date: Mon, 14 Feb 2022 09:01:25 -0700 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Chris Breiding --- packages/driver/src/cy/commands/screenshot.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/driver/src/cy/commands/screenshot.ts b/packages/driver/src/cy/commands/screenshot.ts index 4d6dcc334d79..ac83a2b681e8 100644 --- a/packages/driver/src/cy/commands/screenshot.ts +++ b/packages/driver/src/cy/commands/screenshot.ts @@ -352,7 +352,7 @@ const takeScreenshot = (Cypress, state, screenshotConfig, options: TakeScreensho /* eslint-disable no-console */ console.error('Failed to modify app dom:') console.error(err) - /* eslint-disable no-console */ + /* eslint-enable no-console */ } return sendAsync('before:screenshot', getOptions(true)) @@ -371,7 +371,7 @@ const takeScreenshot = (Cypress, state, screenshotConfig, options: TakeScreensho /* eslint-disable no-console */ console.error('Failed to modify app dom:') console.error(err) - /* eslint-disable no-console */ + /* eslint-enable no-console */ } send('after:screenshot', getOptions(false))