Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update visibility for elements with textContent but without width/height #29688

Merged
merged 48 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
3e25863
Parent with correctly visible
senpl Jun 17, 2024
6036095
cahngelog entry
senpl Jun 17, 2024
4a8a730
changelog update
senpl Jun 17, 2024
a2f67ad
Merge branch 'develop' into issue-29687
senpl Jun 17, 2024
9e0afe3
test cleanup
senpl Jun 17, 2024
f41e92f
Merge branch 'issue-29687' of https://github.com/senpl/cypress into i…
senpl Jun 17, 2024
0759e35
first working version
senpl Jul 1, 2024
35dd425
fix with cleaner code
senpl Jul 1, 2024
ea3dca2
Merge branch 'develop' into issue-29687
senpl Jul 1, 2024
57ec09d
changelog update for pipeline
senpl Jul 1, 2024
e33ee3c
error message fixes
senpl Jul 2, 2024
3657f21
Merge branch 'develop' into issue-29687
senpl Jul 3, 2024
20cabe3
changelog update
senpl Jul 3, 2024
86b21a7
Merge branch 'issue-29687' of https://github.com/senpl/cypress into i…
senpl Jul 3, 2024
a94f560
looks like it works in old form
senpl Jul 8, 2024
8f79ee7
Merge branch 'develop' into issue-29687
senpl Jul 8, 2024
62c341d
fix for display none
senpl Jul 8, 2024
83ffde7
Merge branch 'develop' into issue-29687
jennifer-shehane Jul 8, 2024
832f4e3
Merge branch 'develop' into issue-29687
senpl Jul 11, 2024
641cac9
Update cli/CHANGELOG.md
senpl Jul 11, 2024
2bfecbf
Merge branch 'develop' into issue-29687
senpl Jul 12, 2024
d2ec46b
Merge branch 'develop' into issue-29687
senpl Jul 13, 2024
dbd4533
Merge branch 'develop' into issue-29687
senpl Jul 25, 2024
9b2de0c
Update CHANGELOG.md
senpl Jul 25, 2024
6a44f5a
Merge branch 'develop' into issue-29687
senpl Jul 26, 2024
3a0309f
Merge branch 'develop' into issue-29687
senpl Aug 7, 2024
c127b36
Merge branch 'develop' into issue-29687
senpl Aug 8, 2024
599e3d7
Merge branch 'develop' into issue-29687
senpl Aug 12, 2024
411f60e
Merge branch 'develop' into issue-29687
senpl Aug 16, 2024
3de8716
Merge branch 'develop' into issue-29687
senpl Sep 30, 2024
012d49a
Merge branch 'release/14.0.0' into issue-29687
senpl Oct 1, 2024
cd94f78
Merge branch 'release/14.0.0' into issue-29687
senpl Oct 28, 2024
a561c06
Merge branch 'release/14.0.0' into issue-29687
jennifer-shehane Oct 29, 2024
80ad94c
Merge branch 'release/14.0.0' into issue-29687
senpl Oct 30, 2024
5e53747
changelog fixes
senpl Oct 30, 2024
ceb7148
Merge branch 'release/14.0.0' into issue-29687
jennifer-shehane Nov 6, 2024
c41f1c1
changelog update
jennifer-shehane Nov 6, 2024
9847e19
Rewrite some code / ensure tests cover new rules
jennifer-shehane Nov 6, 2024
1b6d74b
remove whitespace from textContent since that cannot be seen.
jennifer-shehane Nov 7, 2024
b229d5f
Merge branch 'release/14.0.0' into issue-29687
jennifer-shehane Nov 7, 2024
be5587c
add existential check to replace
jennifer-shehane Nov 8, 2024
17b5780
Merge branch 'release/14.0.0' into issue-29687
jennifer-shehane Nov 8, 2024
44d50a6
Merge branch 'release/14.0.0' into issue-29687
mschile Nov 19, 2024
8afed31
Merge branch 'release/14.0.0' into issue-29687
mschile Nov 26, 2024
0afd2c6
updates
mschile Nov 26, 2024
6ae90ee
update test
mschile Nov 26, 2024
7fe35e3
update changelog
mschile Nov 26, 2024
2cff37f
Merge branch 'release/14.0.0' into issue-29687
mschile Dec 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ _Released 7/16/2024 (PENDING)_
**Bugfixes:**

- Fixed an issue where the ReadStream used to upload a Test Replay recording could erroneously be re-used when retrying in cases of retryable upload failures. Fixes [#29227](https://github.com/cypress-io/cypress/issues/29227)
- Fixed an issue where parent width 0 signal element hidden when it's visible. Fixes [#29687](https://github.com/cypress-io/cypress/issues/29687).

## 13.13.0

Expand Down Expand Up @@ -46,6 +47,7 @@ _Released 6/18/2024_
- Fixed an issue where `inlineSourceMaps` was still being used when `sourceMaps` was provided in a users typescript config for typescript version 5. Fixes [#26203](https://github.com/cypress-io/cypress/issues/26203).
- When capture protocol script fails verification, an appropriate error is now displayed. Previously, an error regarding Test Replay archive location was shown. Addressed in [#29603](https://github.com/cypress-io/cypress/pull/29603).
- Fixed an issue where receiving HTTP responses with invalid headers raised an error. Now cypress removes the invalid headers and gives a warning in the console with debug mode on. Fixes [#28865](https://github.com/cypress-io/cypress/issues/28865).
- Fixed an issue where parent width signal element hidden when it's visible [#29687](https://github.com/cypress-io/cypress/issues/29687).
senpl marked this conversation as resolved.
Show resolved Hide resolved

**Misc:**

Expand Down
6 changes: 3 additions & 3 deletions packages/driver/cypress/e2e/commands/actions/click.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -2205,7 +2205,7 @@ describe('src/cy/commands/actions/click', () => {

expect(logsArr).to.have.length(4)
expect(lastLog.get('error')).to.eq(err)
expect(err.message).to.include('`cy.click()` failed because this element is not visible')
expect(err.message).to.include('`cy.click()` failed because this element')

done()
})
Expand Down Expand Up @@ -3344,7 +3344,7 @@ describe('src/cy/commands/actions/click', () => {

expect(logs).to.have.length(4)
expect(lastLog.get('error')).to.eq(err)
expect(err.message).to.include('`cy.dblclick()` failed because this element is not visible')
expect(err.message).to.include('`cy.dblclick()` failed because this element')

done()
})
Expand Down Expand Up @@ -3780,7 +3780,7 @@ describe('src/cy/commands/actions/click', () => {

assertLogLength(this.logs, 4)
expect(lastLog.get('error')).to.eq(err)
expect(err.message).to.include('`cy.rightclick()` failed because this element is not visible')
expect(err.message).to.include('`cy.rightclick()` failed because this element')

done()
})
Expand Down
2 changes: 1 addition & 1 deletion packages/driver/cypress/e2e/commands/actions/select.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ describe('src/cy/commands/actions/select', () => {
cy.$$('#select-maps').show().hide()

cy.on('fail', (err) => {
expect(err.message).to.include('`cy.select()` failed because this element is not visible')
expect(err.message).to.include('`cy.select()` failed because this element')

done()
})
Expand Down
18 changes: 9 additions & 9 deletions packages/driver/cypress/e2e/dom/visibility.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ describe('src/cypress/dom/visibility', () => {
this.$parentVisHidden = add('<div class="invis" style="visibility: hidden;"><button>parent visibility: hidden</button></div>')
this.$displayNone = add('<button style="display: none">display: none</button>')
this.$inputHidden = add('<input type="hidden" value="abcdef">')
this.$divNoWidth = add('<div style="width: 0; height: 100px;">width: 0</div>')
this.$divNoWidth = add('<div id="divNoWidth" style="width: 0; height: 100px;">width: 0</div>')
this.$divNoHeight = add('<div style="width: 50px; height: 0px;">height: 0</div>')
this.$divDetached = $('<div>foo</div>')
this.$divVisible = add(`<div>visible</div>`)
Expand Down Expand Up @@ -721,15 +721,15 @@ describe('src/cypress/dom/visibility', () => {
})

describe('width and height', () => {
it('is hidden if offsetWidth is 0', function () {
expect(this.$divNoWidth.is(':hidden')).to.be.true
expect(this.$divNoWidth.is(':visible')).to.be.false
it('is visible even if offsetWidth is 0', function () {
expect(this.$divNoWidth.is(':hidden')).to.be.false
expect(this.$divNoWidth.is(':visible')).to.be.true

expect(this.$divNoWidth).to.be.hidden
expect(this.$divNoWidth).to.not.be.visible
expect(this.$divNoWidth).to.not.be.hidden
expect(this.$divNoWidth).to.be.visible

cy.wrap(this.$divNoWidth).should('be.hidden')
cy.wrap(this.$divNoWidth).should('not.be.visible')
cy.wrap(this.$divNoWidth).should('be.not.hidden')
cy.wrap(this.$divNoWidth).should('be.visible')
})

it('is hidden if parent has overflow: hidden and no width', function () {
Expand Down Expand Up @@ -1142,7 +1142,7 @@ describe('src/cypress/dom/visibility', () => {
})

it('has effective zero width', function () {
this.reasonIs(this.$divNoWidth, 'This element `<div>` is not visible because it has an effective width and height of: `0 x 100` pixels.')
this.reasonIs(this.$divNoWidth, 'This element `<div#divNoWidth>` is not visible because it has an effective width and height of: `0 x 100` pixels.')
})

it('has effective zero height', function () {
Expand Down
12 changes: 12 additions & 0 deletions packages/driver/cypress/e2e/issues/29687.cy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
describe('issue visible element with parent 0 width are detected as not visible', () => {
before(() => {
cy
.viewport('macbook-16')
.visit('/fixtures/issue-29687.html')
})

it('can click element when effective 0 width parent used', () => {
expect(cy.$$('#id2 >div > p')).to.be.visible
cy.get('#id2 >div > p').click()
})
})
21 changes: 21 additions & 0 deletions packages/driver/cypress/fixtures/issue-29687.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<!DOCTYPE html>
<html lang="en">

<head>
<meta charset="utf-8" />
<title>zero width repro</title>
<style id="compiled-css" type="text/css">
.big {
font-size: 128px;
}
</style>
</head>

<body>
<div id="id2">
<div style="width: 0">
<p class="big">IN SECTION 2 </p>
</div>
</div>
</body>
</html>
77 changes: 61 additions & 16 deletions packages/driver/src/dom/visibility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,35 @@ const ensureEl = (el, methodName) => {
}
}

const checkIsOptionVisible = (el) => {
// an option is considered visible if its parent select is visible
if (isOption(el) || isOptgroup(el)) {
const $el = $jquery.wrap(el)

if (elHasDisplayNone($el)) {
return 2
}

// if its parent select is visible, then it's not hidden
const $select = getFirstParentWithTagName($el, 'select')

if ($select && $select.length) {
// if the select is hidden, the options in it are not visible too
if (isStrictlyHidden($select)) {
return 2 //this signal not visible
}
} else {
if (isStrictlyHidden($el)) {
return 2
}
}

return true //this signal visible
}

return 0 //this signal not option element
}

const isStrictlyHidden = (el, methodName = 'isStrictlyHidden()', options = { checkOpacity: true }, recurse?) => {
ensureEl(el, methodName)
const $el = $jquery.wrap(el)
Expand All @@ -45,23 +74,18 @@ const isStrictlyHidden = (el, methodName = 'isStrictlyHidden()', options = { che
return false // is visible
}

// an option is considered visible if its parent select is visible
if (isOption(el) || isOptgroup(el)) {
// they could have just set to hide the option
if (elHasDisplayNone($el)) {
return true
}
const optionIsVisible = checkIsOptionVisible(el)

// if its parent select is visible, then it's not hidden
const $select = getFirstParentWithTagName($el, 'select')
if (optionIsVisible === true) {
return false
}

// check $select.length here first
// they may have not put the option into a select el,
// in which case it will fall through to regular visibility logic
if ($select && $select.length) {
// if the select is hidden, the options in it are visible too
return recurse ? recurse($select[0], methodName, options) : isStrictlyHidden($select[0], methodName, options)
}
if (optionIsVisible > 1) {
return true
}

if (elHasDisplayNone($el)) {
return true
}

// in Cypress-land we consider the element hidden if
Expand All @@ -73,7 +97,28 @@ const isStrictlyHidden = (el, methodName = 'isStrictlyHidden()', options = { che
return !elHasVisibleChild($el)
}

return true // is hidden
if (el.textContent) {
//this below should be in function
if (elHasVisibilityHiddenOrCollapse($el)) {
return true // is hidden
}

// when an element is scaled to 0 in one axis
// it is not visible to users.
// So, it is hidden.
if ($transform.detectVisibility($el) !== 'visible') {
return true
}

// a transparent element is hidden
if (elHasOpacityZero($el) && options.checkOpacity) {
return true
}

return false
}

return true
}

// additionally if the effective visibility of the element
Expand Down