fix: #29605 for display: contents parent visibility with correct widt…

…h check (#29680) * forst working version * remove visibilityVisible as non essencial * changelog for pipeline fix * fix only for firefox. It works in other browsers without this change * changelog update for pipeline * pipeline fix * return of previous spacing * Update CHANGELOG.md * changelog without unnecessery changes * changelog fix * fix changelog entry * update test to properly have a test for the issue * add test for other elements to not be visible * bump cache --------- Co-authored-by: Jennifer Shehane <[email protected]>
cypress-io · Nov 6, 2024 · a6ddd3d · a6ddd3d · cypress-bot · Nov 6, 2024
1 parent 11f3355
commit a6ddd3d
Show file tree

Hide file tree

Showing 6 changed files with 106 additions and 46 deletions.
diff --git a/.circleci/cache-version.txt b/.circleci/cache-version.txt
@@ -1,3 +1,3 @@
 # Bump this version to force CI to re-create the cache from scratch.
 
-10-30-24-vue2-removal
+11-1-24
diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md
@@ -28,6 +28,7 @@ in this [GitHub issue](https://github.com/cypress-io/cypress/issues/30447). Addr
 
 **Bugfixes:**
 
+- Elements with `display: contents` will no longer use box model calculations for visibility, and correctly show as visible when it is visible. Fixed in [#29680](https://github.com/cypress-io/cypress/pull/29680). Fixes [#29605](https://github.com/cypress-io/cypress/issues/29605).
 - The CSS pseudo-class `:dir()` is now supported when testing in Electron. Addresses [#29766](https://github.com/cypress-io/cypress/issues/29766).
 
 **Dependency Updates:**

diff --git a/packages/driver/cypress/e2e/dom/visibility.cy.ts b/packages/driver/cypress/e2e/dom/visibility.cy.ts
@@ -382,8 +382,8 @@ describe('src/cypress/dom/visibility', () => {
 `)
 
       this.$elOutOfParentBoundsAbove = add(`\
-<div style='width: 100px; height: 100px; overflow: hidden; position: relative;'>
-  <span style='position: absolute; width: 100px; height: 100px; left: 0px; top: -100px;'>position: absolute, out of bounds above</span>
+<div style='width: 100px; height: 100px; overflow: hidden; position: fixed;'>
+  <span id='elOutOfParentBoundsAbove' style='position: absolute; width: 100px; height: 100px; left: 0px; top: -100px;'>position: absolute, out of bounds above</span>
 </div>\
 `)
 
@@ -863,7 +863,7 @@ describe('src/cypress/dom/visibility', () => {
       })
 
       it('is hidden when parent overflow hidden and out of bounds above', function () {
-        expect(this.$elOutOfParentBoundsAbove.find('span')).to.be.hidden
+        expect(this.$elOutOfParentBoundsAbove.find('span#elOutOfParentBoundsAbove')).to.be.hidden
       })
 
       it('is hidden when parent overflow hidden and out of bounds below', function () {
@@ -1200,10 +1200,7 @@ describe('src/cypress/dom/visibility', () => {
       })
 
       it('element is fixed and being covered', function () {
-        this.reasonIs(this.$coveredUpPosFixed.find('#coveredUpPosFixed'), `\
-This element \`<div#coveredUpPosFixed>\` is not visible because it has CSS property: \`position: fixed\` and it's being covered by another element:
-
-\`<div style="position: fixed; bottom: 0; left: 0">on top</div>\``)
+        this.reasonIs(this.$coveredUpPosFixed.find('#coveredUpPosFixed'), `\This element \`<div#coveredUpPosFixed>\` is not visible because it has CSS property: \`position: fixed\` and it's being covered by another element:\n\n\`<div style="position: fixed; bottom: 0; left: 0">on top</div>\``)
       })
 
       it('needs scroll', function () {

diff --git a/packages/driver/cypress/e2e/issues/29605.cy.js b/packages/driver/cypress/e2e/issues/29605.cy.js
@@ -0,0 +1,17 @@
+// https://github.com/cypress-io/cypress/issues/29605
+describe('issue #29605 - els with display: contents', () => {
+  beforeEach(() => {
+    cy.visit('/fixtures/issue-29605.html')
+  })
+
+  it('children of parent with no width/height are visible', () => {
+    cy.get('#parent').should('not.be.visible')
+    cy.get('#child').should('be.visible')
+  })
+
+  // https://drafts.csswg.org/css-display/#unbox
+  it('not rendered by CSS box concept are not visible', () => {
+    cy.get('#input').should('not.be.visible')
+    cy.get('#select').should('not.be.visible')
+  })
+})
diff --git a/packages/driver/cypress/fixtures/issue-29605.html b/packages/driver/cypress/fixtures/issue-29605.html
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html lang="en">
+
+<head>
+  <meta charset="utf-8" />
+  <title>29605 repro</title>
+</head>
+
+<body>
+  <div id="parent" style="overflow: hidden; height: 0; width: 0; display: contents;">
+    <div id="child" style="height:200px; width: 200px; background: #f00"></div>
+  </div>
+  <input id="input" type="text" id="name" name="name" style="overflow: hidden; height: 0; width: 0; display: contents;" />
+  <select name="pets" id="select" style="display: contents;">
+    <option value="">--Please choose an option--</option>
+    <option value="dog">Dog</option>
+    <option value="cat">Cat</option>
+  </select>
+</body>
+</html>
diff --git a/packages/driver/src/dom/visibility.ts b/packages/driver/src/dom/visibility.ts
@@ -65,7 +65,7 @@ const isStrictlyHidden = (el, methodName = 'isStrictlyHidden()', options = { che
   }
 
   // in Cypress-land we consider the element hidden if
-  // either its offsetHeight or offsetWidth is 0 because
+  // either its clientHeight or clientWidth is 0 because
   // it is impossible for the user to interact with this element
   if (elHasNoEffectiveWidthOrHeight($el)) {
     // https://github.com/cypress-io/cypress/issues/6183
@@ -121,32 +121,38 @@ const elHasNoEffectiveWidthOrHeight = ($el) => {
   // Is the element's CSS width OR height, including any borders,
   // padding, and vertical scrollbars (if rendered) less than 0?
   //
-  // elOffsetWidth:
+  // elClientWidth:
   // If the element is hidden (for example, by setting style.display
   // on the element or one of its ancestors to "none"), then 0 is returned.
 
   // $el[0].getClientRects().length:
   // For HTML <area> elements, SVG elements that do not render anything themselves,
   // display:none elements, and generally any elements that are not directly rendered,
   // an empty list is returned.
-
   const el = $el[0]
-  const style = getComputedStyle(el)
-  const transform = style.getPropertyValue('transform')
-  const width = elOffsetWidth($el)
-  const height = elOffsetHeight($el)
-  const overflowHidden = elHasOverflowHidden($el)
+  let transform
+
+  if ($el[0].style.transform) {
+    const style = getComputedStyle(el)
+
+    transform = style.getPropertyValue('transform')
+  } else {
+    transform = 'none'
+  }
+
+  const width = elClientWidth($el)
+  const height = elClientHeight($el)
 
   return isZeroLengthAndTransformNone(width, height, transform) ||
-  isZeroLengthAndOverflowHidden(width, height, overflowHidden) ||
+  isZeroLengthAndOverflowHidden(width, height, elHasOverflowHidden($el)) ||
   (el.getClientRects().length <= 0)
 }
 
 const isZeroLengthAndTransformNone = (width, height, transform) => {
   // From https://github.com/cypress-io/cypress/issues/5974,
   // we learned that when an element has non-'none' transform style value like "translate(0, 0)",
   // it is visible even with `height: 0` or `width: 0`.
-  // That's why we're checking `transform === 'none'` together with elOffsetWidth/Height.
+  // That's why we're checking `transform === 'none'` together with elClientWidth/Height.
 
   return (width <= 0 && transform === 'none') ||
   (height <= 0 && transform === 'none')
@@ -157,17 +163,15 @@ const isZeroLengthAndOverflowHidden = (width, height, overflowHidden) => {
   (height <= 0 && overflowHidden)
 }
 
-const elHasNoOffsetWidthOrHeight = ($el) => {
-  return (elOffsetWidth($el) <= 0) || (elOffsetHeight($el) <= 0)
+const elHasNoClientWidthOrHeight = ($el) => {
+  return (elClientWidth($el) <= 0) || (elClientHeight($el) <= 0)
 }
 
-const elOffsetWidth = ($el) => {
-  return $el[0].offsetWidth
-}
+const elementBoundingRect = ($el) => $el[0].getBoundingClientRect()
 
-const elOffsetHeight = ($el) => {
-  return $el[0].offsetHeight
-}
+const elClientHeight = ($el) => elementBoundingRect($el).height
+
+const elClientWidth = ($el) => elementBoundingRect($el).width
 
 const elHasVisibilityHiddenOrCollapse = ($el) => {
   return elHasVisibilityHidden($el) || elHasVisibilityCollapse($el)
@@ -185,6 +189,10 @@ const elHasOpacityZero = ($el) => {
   return $el.css('opacity') === '0'
 }
 
+const elHasDisplayContents = ($el) => {
+  return $el.css('display') === 'contents'
+}
+
 const elHasDisplayNone = ($el) => {
   return $el.css('display') === 'none'
 }
@@ -219,6 +227,11 @@ const canClipContent = function ($el, $ancestor) {
     return false
   }
 
+  // fix for 29605 - display: contents
+  if (elHasDisplayContents($ancestor)) {
+    return false
+  }
+
   // the closest parent with position relative, absolute, or fixed
   const $offsetParent = $el.offsetParent()
 
@@ -308,35 +321,41 @@ const elIsNotElementFromPoint = function ($el) {
   return true
 }
 
-const elIsOutOfBoundsOfAncestorsOverflow = function ($el, $ancestor = getParent($el)) {
+const elIsOutOfBoundsOfAncestorsOverflow = function ($el: JQuery<any>, $ancestor = getParent($el)) {
   // no ancestor, not out of bounds!
   // if we've reached the top parent, which is not a normal DOM el
   // then we're in bounds all the way up, return false
   if (isUndefinedOrHTMLBodyDoc($ancestor)) {
     return false
   }
 
+  // fix for 29605 - display: contents
+  if (elHasDisplayContents($el)) {
+    return false
+  }
+
   if (canClipContent($el, $ancestor)) {
-    const elProps = $coordinates.getElementPositioning($el)
-    const ancestorProps = $coordinates.getElementPositioning($ancestor)
+    const ancestorProps = $ancestor.get(0).getBoundingClientRect()
 
     if (elHasPositionAbsolute($el) && (ancestorProps.width === 0 || ancestorProps.height === 0)) {
       return elIsOutOfBoundsOfAncestorsOverflow($el, getParent($ancestor))
     }
 
+    const elProps = $el.get(0).getBoundingClientRect()
+
     // target el is out of bounds
     if (
       // target el is to the right of the ancestor's visible area
-      (elProps.fromElWindow.left >= (ancestorProps.width + ancestorProps.fromElWindow.left)) ||
+      (elProps.left >= (ancestorProps.width + ancestorProps.left)) ||
 
       // target el is to the left of the ancestor's visible area
-      ((elProps.fromElWindow.left + elProps.width) <= ancestorProps.fromElWindow.left) ||
+      ((elProps.left + elProps.width) <= ancestorProps.left) ||
 
       // target el is under the ancestor's visible area
-      (elProps.fromElWindow.top >= (ancestorProps.height + ancestorProps.fromElWindow.top)) ||
+      (elProps.top >= (ancestorProps.height + ancestorProps.top)) ||
 
       // target el is above the ancestor's visible area
-      ((elProps.fromElWindow.top + elProps.height) <= ancestorProps.fromElWindow.top)
+      ((elProps.top + elProps.height) <= ancestorProps.top)
     ) {
       return true
     }
@@ -348,7 +367,7 @@ const elIsOutOfBoundsOfAncestorsOverflow = function ($el, $ancestor = getParent(
 const elIsHiddenByAncestors = function ($el, checkOpacity, $origEl = $el) {
   // walk up to each parent until we reach the body
   // if any parent has opacity: 0
-  // or has an effective offsetHeight of 0
+  // or has an effective clientHeight of 0
   // and its set overflow: hidden then our child element
   // is effectively hidden
   // -----UNLESS------
@@ -363,14 +382,20 @@ const elIsHiddenByAncestors = function ($el, checkOpacity, $origEl = $el) {
     return false
   }
 
+  if (elHasDisplayContents($el)) {
+    let $parent = getParent($el)
+
+    return elIsHiddenByAncestors($parent, checkOpacity, $parent)
+  }
+
   // a child can never have a computed opacity
   // greater than that of its parent
   // so if the parent has an opacity of 0, so does the child
   if (elHasOpacityZero($parent) && checkOpacity) {
     return true
   }
 
-  if (elHasOverflowHidden($parent) && elHasNoEffectiveWidthOrHeight($parent)) {
+  if (elHasOverflowHidden($parent) && !elHasDisplayContents($parent) && elHasNoEffectiveWidthOrHeight($parent)) {
     // if any of the elements between the parent and origEl
     // have fixed or position absolute
     return !elDescendentsHavePositionFixedOrAbsolute($parent, $origEl)
@@ -380,7 +405,7 @@ const elIsHiddenByAncestors = function ($el, checkOpacity, $origEl = $el) {
   return elIsHiddenByAncestors($parent, checkOpacity, $origEl)
 }
 
-const parentHasNoOffsetWidthOrHeightAndOverflowHidden = function ($el) {
+const parentHasNoClientWidthOrHeightAndOverflowHidden = function ($el: JQuery<HTMLElement>) {
   // if we've walked all the way up to body or html then return false
   if (isUndefinedOrHTMLBodyDoc($el)) {
     return false
@@ -392,7 +417,7 @@ const parentHasNoOffsetWidthOrHeightAndOverflowHidden = function ($el) {
   }
 
   // continue walking
-  return parentHasNoOffsetWidthOrHeightAndOverflowHidden(getParent($el))
+  return parentHasNoClientWidthOrHeightAndOverflowHidden(getParent($el))
 }
 
 const parentHasDisplayNone = function ($el) {
@@ -463,8 +488,8 @@ export const getReasonIsHidden = function ($el, options = { checkOpacity: true }
   // either being covered or there is no el
 
   const node = stringifyElement($el, 'short')
-  let width = elOffsetWidth($el)
-  let height = elOffsetHeight($el)
+  let width = elClientWidth($el)
+  let height = elClientHeight($el)
   let $parent
   let parentNode
 
@@ -513,24 +538,24 @@ export const getReasonIsHidden = function ($el, options = { checkOpacity: true }
     return `This element \`${node}\` is not visible because its parent \`${parentNode}\` has CSS property: \`opacity: 0\``
   }
 
-  if (elHasNoOffsetWidthOrHeight($el)) {
-    return `This element \`${node}\` is not visible because it has an effective width and height of: \`${width} x ${height}\` pixels.`
-  }
-
   const transformResult = $transform.detectVisibility($el)
 
   if (transformResult === 'transformed') {
     return `This element \`${node}\` is not visible because it is hidden by transform.`
   }
 
+  if (elHasNoClientWidthOrHeight($el)) {
+    return `This element \`${node}\` is not visible because it has an effective width and height of: \`${width} x ${height}\` pixels.`
+  }
+
   if (transformResult === 'backface') {
     return `This element \`${node}\` is not visible because it is rotated and its backface is hidden.`
   }
 
-  if ($parent = parentHasNoOffsetWidthOrHeightAndOverflowHidden(getParent($el))) {
+  if ($parent = parentHasNoClientWidthOrHeightAndOverflowHidden(getParent($el))) {
     parentNode = stringifyElement($parent, 'short')
-    width = elOffsetWidth($parent)
-    height = elOffsetHeight($parent)
+    let width = elClientWidth($parent)
+    let height = elClientHeight($parent)
 
     return `This element \`${node}\` is not visible because its parent \`${parentNode}\` has CSS property: \`overflow: hidden\` and an effective width and height of: \`${width} x ${height}\` pixels.`
   }