741666 - touches for touchLists are not refcounted correctly

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Wesley Johnston (:wesj)]

If I do the following:

addEventListener("touchstart", function(aEvent) {
  // console.log(aEvent.touches.length); // If I include this line,
                                         // the second log will return 1, otherwise 0
  setTimeout(function() {
    console.log(aEvent.touches.length);
  }, 0);
}, false);

I find that all the touch lists are length 0 inside the setTimeout loop. We lazily build and cache the touchlist when you access it. If we don't create the cache outside the loop, the nsTArray of touches held by our nsTouchEvent is being destroyed, and we build a touch list of length 0.

Comment 1

[Wesley Johnston (:wesj)]

Attachment: Patch

We aren't duplicating these correctly.

Fix + test. Still building to run the test here, and will push to try as well.

Comment 2

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

Comment on attachment 612034 [details] [diff] [review]
Patch


>     case NS_TOUCH_EVENT:
>     {
>-      newEvent = new nsTouchEvent(false, msg, nsnull);
>+      nsTouchEvent *oldTouchEvent = static_cast<nsTouchEvent*>(mEvent);
>+      newEvent = new nsTouchEvent(oldTouchEvent);
>       NS_ENSURE_TRUE(newEvent, NS_ERROR_OUT_OF_MEMORY);
>       isInputEvent = true;
>       break;
To be consistent with other events, make sure newEvent is not trusted, and add a test for that.
(your code makes it now trusted, if oldTouchEvent is trusted)

with that, r=me

Comment 3

[Wesley Johnston (:wesj)]

Attachment: Patch v2

This modifies our constructor so that it takes an isTruested parameter and passes in false like everyone else, but in (all?) cases, that gets overridden down here:


http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsDOMEvent.cpp#925


For now I left that in (but the test checks that isTrusted is always true now). Do we want to change that behavior?

Comment 4

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

Comment on attachment 612073 [details] [diff] [review]
Patch v2


> 
>-      nsTouchEvent newEvent(touchEvent);
>+      nsTouchEvent newEvent(touchEvent->flags & NS_EVENT_FLAG_TRUSTED ?
>+                              true : false,
NS_IS_TRUSTED_EVENT(touchEvent)

Comment 5

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/507c719614ec

Comment 6

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/507c719614ec