event should not work on iframe content (closes #1791, closes #1822 in testcafe) #1789
Conversation
|
@testcafe-build-bot retest |
|
@testcafe-build-bot retest |
AlexKamaev
changed the title
AlexKamaev
changed the title
AlexKamaev
changed the title
| asyncTest('hover style in iframe', function () { | ||
| return createTestIframe() | ||
| .then(function (iframe) { | ||
| var $style = $('<style>').appendTo('body'); |
There was a problem hiding this comment.
Don't use jquery for elements and property manipulation.
src/client/utils/position.js
Outdated
| @@ -226,6 +226,19 @@ export function getElementRectangle (el) { | |||
| return rectangle; | |||
| } | |||
|
|
|||
| export function isPositionInsideElement (el, x, y) { | |||
There was a problem hiding this comment.
export function shouldRaiseMouseoverInsideIframe (iframe, { x, y })
src/client/sandbox/event/hover.js
Outdated
| @@ -56,6 +58,14 @@ export default class HoverSandbox extends SandboxBase { | |||
| return jointParent; | |||
| } | |||
|
|
|||
| _onHover ({ target, clientX, clientY }) { | |||
| const isIframeContent = domUtils.getTagName(target) === 'iframe' && positionUtils.isPositionInsideElement(target, clientX, clientY); | |||
There was a problem hiding this comment.
1)Convert expression
domUtils.getTagName(target) === 'iframe' && positionUtils.isPositionInsideElement(target, clientX, clientY)
to utility function isPositionInsideIframe(el, {x, y}) and move this to the src/client/utils/position.js file.
2)Merge onHover and _hover function together. And rewrite using fast forward paradigm
_hover() {
const isPositionIsInIframe = postionUtils.isInIframe(el, x, y);
const shouldMarkElement = browserUtils.isIE && isPositionIsInIframe;
if (!shouldMarkElement)
return;
....
}There was a problem hiding this comment.
- ok
- I've separated them intentionally, because
_onHovermethod takesx,yparams while_hoveris just api method without any extra params. Besides_hoveris used in our tests
|
|
||
| const left = rect.left + borders.left + padding.left; | ||
| const top = rect.top + borders.top + padding.top; | ||
| const right = rect.left + rect.width - borders.right - padding.right; |
There was a problem hiding this comment.
Add comment for this line
| @@ -314,6 +315,15 @@ test('should not wrap invalid event handlers (GH-1251)', function () { | |||
| }); | |||
|
|
|||
| strictEqual(listeningCtx.getEventCtx(target, 'click').wrappers.length, storedHandlerWrappersCount); | |||
|
|
|||
| handlers.forEach(function (handler) { | |||
There was a problem hiding this comment.
Why do we need a try/catch statement?
There was a problem hiding this comment.
It is a test for invalid event handlers. See the line above: var handlers = [void 0, 1, null, 'str', {}];. The test adds some wrong event handlers, but does not remove them, so it leads to problems with next texts. I've added code which removes handlers in the same manner as code which adds handlers
| @@ -529,6 +529,9 @@ export default class EventSimulator { | |||
| } | |||
|
|
|||
| _dispatchMouseRelatedEvents (el, args, dataTransfer) { | |||
| if (shouldIgnoreMouseEventInsideIframe(el, args.clientX, args.clientY) && args.type !== 'mouseover' && args.type !== 'mouseenter') | |||
There was a problem hiding this comment.
I think that the type conditions should be the first for performance improvement.
DevExpress#1822 in testcafe) (DevExpress#1789) * event should not work on iframe content (closes DevExpress#1822) * fix test * fix safari tests * focus should work while click should not * changes * improve
No description provided.