third_party/WebKit/LayoutTests/virtual/pointerevent/fast/events/pointerevents/pointer-event-properties-in-iframe-expected.txt - Issue 1964523002: Fix clientX/Y properties of touch pointer events

Side by Side Diff: third_party/WebKit/LayoutTests/virtual/pointerevent/fast/events/pointerevents/pointer-event-properties-in-iframe-expected.txt

Issue 1964523002: Fix clientX/Y properties of touch pointer events (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Applying comments Created 4 years, 7 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« third_party/WebKit/LayoutTests/virtual/pointerevent/fast/events/pointerevents/pointer-event-properties-in-iframe.html ('K') | « third_party/WebKit/LayoutTests/virtual/pointerevent/fast/events/pointerevents/pointer-event-properties-in-iframe.html ('k') | third_party/WebKit/Source/core/events/PointerEventFactory.h » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1

	2 This test verifies clientX/Y of pointer events inside iframe.

	3

	4 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE ".

	5

	6

	7 PASS successfullyParsed is true

	8

	9 TEST COMPLETE

	10 pointerdown of touch is recieved:

	11 clientX = 150

	12 clientY = 100

	13 pointermove of touch is recieved:

	14 clientX = 150

	15 clientY = 100

	16 pointerup of touch is recieved:

	17 clientX = 150

	18 clientY = 100

	19 pointerdown of touch is recieved:

	20 clientX = 150

	21 clientY = 100

	22 pointermove of touch is recieved:

	23 clientX = 150

	24 clientY = 100

	25 pointerup of touch is recieved:

	26 clientX = 150

	27 clientY = 100

	28 pointerdown of touch is recieved:

	29 clientX = 10

	30 clientY = -107
	bokan 2016/05/09 21:03:07 This looks wrong to me. For starters, at 2X zoom t This looks wrong to me. For starters, at 2X zoom the iframe is at (100px, 200px) in DIPs so I think the touch is occurring at (50px, 0px) in CSS pixels in the iframe's client coordinates. We should certainly not be seeing negative client coordinates here. Touching/clicking at the bottom right corner of the 300x300 iframe should give client coordinates of (299,299) CSS pixels regardless of zoom level. That said, I'm assuming eventSender coordinates are "renderer coordinates", so they'd be in DIPs relative to the window origin. That makes most sense to me but it's not an assumption I've confirmed. This would mean that as you zoom in, you'd have to increase the coordinates given to eventSender to hit the bottom right of the iframe. FYI, DIP / pageZoom = CSS pixel for this case. Navid Zolghadr 2016/05/10 15:01:40 I was looking at the implementation of contentsToF Show quoted text On 2016/05/09 21:03:07, bokan wrote: > This looks wrong to me. For starters, at 2X zoom the iframe is at (100px, 200px) > in DIPs so I think the touch is occurring at (50px, 0px) in CSS pixels in the > iframe's client coordinates. We should certainly not be seeing negative client > coordinates here. > > Touching/clicking at the bottom right corner of the 300x300 iframe should give > client coordinates of (299,299) CSS pixels regardless of zoom level. > > That said, I'm assuming eventSender coordinates are "renderer coordinates", so > they'd be in DIPs relative to the window origin. That makes most sense to me but > it's not an assumption I've confirmed. This would mean that as you zoom in, > you'd have to increase the coordinates given to eventSender to hit the bottom > right of the iframe. > > FYI, DIP / pageZoom = CSS pixel for this case. I was looking at the implementation of contentsToFrame function. It seems that it doesn't consider the zoom factor. I couldn't find another function that also considers that. Is that okay that I do it manually once again (basically the same way it was in the first patch)? bokan 2016/05/10 16:33:53 Ah, touchInfo is already zoomed...we should only b Show quoted text On 2016/05/10 15:01:40, Navid Zolghadr wrote: > On 2016/05/09 21:03:07, bokan wrote: > > This looks wrong to me. For starters, at 2X zoom the iframe is at (100px, > 200px) > > in DIPs so I think the touch is occurring at (50px, 0px) in CSS pixels in the > > iframe's client coordinates. We should certainly not be seeing negative client > > coordinates here. > > > > Touching/clicking at the bottom right corner of the 300x300 iframe should give > > client coordinates of (299,299) CSS pixels regardless of zoom level. > > > > That said, I'm assuming eventSender coordinates are "renderer coordinates", so > > they'd be in DIPs relative to the window origin. That makes most sense to me > but > > it's not an assumption I've confirmed. This would mean that as you zoom in, > > you'd have to increase the coordinates given to eventSender to hit the bottom > > right of the iframe. > > > > FYI, DIP / pageZoom = CSS pixel for this case. > > I was looking at the implementation of contentsToFrame function. It seems that > it doesn't consider the zoom factor. I couldn't find another function that also > considers that. > Is that okay that I do it manually once again (basically the same way it was in > the first patch)? Ah, touchInfo is already zoomed...we should only be applying zoom factor just before we provide it to the web contents. Internally, everything should be in unzoomed DIPs. lgtm to do this manually since this is all in flux (you mentioned getting rid of TouchInfo, right) but we should avoid making manual conversions like this long term. Navid Zolghadr 2016/05/10 17:46:42 Done. Show quoted text On 2016/05/10 16:33:53, bokan wrote: > On 2016/05/10 15:01:40, Navid Zolghadr wrote: > > On 2016/05/09 21:03:07, bokan wrote: > > > This looks wrong to me. For starters, at 2X zoom the iframe is at (100px, > > 200px) > > > in DIPs so I think the touch is occurring at (50px, 0px) in CSS pixels in > the > > > iframe's client coordinates. We should certainly not be seeing negative > client > > > coordinates here. > > > > > > Touching/clicking at the bottom right corner of the 300x300 iframe should > give > > > client coordinates of (299,299) CSS pixels regardless of zoom level. > > > > > > That said, I'm assuming eventSender coordinates are "renderer coordinates", > so > > > they'd be in DIPs relative to the window origin. That makes most sense to me > > but > > > it's not an assumption I've confirmed. This would mean that as you zoom in, > > > you'd have to increase the coordinates given to eventSender to hit the > bottom > > > right of the iframe. > > > > > > FYI, DIP / pageZoom = CSS pixel for this case. > > > > I was looking at the implementation of contentsToFrame function. It seems that > > it doesn't consider the zoom factor. I couldn't find another function that > also > > considers that. > > Is that okay that I do it manually once again (basically the same way it was > in > > the first patch)? > > Ah, touchInfo is already zoomed...we should only be applying zoom factor just > before we provide it to the web contents. Internally, everything should be in > unzoomed DIPs. lgtm to do this manually since this is all in flux (you mentioned > getting rid of TouchInfo, right) but we should avoid making manual conversions > like this long term. Done.
	31 pointermove of touch is recieved:

	32 clientX = 10

	33 clientY = -107

	34 pointerup of touch is recieved:

	35 clientX = 10

	36 clientY = -107

	37

OLD	NEW