The default bug view has changed. See this FAQ.

Cross-process touch coordinates are ... inconsistent

RESOLVED FIXED in mozilla17

Status

()

Core
DOM: Events
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: cjones, Assigned: cjones)

Tracking

Trunk
mozilla17
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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?
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 ...
Created attachment 643263 [details] [diff] [review]
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?
Attachment #643263 - Flags: feedback?(wjohnston)
Attachment #643263 - Flags: feedback?(bugs)
Felipe, don't we translate mouse event coordinates somewhere?
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..?
(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.
(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)
Created attachment 643313 [details] [diff] [review]
Transform touch-event coordinates for remote content
Attachment #643313 - Flags: review?(bugs)
Attachment #643263 - Attachment is obsolete: true
Attachment #643263 - Flags: feedback?(wjohnston)
Attachment #643263 - Flags: feedback?(bugs)
Assignee: nobody → jones.chris.g
Attachment #643313 - Flags: review?(bugs) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/c1da77bc0bc2
https://hg.mozilla.org/mozilla-central/rev/c1da77bc0bc2
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.