774989 - Cross-process touch coordinates are ... inconsistent

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

In b2g, the DOM setup looks something like

 <xul:window fullscreen>             // @x=0, y=0 in screen space
  <html:iframe id="system">          // @x=0, y=0
    <html:iframe id="browser-app">   // @x=0, y=20
      <html:iframe id="content">     // @x=y, y=70

I'm seeing touch points inside the "content" iframe at the wrong coordinates.  Synthetic mouse events dispatched from the parent process have the correct coordinates.

If I touch and drag a little near the top-left of the content iframe, I see a sequence of events like

  touchstart = { refpoint: x=0, y=-70 }
  touchmove = { refpoint: x=0, y=0, touches: [ x=0, y=70 ] }
  ...
  touchend = { refpoint: x=0, y=0 }

The touch points always seem to be in screen space.  That would be OK, except that the refpoint is inconsistent --- if it were always the frame offset to the widget, like it is for touchstart, then I could use that to move the points into the right space.

I'm not really sure where to start here --- wesj, smaug any hints?

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Random wild guess --- I wonder if the problem might be that some code is doing something like

    nsTouchEvent touchEvent(*static_cast<nsTouchEvent*>(aEvent));

and expecting a deep copy of the touch points, but in fact we're only shallowly copying the pointers to nsIDOMTouch ...

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Workaround

This patch fixes things, but is of course horrible.

Does this give you guys any hints about where to look for a real fix?

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Felipe, don't we translate mouse event coordinates somewhere?

Comment 4

[:Felipe Gomes (needinfo for replies!)]

In fact, we do change the mouse event coordinates to translate them to the target's coordinates before sending them, here:

http://hg.mozilla.org/mozilla-central/annotate/9799dad7067b/content/events/src/nsEventStateManager.cpp#l1738

So yeah, this adjustment is also probably needed for each touch point, which I didn't realize before

Chris, also note that you do that shallow copy here: https://hg.mozilla.org/integration/mozilla-inbound/rev/d1045701fee9#l6.15
But if the nsIDOMTouch points are read correctly in the serializer I don't expect that copy to be a problem (as they should arrive with the right coordinates)


I would guess the difference of behavior between event types is related to these two parts:

>  if (aEvent->eventStructType != NS_TOUCH_EVENT ||
>      aEvent->message == NS_TOUCH_START) {
...
>  } else {

and

>    nsPoint pt = nsLayoutUtils::GetEventCoordinatesRelativeTo(aEvent, aTargetFrame);

The coordinates translation here uses aTargetFrame, which is used in the if branch for touchstart, but for touchmove branch the target is different..?

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to Felipe Gomes (:felipe) from comment #4)
> In fact, we do change the mouse event coordinates to translate them to the
> target's coordinates before sending them, here:
> 
> http://hg.mozilla.org/mozilla-central/annotate/9799dad7067b/content/events/
> src/nsEventStateManager.cpp#l1738
> 

Aha!  That's exactly what I needed, thanks.
 
> Chris, also note that you do that shallow copy here:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/d1045701fee9#l6.15
> But if the nsIDOMTouch points are read correctly in the serializer I don't
> expect that copy to be a problem (as they should arrive with the right
> coordinates)
> 

Yes, this is another potential instance of that problem.  In general, any code like

  nsTouchEvent ev;
  Dispatch(ev);
  // do something with ev

that expects |ev| not to have changed, might not work.

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #5)
> (In reply to Felipe Gomes (:felipe) from comment #4)

>   nsTouchEvent ev;
>   nsTouchEvent ev;

Sorry, I meant

  nsTouchEvent copy(ev);
  Dispatch(copy)

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Transform touch-event coordinates for remote content

Comment 8

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c1da77bc0bc2

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c1da77bc0bc2