Closed
Bug 774989
Opened 12 years ago
Closed 12 years ago
Cross-process touch coordinates are ... inconsistent
Categories
(Core :: DOM: Events, defect)
Core
DOM: Events
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: cjones, Assigned: cjones)
Details
Attachments
(1 file, 1 obsolete file)
5.10 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
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?
Assignee | ||
Comment 1•12 years ago
|
||
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 ...
Assignee | ||
Comment 2•12 years ago
|
||
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)
Comment 3•12 years ago
|
||
Felipe, don't we translate mouse event coordinates somewhere?
Comment 4•12 years ago
|
||
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..?
Assignee | ||
Comment 5•12 years ago
|
||
(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.
Assignee | ||
Comment 6•12 years ago
|
||
(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)
Assignee | ||
Comment 7•12 years ago
|
||
Attachment #643313 -
Flags: review?(bugs)
Assignee | ||
Updated•12 years ago
|
Attachment #643263 -
Attachment is obsolete: true
Attachment #643263 -
Flags: feedback?(wjohnston)
Attachment #643263 -
Flags: feedback?(bugs)
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → jones.chris.g
Updated•12 years ago
|
Attachment #643313 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 8•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c1da77bc0bc2
Comment 9•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c1da77bc0bc2
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in
before you can comment on or make changes to this bug.
Description
•