Should find element with not-integer offset (closes #2080) #2760
Conversation
|
❌ Tests for the commit ef5c8b6 have failed. See details: |
|
@testcafe-build-bot retest |
|
❌ Tests for the commit ef5c8b6 have failed. See details: |
| const roundFn = browserUtils.isFirefox ? Math.ceil : Math.round; | ||
| const offsetX = this.options.offsetX; | ||
| const offsetY = this.options.offsetY; | ||
| const screenPointBeforeAction = getAutomationPoint(this.element, offsetX, offsetY, roundFn); |
There was a problem hiding this comment.
Why we need to pass roundFn to the getAutomationPoint?
There was a problem hiding this comment.
getAutomationPoint based on hammerhead's getOffsetPosition. Currently the getOffsetPosition is implemented to return rounded X, Y offset of element. When it rounds 50.1px to '50'px it makes document.elementFromPoint work wrong in FF, but if works perfectly fine in Chrome/IE. So we need to round 50.1 not to 50 but to 51 only in FF.
The test checks that that even with different positions we click in correct position of element (0, 0) in all browsers.
There was a problem hiding this comment.
Should we use these different round functions for getAutomationPoint only in this place? What about drag or move automation and etc.?
There was a problem hiding this comment.
Need to research, currently I'm not working on this PR
There was a problem hiding this comment.
Looks like we do not need to use different round functions in other places. The issue occurs because of combination getAutomationPoint and elementFromPoint. There is no such a combination in other places.
There was a problem hiding this comment.
I've checked one more time, and I've found that actually we can get error in FF in other places (i.e. drag and hover). So I'd better to modify getAutomationPoint function.
Moreover, I've discovered that new drag test does not work in IE. The reason is unclear but it does not connected to this issue. I think we need to create the separate issue for this case. What do you think?
There was a problem hiding this comment.
Yep, let's create separate issue for IE
There was a problem hiding this comment.
I've reasearched the issue in IE, and I've found that in case of drag (or hover) we can't guarantee that target element in { x, y } will be an expected element. For example in the drag automation, for calculating the target { x, y } position we get source element coordinates with the getAutomationPoint method and then add { offsetX, offsetY } integer values. Depending on source element position it can be rounded down or up, i.e. 3.1 => 3 but 3.5 => 4, so target coors can differ. Besides, target element can also have fractional position, so we can not predict how we need to round the coordinates.
I've decided to fix it only in FF, because this scenario is common and very easy to reproduce.
|
❌ Tests for the commit 7e8f42e have failed. See details: |
|
❌ Tests for the commit 26beaa6 have failed. See details: |
|
❌ Tests for the commit 6456c3c have failed. See details: |
|
✅ Tests for the commit 6456c3c have passed. See details: |
AlexKamaev
changed the title
|
please review |
| <div id="parent"> | ||
| <div id="child1" class="child"> | ||
| <div id="leaf1" class="leaf"></div> | ||
| <div id="drag"></div> |
There was a problem hiding this comment.
now we can remove this element
|
✅ Tests for the commit d0fa5c6 have passed. See details: |
|
@churkin FPR |
| var top = element === document.documentElement ? 0 : elementOffset.top; | ||
| const roundFn = browserUtils.isFirefox ? Math.ceil : Math.round; | ||
| const elementOffset = positionUtils.getOffsetPosition(element, roundFn); | ||
| const left = element === document.documentElement ? 0 : elementOffset.left; |
There was a problem hiding this comment.
Little thing:
const isDocumentElement = element === document.documentElement;But It does not matter.
…evExpress#2760) * Should find element with not-integer offset (closes DevExpress#2080) * more tests * remove tests * remove excess element
No description provided.