3.5.0 - fix click events sent to proper element, respect select… (#5496)

* fix click during overlay on mousedown, fix selectionchange during focus handler

* fix stop-only script

* fix 5439 replace selection during modifier

* cleanup and refactor

* fix test

* fix actionability not erroring on covered element

* Revert "fix actionability not erroring on covered element"

This reverts commit 208af647de.
This commit is contained in:
Ben Kucera
2019-10-29 17:59:37 -04:00
committed by Jennifer Shehane
parent fc2dd21f8e
commit cffe8b8fb2
7 changed files with 254 additions and 72 deletions
+1 -1
View File
@@ -47,7 +47,7 @@
"set-next-ci-version": "node ./scripts/binary.js setNextVersion",
"prestart": "npm run check-deps-pre",
"start": "node ./cli/bin/cypress open --dev --global",
"stop-only": "npx stop-only --skip .cy,.publish,.projects,node_modules,dist,dist-test,fixtures,lib,bower_components --exclude e2e.coffee",
"stop-only": "npx stop-only --skip .cy,.publish,.projects,node_modules,dist,dist-test,fixtures,lib,bower_components,src --exclude e2e.coffee",
"stop-only-all": "npm run stop-only -- --folder packages",
"test": "echo '⚠️ This root monorepo is only for local development and new contributions. There are no tests.'",
"test-debug-package": "node ./scripts/test-debug-package.js",
+1 -1
View File
@@ -237,7 +237,7 @@ const shouldIgnoreEvent = <
}
const shouldUpdateValue = (el: HTMLElement, key: KeyDetails) => {
if (!key.text) return true
if (!key.text) return false
const bounds = $selection.getSelectionBounds(el)
const noneSelected = bounds.start === bounds.end
+58 -19
View File
@@ -54,6 +54,26 @@ const shouldFireMouseMoveEvents = (targetEl, lastHoveredEl, fromElViewport, coor
return !_.isEqual(xy(fromElViewport), xy(coords))
}
const shouldMoveCursorToEndAfterMousedown = (el) => {
if (!$elements.isElement(el)) {
return false
}
if (!($elements.isInput(el) || $elements.isTextarea(el) || $elements.isContentEditable(el))) {
return false
}
if (!$elements.isFocused(el)) {
return false
}
if ($elements.isNeedSingleValueChangeInputElement(el)) {
return false
}
return true
}
const create = (state, keyboard, focused) => {
const mouse = {
_getDefaultMouseOptions (x, y, win) {
@@ -352,11 +372,9 @@ const create = (state, keyboard, focused) => {
}
}
if ($elements.isInput(el) || $elements.isTextarea(el) || $elements.isContentEditable(el)) {
if (!$elements.isNeedSingleValueChangeInputElement(el)) {
debug('moveSelectionToEnd due to click')
$selection.moveSelectionToEnd($dom.getDocumentFromElement(el))
}
if (shouldMoveCursorToEndAfterMousedown($elToFocus[0])) {
debug('moveSelectionToEnd due to click')
$selection.moveSelectionToEnd($dom.getDocumentFromElement($elToFocus[0]), { onlyIfEmptySelection: true })
}
return mouseDownEvents
@@ -392,8 +410,8 @@ const create = (state, keyboard, focused) => {
* el2 = moveToCoordsOrNoop(coords)
* sendMouseup(el2)
* el3 = moveToCoordsOrNoop(coords)
* if (notDetached(el1))
* sendClick(el3)
* if (notDetached(el1) && el1 === el2)
* sendClick(el3)
*/
click (fromElViewport, forceEl, pointerEvtOptionsExtend = {}, mouseEvtOptionsExtend = {}) {
debug('mouse.click', { fromElViewport, forceEl })
@@ -404,9 +422,28 @@ const create = (state, keyboard, focused) => {
const mouseUpEvents = mouse.up(fromElViewport, forceEl, skipMouseupEvent, pointerEvtOptionsExtend, mouseEvtOptionsExtend)
const skipClickEvent = $elements.isDetachedEl(mouseDownEvents.pointerdownProps.el)
// Only send click event if the same element received both pointerdown and pointerup, and it's not detached.
const getSkipClickEventAndReason = () => {
// Never skip the click event when force:true
if (forceEl) {
return false
}
const mouseClickEvents = mouse._mouseClickEvents(fromElViewport, forceEl, skipClickEvent, mouseEvtOptionsExtend)
if ($elements.isDetachedEl(mouseDownEvents.pointerdownProps.el)) {
return 'element was detached'
}
if (!mouseUpEvents.pointerupProps.el || mouseDownEvents.pointerdownProps.el !== mouseUpEvents.pointerupProps.el) {
return 'mouseup and mousedown not received by same element'
}
// No reason to skip the click event
return false
}
const skipClickEvent = getSkipClickEventAndReason()
const mouseClickEvents = mouse._mouseClickEvents(fromElViewport, mouseDownEvents.pointerdownProps.el, forceEl, skipClickEvent, mouseEvtOptionsExtend)
return _.extend({}, mouseDownEvents, mouseUpEvents, mouseClickEvents)
},
@@ -453,8 +490,18 @@ const create = (state, keyboard, focused) => {
}
},
_mouseClickEvents (fromElViewport, forceEl, skipClickEvent, mouseEvtOptionsExtend = {}) {
const el = forceEl || mouse.moveToCoords(fromElViewport)
_mouseClickEvents (fromElViewport, el, forceEl, skipClickEvent, mouseEvtOptionsExtend = {}) {
if (skipClickEvent) {
return {
clickProps: {
skipped: formatReasonNotFired(skipClickEvent),
},
}
}
if (!forceEl) {
mouse.moveToCoords(fromElViewport)
}
const win = $dom.getWindowByElement(el)
@@ -465,14 +512,6 @@ const create = (state, keyboard, focused) => {
detail: 1,
}, mouseEvtOptionsExtend)
if (skipClickEvent) {
return {
clickProps: {
skipped: formatReasonNotFired('Element was detached'),
},
}
}
let clickProps = sendClick(el, clickEventOptions)
return { clickProps }
+17 -29
View File
@@ -1,3 +1,4 @@
import _ from 'lodash'
import * as $dom from '../dom'
import * as $document from './document'
import * as $elements from './elements'
@@ -402,18 +403,20 @@ const getSelectionBounds = function (el) {
}
}
const moveSelectionToEnd = (doc) => {
return _moveSelectionTo(false, doc)
}
const _moveSelectionTo = function (toStart: boolean, doc: Document, options = {}) {
const opts = _.defaults({}, options, {
onlyIfEmptySelection: false,
})
const moveSelectionToStart = (doc) => {
return _moveSelectionTo(true, doc)
}
const _moveSelectionTo = function (toStart, doc) {
const el = _getActive(doc)
if ($elements.canSetSelectionRangeElement(el)) {
if (opts.onlyIfEmptySelection) {
const { start, end } = getSelectionBounds(el)
if (start !== end) return
}
let cursorPosition
if (toStart) {
@@ -430,6 +433,10 @@ const _moveSelectionTo = function (toStart, doc) {
$elements.callNativeMethod(doc, 'execCommand', 'selectAll', false, null)
const selection = doc.getSelection()
if (!selection) {
return
}
// collapsing the range doesn't work on input/textareas, since the range contains more than the input element
// However, IE can always* set selection range, so only modern browsers (with the selection API) will need this
const direction = toStart ? 'backward' : 'forward'
@@ -437,29 +444,10 @@ const _moveSelectionTo = function (toStart, doc) {
selection.modify('move', direction, 'line')
}
// if $elements.isInput(el) || $elements.isTextarea(el)
// length = $elements.getNativeProp(el, "value").length
// setSelectionRange(el, length, length)
const moveSelectionToEnd = _.curry(_moveSelectionTo)(false)
// else if $elements.isContentEditable(el)
// ## NOTE: can't use execCommand API here because we would have
// ## to selectAll and then collapse so we use the Selection API
// # doc = $document.getDocumentFromElement(el)
// # range = $elements.callNativeMethod(doc, "createRange")
// # hostContenteditable = _getHostContenteditable(el)
// # lastTextNode = _getInnerLastChild(hostContenteditable)
const moveSelectionToStart = _.curry(_moveSelectionTo)(true)
// # if lastTextNode.tagName is "BR"
// # lastTextNode = lastTextNode.parentNode
// # range.setStart(lastTextNode, lastTextNode.length)
// # range.setEnd(lastTextNode, lastTextNode.length)
// # sel = $elements.callNativeMethod(doc, "getSelection")
// # $elements.callNativeMethod(sel, "removeAllRanges")
// # $elements.callNativeMethod(sel, "addRange", range)
// TODO: think about renaming this
const replaceSelectionContents = function (el: HTMLElement, key: string) {
if ($elements.canSetSelectionRangeElement(el)) {
// if ($elements.isRead)
@@ -268,7 +268,10 @@ describe('src/cy/commands/actions/click', () => {
it('will not send mouseEvents/focus if pointerdown is defaultPrevented', () => {
const $btn = cy.$$('#button')
// let clicked = false
$btn.get(0).addEventListener('pointerdown', (e) => {
// clicked = true
e.preventDefault()
expect(e.defaultPrevented).to.be.true
@@ -277,6 +280,7 @@ describe('src/cy/commands/actions/click', () => {
attachMouseClickListeners({ $btn })
cy.get('#button').click().should('not.have.focus')
// cy.wrap(null).should(() => expect(clicked).ok)
cy.getAll('$btn', 'pointerdown pointerup click').each(shouldBeCalledOnce)
cy.getAll('$btn', 'mousedown mouseup').each(shouldNotBeCalled)
@@ -398,6 +402,76 @@ describe('src/cy/commands/actions/click', () => {
cy.getAll('div', 'pointerover pointerenter pointerdown mousedown pointerup mouseup click').each(shouldBeCalled)
})
// https://github.com/cypress-io/cypress/issues/5459
it('events when element moved on mousedown', () => {
const btn = cy.$$('button:first')
const div = cy.$$('div#tabindex')
attachFocusListeners({ btn, div })
attachMouseClickListeners({ btn, div })
attachMouseHoverListeners({ btn, div })
// let clicked = false
btn.on('mousedown', () => {
// clicked = true
div.css(overlayStyle)
})
cy.contains('button').click()
// cy.wrap(null).should(() => expect(clicked).ok)
cy.getAll('btn', 'mouseover mouseenter mousedown focus').each(shouldBeCalled)
cy.getAll('btn', 'click mouseup').each(shouldNotBeCalled)
cy.getAll('div', 'mouseover mouseenter mouseup').each(shouldBeCalled)
cy.getAll('div', 'click focus').each(shouldNotBeCalled)
})
it('events when element moved on mouseup', () => {
const btn = cy.$$('button:first')
const div = cy.$$('div#tabindex')
attachFocusListeners({ btn, div })
attachMouseClickListeners({ btn, div })
attachMouseHoverListeners({ btn, div })
// let clicked = false
btn.on('mouseup', () => {
// clicked = true
div.css(overlayStyle)
})
cy.contains('button').click()
// cy.wrap(null).should(() => expect(clicked).ok)
cy.getAll('btn', 'mouseover mouseenter mousedown focus click mouseup').each(shouldBeCalled)
cy.getAll('div', 'mouseover mouseenter').each(shouldBeCalled)
cy.getAll('div', 'focus click mouseup mousedown').each(shouldNotBeCalled)
})
it('events when element moved on click', () => {
const btn = cy.$$('button:first')
const div = cy.$$('div#tabindex')
attachFocusListeners({ btn, div })
attachMouseClickListeners({ btn, div })
attachMouseHoverListeners({ btn, div })
// let clicked = false
btn.on('click', () => {
// clicked = true
div.css(overlayStyle)
})
cy.contains('button').click()
// cy.wrap(null).should(() => expect(clicked).ok)
cy.getAll('btn', 'mouseover mouseenter mousedown focus click mouseup').each(shouldBeCalled)
cy.getAll('div', 'focus click mouseup mousedown').each(shouldNotBeCalled)
})
it('does not fire a click when element has been removed on mouseup', () => {
const $btn = cy.$$('button:first')
@@ -2457,7 +2531,7 @@ describe('src/cy/commands/actions/click', () => {
})
})
it('#consoleProps when no click', () => {
it('#consoleProps when no click due to detached', () => {
const $btn = cy.$$('button:first')
$btn.on('mouseup', function () {
@@ -2497,7 +2571,56 @@ describe('src/cy/commands/actions/click', () => {
},
{
'Event Name': 'click',
'Target Element': '⚠️ not fired (Element was detached)',
'Target Element': '⚠️ not fired (element was detached)',
'Prevented Default?': null,
'Stopped Propagation?': null,
'Modifiers': null,
},
])
})
})
it('#consoleProps when no click due to move', () => {
const $btn = cy.$$('button:first')
// add on overlay on mousedown
$btn.on('mousedown', () => {
cy.$$('div#tabindex').css(overlayStyle)
})
cy.contains('button').click().then(function () {
expect(this.lastLog.invoke('consoleProps').table[2]().data).to.containSubset([
{
'Event Name': 'pointerdown',
'Target Element': { id: 'button' },
'Prevented Default?': false,
'Stopped Propagation?': false,
'Modifiers': null,
},
{
'Event Name': 'mousedown',
'Target Element': { id: 'button' },
'Prevented Default?': false,
'Stopped Propagation?': false,
'Modifiers': null,
},
{
'Event Name': 'pointerup',
'Target Element': { id: 'tabindex' },
'Prevented Default?': false,
'Stopped Propagation?': false,
'Modifiers': null,
},
{
'Event Name': 'mouseup',
'Target Element': { id: 'tabindex' },
'Prevented Default?': false,
'Stopped Propagation?': false,
'Modifiers': null,
},
{
'Event Name': 'click',
'Target Element': '⚠️ not fired (mouseup and mousedown not received by same element)',
'Prevented Default?': null,
'Stopped Propagation?': null,
'Modifiers': null,
@@ -3948,8 +4071,9 @@ describe('mouse state', () => {
cy.get('#sq6')
.click()
cy.getAll('sq6', 'mousedown pointerdown click').each(shouldBeCalledOnce)
cy.getAll('sq6', 'mousedown pointerdown').each(shouldBeCalledOnce)
cy.getAll('sq6', 'mouseover').each(shouldBeCalledWithCount(2))
cy.getAll('sq6', 'click').each(shouldNotBeCalled)
})
})
@@ -4128,7 +4252,7 @@ describe('mouse state', () => {
})
it('can target new element after mouseup sequence', () => {
const btn = cy.$$(/*html*/`<button id='btn'></button>`)
const btn = cy.$$(/*html*/`<button id='btn'>#btn</button>`)
.css({
float: 'left',
display: 'block',
@@ -4137,18 +4261,22 @@ describe('mouse state', () => {
})
.appendTo(cy.$$('body'))
const cover = cy.$$(/*html*/`<div id='cover'></div>`).css({
backgroundColor: 'blue',
const cover = cy.$$(/*html*/`<div id='cover'>#cover</div>`).css({
backgroundColor: 'salmon',
position: 'relative',
height: 50,
width: 300,
})
.appendTo(btn.parent())
// let clicked = false
cover.on('mouseup', () => {
cover.hide()
// clicked = true
})
attachFocusListeners({ btn, cover })
attachMouseHoverListeners({ btn, cover })
attachMouseClickListeners({ btn, cover })
@@ -4157,14 +4285,11 @@ describe('mouse state', () => {
})
cy.get('#cover').click()
// cy.wrap(null).should(() => expect(clicked).ok)
cy.getAll('cover', 'click').each((stub) => {
expect(stub).to.not.be.called
})
cy.getAll('cover', 'pointerdown mousedown mouseup').each((stub) => {
expect(stub).to.be.calledOnce
})
cy.getAll('cover', 'mousedown mouseup click mouseout mouseleave').each(shouldBeCalledOnce)
cy.getAll('cover', 'focus').each(shouldNotBeCalled)
cy.getAll('btn', 'mouseover mouseenter').each(shouldBeCalled)
})
it('responds to changes in move handlers', () => {
@@ -4185,22 +4310,22 @@ describe('mouse state', () => {
})
.appendTo(btn.parent())
// let clicked = false
cover.on('mouseover', () => {
cover.hide()
// clicked = true
})
attachFocusListeners({ btn, cover })
attachMouseHoverListeners({ btn, cover })
attachMouseClickListeners({ btn, cover })
cy.get('#cover').click()
// cy.wrap(null).should(() => expect(clicked).ok)
cy.getAll('cover', 'mousedown').each((stub) => {
expect(stub).to.not.be.called
})
cy.getAll('btn', 'pointerdown mousedown mouseup pointerup click').each((stub) => {
expect(stub).to.be.calledOnce
})
cy.getAll('cover', 'mousedown mouseup click focus').each(shouldNotBeCalled)
cy.getAll('btn', 'pointerdown mousedown mouseup pointerup click').each(shouldBeCalledOnce)
})
})
@@ -69,7 +69,12 @@ describe('src/cy/commands/actions/type', () => {
it('appends subsequent type commands', () => {
cy
.get('input:first').type('123').type('456')
.get('input:first').type('123')
.then(($el) => {
$el[0].setSelectionRange(0, 0)
})
.blur()
.type('456')
.should('have.value', '123456')
})
@@ -121,6 +126,7 @@ describe('src/cy/commands/actions/type', () => {
})
})
// cursor should be moved to the end before type, so text is appended
it('can type into contenteditable', () => {
const oldText = cy.$$('#contenteditable').get(0).innerText
@@ -951,6 +957,19 @@ describe('src/cy/commands/actions/type', () => {
})
})
// https://github.com/cypress-io/cypress/issues/5456
it('respects changed selection in focus handler', () => {
cy.get('#input-without-value')
.then(($el) => {
$el.val('foo')
.on('focus', function (e) {
e.currentTarget.setSelectionRange(0, 1)
})
})
.type('bar')
.should('have.value', 'baroo')
})
it('overwrites text when selectAll in mouseup handler', () => {
cy.$$('#input-without-value').val('0').mouseup(function () {
$(this).select()
@@ -2832,6 +2851,17 @@ describe('src/cy/commands/actions/type', () => {
})
})
// https://github.com/cypress-io/cypress/issues/5439
it('do not replace selection during modifier key', () => {
cy
.get('input:first').type('123')
.then(($el) => {
$el[0].setSelectionRange(0, 3)
})
.type('{ctrl}')
.should('have.value', '123')
})
// sends keyboard events for modifiers https://github.com/cypress-io/cypress/issues/3316
it('sends keyup event for activated modifiers when typing is finished', (done) => {
const $input = cy.$$('input:text:first')
+1 -1
View File
@@ -187,7 +187,7 @@ const commonConfig: webpack.Configuration = {
}),
]
),
...(liveReloadEnabled ? [new LiveReloadPlugin({ appendScriptTag: 'true', port: 0, hostname: 'localhost' })] : []),
...(liveReloadEnabled ? [new LiveReloadPlugin({ appendScriptTag: 'true', port: 0, hostname: 'localhost', protocol: 'http' })] : []),
],
cache: true,