745071 - Work - Implement touch events back end for metro

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Jim Mathies [:jimm]]

Todo:
- Implement w3c basic touch events (recently implemented in bug 603008).
- Add new simple gestures events for unique win8 actions like edge swipe.

Comment 1

[Jim Mathies [:jimm]]

The simple gesture is complete on Elm. W3C touch events for content still need to be added.

Comment 2

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Spoke with jimm about this on IRC; I'll investigate hooking up W3C touch events for content

Comment 4

[Jim Mathies [:jimm]]

Tim, the front end code that landed in bug 786519 will need to be updated when this bug is fixed. Ping me on irc when you get there so we can discuss.

Comment 5

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: WIP patch

TODO:
  1. Figure out why the soft keyboard isn't appearing with this patch applied
  2. Determine how/whether to dispatch `touchenter`, `touchleave`, `touchcancel`
  3. Remove code that simulates touch events from mouse events (see bug 786519)

Comment 6

[Jim Mathies [:jimm]]

Tim, could you test www.cuttherope.ie with this work to see if it fixes bug 769114 for metrofx?

Comment 7

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: WIP patch

This patch has multi-touch support, and is able to forward touch events to content.  However, the content events aren't getting propagated beyond the content window.  See following attachment for working test case.

Comment 8

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: Test case

This test shows that content touch events are being generated, and that we can support multi-touch.  Note, however, that the event listeners are on the content window rather than on the canvas element.  This is because touch events aren't yet being propagated to the objects on the page correctly.

Comment 9

[Jim Mathies [:jimm]]

Attachment: mdn sample

Comment 10

[Jim Mathies [:jimm]]

I applied your patch and made one change - rather than make a copy of the touch event in BTH, I just forwarded the original event as such:

// XXX position will be off when page is scrolled
getBrowser().contentWindow.dispatchEvent(aEvent);

Then I loaded up this sample, which has event listeners on the canvas, and I'm able to draw with multiple fingers.

I also killed off TouchEventHandler by commenting out it's init call.

The one problem with this is that because we have this crazy custom scroller which content doesn't know about, when scrolled the input will be off. (This is why we call transformClientToBrowser on mouse coordinates we forward.)

I think you are closer than you think.

cc'ing mbrubeck - matt, is it acceptable to catch a dom event in the overlay and forward that same event to the content window? Seems to work. The only problem is I'm not sure we can modify the client coordinates (I think they are read only) to adjust for our custom scroller, so maybe a copy of the event is needed. (Which would kinda suck.)

Comment 11

[Jim Mathies [:jimm]]

(Potentially the mouse coords in the touch data would need to be offset due to the scroller as well, which would really really suck.)

Comment 12

[Matt Brubeck (:mbrubeck)]

(In reply to Jim Mathies [:jimm] from comment #10)
> cc'ing mbrubeck - matt, is it acceptable to catch a dom event in the overlay
> and forward that same event to the content window? Seems to work. The only
> problem is I'm not sure we can modify the client coordinates (I think they
> are read only) to adjust for our custom scroller, so maybe a copy of the
> event is needed. (Which would kinda suck.)

If you can get the coordinates right, re-dispatching the event should work, as long as we are using in-process content.

Comment 13

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

(In reply to Jim Mathies [:jimm] from comment #10)
> I applied your patch and made one change - rather than make a copy of the
> touch event in BTH, I just forwarded the original event as such:
> 
> // XXX position will be off when page is scrolled
> getBrowser().contentWindow.dispatchEvent(aEvent);
> 
> Then I loaded up this sample, which has event listeners on the canvas, and
> I'm able to draw with multiple fingers.

I had tried forwarding the event without creating a copy but I got js and XPCOM errors, and the events failed to reach content.  I tried this again just now and got the same result.  The error text I get is:

WARNING: NS_ENSURE_TRUE(!NS_IS_EVENT_IN_DISPATCH(aEvent)) failed: file s:/elm1/obj/content/events/src/../../../../content/events/src/nsEventDispatcher.cpp, line 455

JavaScript error: chrome://browser/content/BrowserTouchHandler.js, line 308: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIDOMEventTarget.dispatchEvent]

`nsEventDispatcher` seems to think that the event is already being dispatched, and so can't be dispatched again.  It's interesting that this issue doesn't crop up in your builds; I'll pull from elm and clobber build in case this is only happening in older branches.

Comment 14

[Jim Mathies [:jimm]]

Attachment: testing

Comment 15

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: Updated test - adds event listeners to canvas instead of window

The updated patch (will post asap) works with this and other test cases

Comment 16

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: patch

winrt widget:
  - Generates and dispatches touch events
  - If `preventDefault()` is not called, also generates mouse events

BrowserTouchHandler.js overlay:
  - Captures events generated by widget
  - Generates content events, converting client coordinates
  - Dispatches content events to content window

Content.js overlay:
  - Receives events sent by BrowserTouchHandler.js
  - Generates new content events with correct targets
  - Dispatches content events to their targets

This is more complicated than I would have liked.  Specifically, it worries me that we now have 3 places where we're keeping track of touches (nsPresShell, MetroWidget, Content.js).  It's possible that Content.js and nsPresShell might disagree about the target for a particular `touchstart` event, and I'm not sure what the resulting behavior will look like.

This patch works beautifully with all the test pages I've tried:
  https://bug745071.bugzilla.mozilla.org/attachment.cgi?id=663183
  https://bug745071.bugzilla.mozilla.org/attachment.cgi?id=661103
  http://www.cuttherope.ie
  http://paulirish.com/demo/multi

Jim, what do you think?

Comment 17

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: patch

This cleans up some js exceptions in the console.  It uses the browser's `elementFromPoint` instead of the one in Content.js, and checks for invalid touch targets.

Comment 18

[Jim Mathies [:jimm]]

Curious, when you were getting those exception, were you running a debug build? I'm running a release, so maybe I just wasn't seeing them.

I'm working up a debug build today to do some testing in it.

Comment 19

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

(In reply to Jim Mathies [:jimm] from comment #18)
> Curious, when you were getting those exception, were you running a debug
> build? I'm running a release, so maybe I just wasn't seeing them.

Yup! It was a debug build. Though I would expect that even in release builds the exceptions would prevent the touch events from hitting the content window.

> I'm working up a debug build today to do some testing in it.

Thanks for taking a look!

Comment 20

[Jim Mathies [:jimm]]

(In reply to Tim Abraldes (:timA) (:tabraldes) from comment #19)
> (In reply to Jim Mathies [:jimm] from comment #18)
> > Curious, when you were getting those exception, were you running a debug
> > build? I'm running a release, so maybe I just wasn't seeing them.
> 
> Yup! It was a debug build. Though I would expect that even in release builds
> the exceptions would prevent the touch events from hitting the content
> window.

That's the wired thing, it definitely worked.

Comment 21

[Jim Mathies [:jimm]]

(In reply to Jim Mathies [:jimm] from comment #20)
> (In reply to Tim Abraldes (:timA) (:tabraldes) from comment #19)
> > (In reply to Jim Mathies [:jimm] from comment #18)
> > > Curious, when you were getting those exception, were you running a debug
> > > build? I'm running a release, so maybe I just wasn't seeing them.
> > 
> > Yup! It was a debug build. Though I would expect that even in release builds
> > the exceptions would prevent the touch events from hitting the content
> > window.
> 
> That's the wired thing, it definitely worked.

*weird

Comment 22

[Jim Mathies [:jimm]]

This isn't quite working right. On demos like paulirish and the mdn sample it works for two fingers, but once you go over two touch points input rendering doesn't occur until after you lift all your fingers off the screen.

Comment 23

[Jim Mathies [:jimm]]

Comment on attachment 663221 [details] [diff] [review]
patch

Review of attachment 663221 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/metro/base/content/BrowserTouchHandler.js
@@ +64,5 @@
>  
> +  _dispatchSyncTouchEventToContent: function(aName,
> +                                             aBubbles,
> +                                             aCancelable,
> +                                             aEvent) {

nit - please match the current parameter formatting in this module.

@@ +169,5 @@
> +      case "TapMove":
> +        this.tapMove(aEvent.clientX, aEvent.clientY);
> +        break;
> +      case "touchstart":
> +        this._dispatchSyncTouchEventToContent("Browser:TouchStart",

nit - same here and below, you just need to keep things to the 80 character limit.

::: browser/metro/base/content/contenthandlers/Content.js
@@ +270,5 @@
>    },
>  
>    init: function init() {
>      this._isZoomedToElement = false;
> +    this._touchTargets = {};

Let's move all this code out of Content.js and into TouchEventHandler.js.

::: browser/metro/base/content/contenthandlers/TouchEventHandler.js
@@ +38,5 @@
>  
>      // Synchronous events from browser we only receive when in-process
> +    //addEventListener("Browser:MouseDown", this, true);
> +    //addEventListener("Browser:MouseMove", this, true);
> +    //addEventListener("Browser:MouseUp", this, true);

We need to finish the cleanup on this module.

::: widget/windows/winrt/FrameworkView.cpp
@@ +323,5 @@
> +      preventDefault = mWidget->DispatchTouchEvent(NS_TOUCH_START,
> +                                                   aArgs->CurrentPoint);
> +    }
> +
> +    if (!preventDefault) {

You should get an sr on this from smaug or wesj. Personally I think widget backends should fire all events. Filtering should be handled by the event manager, not in a low level module like widget. This would also bring parity with the win32 backend.

@@ +372,5 @@
>      }
>    }
>  
> +  if (!preventDefault
> +   && mGestureInput) {

nit - weird formatting

@@ +397,4 @@
>    }
> +
> +  if (!preventDefault
> +   && mGestureInput) {

nit - weird formatting

::: widget/windows/winrt/MetroWidget.cpp
@@ +1079,5 @@
> +                                Windows::UI::Input::PointerPoint^ currentPoint)
> +{
> +  if (NS_TOUCH_MOVE == aEventType || NS_TOUCH_END == aEventType) {
> +    // Remove this touch from mTouches
> +    for (int i = 0; i < mTouches.Length(); i++) {

nit - idx or index vs. 'i'

@@ +1084,5 @@
> +      int32_t identifier;
> +      mTouches.ElementAt(i)->GetIdentifier(&identifier);
> +      if (currentPoint->PointerId == identifier) {
> +        mTouches.RemoveElementAt(i);
> +        break;

Is there any way we can avoid tracking mouse points and instead get at the real touch data? (Maybe through GetIntermediatePoints?) I'm concerned we're loosing data here by relying on what is considered mouse input vs. touch input data. If we can't get this data through the winrt event, maybe we could hook the touch event in our subclass and reuse the win32 code.

Comment 24

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: patch

(In reply to Jim Mathies [:jimm] from comment #23)
> Comment on attachment 663221 [details] [diff] [review]
> patch
> 
> Review of attachment 663221 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/metro/base/content/BrowserTouchHandler.js
> @@ +64,5 @@
> >  
> > +  _dispatchSyncTouchEventToContent: function(aName,
> > +                                             aBubbles,
> > +                                             aCancelable,
> > +                                             aEvent) {
> 
> nit - please match the current parameter formatting in this module.

Do you mean not putting the parameters on separate lines? That would cause the line to be longer than 80 characters. Other functions in the module that have long names / lots of parameters just go ahead and go over the 80 character limit (e.g. `_dispatchAsyncMouseEventToContent).  Is that the appropriate thing to do here?

> @@ +169,5 @@
> > +      case "TapMove":
> > +        this.tapMove(aEvent.clientX, aEvent.clientY);
> > +        break;
> > +      case "touchstart":
> > +        this._dispatchSyncTouchEventToContent("Browser:TouchStart",
> 
> nit - same here and below, you just need to keep things to the 80 character
> limit.

This line and the ones below are shorter than 80 characters, but bringing the rest of the parameters onto the first line would cause it to be longer than 80 characters.

> ::: browser/metro/base/content/contenthandlers/Content.js
> @@ +270,5 @@
> >    },
> >  
> >    init: function init() {
> >      this._isZoomedToElement = false;
> > +    this._touchTargets = {};
> 
> Let's move all this code out of Content.js and into TouchEventHandler.js.

Done.

> ::: browser/metro/base/content/contenthandlers/TouchEventHandler.js
> @@ +38,5 @@
> >  
> >      // Synchronous events from browser we only receive when in-process
> > +    //addEventListener("Browser:MouseDown", this, true);
> > +    //addEventListener("Browser:MouseMove", this, true);
> > +    //addEventListener("Browser:MouseUp", this, true);
> 
> We need to finish the cleanup on this module.

Done: This patch removes dead code and adds comments indicating what we should change if we want to support OoPC in the future.  Let me know if more cleanup is desired.

> ::: widget/windows/winrt/FrameworkView.cpp
> @@ +323,5 @@
> > +      preventDefault = mWidget->DispatchTouchEvent(NS_TOUCH_START,
> > +                                                   aArgs->CurrentPoint);
> > +    }
> > +
> > +    if (!preventDefault) {
> 
> You should get an sr on this from smaug or wesj. Personally I think widget
> backends should fire all events. Filtering should be handled by the event
> manager, not in a low level module like widget. This would also bring parity
> with the win32 backend.

I put this logic in widget based on the comment in [1].  It seems to make sense to avoid sending spurious mouse and gesture events from widget if we know they should be ignored.  Smaug: Do you have time to take a look at this?

> @@ +372,5 @@
> >      }
> >    }
> >  
> > +  if (!preventDefault
> > +   && mGestureInput) {
> 
> nit - weird formatting

Fixed.

> @@ +397,4 @@
> >    }
> > +
> > +  if (!preventDefault
> > +   && mGestureInput) {
> 
> nit - weird formatting

Fixed.

> ::: widget/windows/winrt/MetroWidget.cpp
> @@ +1079,5 @@
> > +                                Windows::UI::Input::PointerPoint^ currentPoint)
> > +{
> > +  if (NS_TOUCH_MOVE == aEventType || NS_TOUCH_END == aEventType) {
> > +    // Remove this touch from mTouches
> > +    for (int i = 0; i < mTouches.Length(); i++) {
> 
> nit - idx or index vs. 'i'

Fixed.
 
> @@ +1084,5 @@
> > +      int32_t identifier;
> > +      mTouches.ElementAt(i)->GetIdentifier(&identifier);
> > +      if (currentPoint->PointerId == identifier) {
> > +        mTouches.RemoveElementAt(i);
> > +        break;
> 
> Is there any way we can avoid tracking mouse points and instead get at the
> real touch data? (Maybe through GetIntermediatePoints?) I'm concerned we're
> loosing data here by relying on what is considered mouse input vs. touch
> input data. If we can't get this data through the winrt event, maybe we
> could hook the touch event in our subclass and reuse the win32 code.

`GetIntermediatePoints` is useful for finding out if the user is moving his finger faster than events can be processed: It gives a collection of the points that the finger moved to between the last event firing and this event firing.  If we're losing data by looking at `CurrentPoint` and not the intermediate points, it means that we're processing touch events too slowly to respond in real-time.  I think it's probably best to let Windows throttle the touch events for us; otherwise we would have to do that ourselves.  This code does start to get choppy with more fingers and when moving them faster (see comments below).  I think it'll be illuminating to test with a release build and see how bad our performance is there.

(In reply to Jim Mathies [:jimm] from comment #22)
> This isn't quite working right. On demos like paulirish and the mdn sample
> it works for two fingers, but once you go over two touch points input
> rendering doesn't occur until after you lift all your fingers off the screen.

I've noticed this also.  I think this is a performance issue: It seems like we're blocking the UI from updating while we're receiving a barrage of touch events.  Once the touch events stop then the UI updates.

[1] https://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp?rev=de04a102395f#6522

Comment 25

[Jim Mathies [:jimm]]

Tim, could you do me a favor and split the front and back end code up into two different patches?

Comment 26

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

Comment on attachment 664739 [details] [diff] [review]
patch

>+++ b/browser/metro/base/content/BrowserTouchHandler.js
I know nothing about this.

>diff --git a/browser/metro/base/content/contenthandlers/TouchEventHandler.js b/browser/metro/base/content/contenthandlers/TouchEventHandler.js
Or this

>diff --git a/widget/windows/winrt/FrameworkView.cpp b/widget/windows/winrt/FrameworkView.cpp
Or this

>diff --git a/widget/windows/winrt/MetroWidget.cpp b/widget/windows/winrt/MetroWidget.cpp
Or this

>diff --git a/widget/windows/winrt/MetroWidget.h b/widget/windows/winrt/MetroWidget.h
Or this


In general I wish we could keep all touch event dispatching in C++.
What is the reason for doing stuff in JS?

Comment 27

[Jim Mathies [:jimm]]

(In reply to Olli Pettay [:smaug] from comment #26)
> In general I wish we could keep all touch event dispatching in C++.
> What is the reason for doing stuff in JS?

This has to do with the crazy front end code we have in xul fennec. We have an overlay that traps incoming events and forwards them over to content. Since we haven't made the decision on e10s for metrofx, we kept the forwarding intact despite content being in process.

What I am curious about is sending events from the widget backend. On win32 we send mouse input and touch events and leave it to the event manager to filter out what gets sent to content. Here tim is doing the filtering down in widget by sending touch events first and looking at the result. 

I'm just interested in parity. Seems like widget shouldn't worry about w3c specs, it should just send all the events it has.

Comment 28

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: Patch - Dispatches W3C touch events when Windows sends us touch events

Comment 29

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: Part 2: Overlay code - Captures touch events and forwards them to content

Comment 30

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

(In reply to Jim Mathies [:jimm] from comment #27)
>
[snip]
> 
> What I am curious about is sending events from the widget backend. On win32
> we send mouse input and touch events and leave it to the event manager to
> filter out what gets sent to content. Here tim is doing the filtering down
> in widget by sending touch events first and looking at the result. 

I couldn't find anything in our widget code for touch processing [1] that dispatches mouse events in response to touch input: Maybe it happens somewhere other than our processing of `WM_TOUCH`?.  I did find IPC code that dispatches touch events and then dispatches mouse events if `preventDefault()` was not called on the touch event [2].  This seems to suggest that widget is not sending mouse events in response to touch input.  I'll argue below that, at least in our metro browser, we should be sending mouse events and gesture events from widget code in response to touch input.

> I'm just interested in parity. Seems like widget shouldn't worry about w3c
> specs, it should just send all the events it has.

By dispatching mouse events from widget in response to touch input, I think we're already putting w3c spec logic in widget.  If we want to remove all the w3c logic from widget, then widget would send only touch events and some higher-level module would decide whether to send mouse events and gesture events based on the touch events.

Separating the w3c logic from widget is complicated by the fact that we're relying on Windows to tell us when specific gestures have happened: The higher-level module would either need to do the gesture recognition itself (and we ignore Windows' ability to identify gestures for us) or the higher-level module would need to ignore gesture events if `preventDefault()` was called on the "touchstart" event or first "touchmove" event of the fingers associated with the gesture.  Putting the w3c logic in widget allows us to rely on Windows' gesture recognition, to avoid sending gesture events that need to be ignored, and to avoid writing complicated "ignore certain gestures if their constituent touches had `preventDefault()` called on them" logic.

I understand the concern in adding higher-level logic to widget: Widget seems to have been conceived as a low-level module for taking platform-specific input information and creating "Mozilla standard" events that can be handled in a generic way by higher-level logic.  However, the Metro platform is capable of giving us higher-level information than the win32 platform, making some of our higher-level processing unnecessary.  If we take advantage of the modern features of the platform by utilizing them in our widget module, we can avoid duplicating these features elsewhere.

[1] https://mxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow.cpp#6177
[2] https://mxr.mozilla.org/mozilla-central/source/dom/ipc/TabChild.cpp#989

Comment 31

[Jim Mathies [:jimm]]

(In reply to Tim Abraldes (:timA) (:tabraldes) from comment #30)
> I couldn't find anything in our widget code for touch processing [1] that
> dispatches mouse events in response to touch input: Maybe it happens
> somewhere other than our processing of `WM_TOUCH`?.

That's what I was curious about, if we receive WM_TOUCH before the winrt events process the input such that we can intercept them and process early.

> By dispatching mouse events from widget in response to touch input, I think
> we're already putting w3c spec logic in widget.  If we want to remove all
> the w3c logic from widget, then widget would send only touch events and some
> higher-level module would decide whether to send mouse events and gesture
> events based on the touch events.
> 
> Separating the w3c logic from widget is complicated by the fact that we're
> relying on Windows to tell us when specific gestures have happened: The
> higher-level module would either need to do the gesture recognition itself
> (and we ignore Windows' ability to identify gestures for us) or the
> higher-level module would need to ignore gesture events if
> `preventDefault()` was called on the "touchstart" event or first "touchmove"
> event of the fingers associated with the gesture.  Putting the w3c logic in
> widget allows us to rely on Windows' gesture recognition, to avoid sending
> gesture events that need to be ignored, and to avoid writing complicated
> "ignore certain gestures if their constituent touches had `preventDefault()`
> called on them" logic.
> 
> I understand the concern in adding higher-level logic to widget: Widget
> seems to have been conceived as a low-level module for taking
> platform-specific input information and creating "Mozilla standard" events
> that can be handled in a generic way by higher-level logic.  However, the
> Metro platform is capable of giving us higher-level information than the
> win32 platform, making some of our higher-level processing unnecessary.  If
> we take advantage of the modern features of the platform by utilizing them
> in our widget module, we can avoid duplicating these features elsewhere.

Ok, we'll see how it works out.

Comment 32

[Jim Mathies [:jimm]]

Comment on attachment 665049 [details] [diff] [review]
Part 2: Overlay code - Captures touch events and forwards them to content

So my greatest concern here is that all this event process seems to drag painting down such that using more than two fingers on various touch apps doesn't work. Although due to the overlay and the content window target, there doesn't seem to be a way around this.

AFAICT the backend is not the slow part but we could do some profiling to confirm that.

Transferring review over to Mark. Curious if he has any ideas.

Comment 33

[Jim Mathies [:jimm]]

A quick test of removing the overlay and disabling BrowserTouchHandler event listeners improves things quite a bit. So a big part of the performance bottle neck is in all this crazy event forwarding we need to do.

Comment 34

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

(it is not clear to me why the need for event forwarding and for the overlay)

Comment 35

[Jim Mathies [:jimm]]

Comment on attachment 665049 [details] [diff] [review]
Part 2: Overlay code - Captures touch events and forwards them to content

I think before this can land the input overlay needs to come out.

Comment 36

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Comment on attachment 665049 [details] [diff] [review]
Part 2: Overlay code - Captures touch events and forwards them to content

(In reply to Olli Pettay [:smaug] from comment #34)
> (it is not clear to me why the need for event forwarding and for the overlay)

As Jim mentioned, the overlay is a remnant from the mobile XUL code.  We were keeping it around because that would have been a path forward if we ever wanted to support e10s.

In bug 794621 we're deciding to get rid of the overlay: If we want to support e10s in the future we'll find another way, like we did for mobile.  That makes the front-end patch of this bug obsolete.

Comment 37

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Comment on attachment 665048 [details] [diff] [review]
Patch - Dispatches W3C touch events when Windows sends us touch events

The patch for this bug is going to be integrated into the patch in bug 808847, so I'm marking the patch here as obsolete.  This patch needs a major overhaul since we've backported to VS2010 anyway.

Comment 38

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

I'm porting this patch to VS2010 and getting it ready for submission with bug 794621.  Marking this bug as metro-it1

Comment 39

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

The patches in bug 794621 have been submitted to elm.  The patch that contains the changes for this bug is:
https://hg.mozilla.org/projects/elm/rev/e38fa39ccf69

This build contains the changes:
https://tbpl.mozilla.org/?tree=Elm&rev=e38fa39ccf69

Comment 41

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Marking my [completed-elm] bugs as resolved