1036985 - Looks like, MetroFireFox got regressions with pointercancel event

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: General, defect)

Description

[Maksim Lebedev]

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0 (Beta/Release)
Build ID: 20140605174243

Steps to reproduce:

I use MetroFireFox from m-c


Actual results:

Tests, which can detect pointercancel event in past, now doesn't triggered pointercancel event.


Expected results:

Tests should be passed.
Behavior should be correct according with specification.

Comment 1

[Maksim Lebedev]

First investigation: looks like, regression was made by Bug 998025.
Very strange changes were made in file widget/windows/winrt/MetroInput.cpp
Looks like, pointercancel events doesn't fire after this changes.

Comment 2

[Botond Ballo [:botond]]

The changes to MetroInput.cpp were made in the Part 9 patch of bug 998025, "Ignore touch/mouse events when in an overscrolled state".

This patch changed the semantics of the return value of APZCTreeManager::ReceiveInputEvent so that it returns: 

  - nsEventStatus_eConsumeDoDefault if the event matched a non-overscrolled APZC (the usual case)
  - nsEventStatus_eConsumeNoDefault if the event matched an overscrolled APZC
  - nsEventStatus_eIgnore if the event did not match any APZC

and modified callers of ReceiveInputEvent (in RenderFrameParent/TabParent for B2G and in MetroInput for Metro), which previous ignored this return value, to ignore the event (i.e. do not send it to content) if eConsumeNoDefault was returned.

Some relevant discussion can be found in comments 29-32 of bug 998025.

It is possible that we modified MetroInput incorrectly.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Please provide a test case, or clarify what the expected behaviour is intended to be.

Comment 4

[Maksim Lebedev]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #3)
> Please provide a test case, or clarify what the expected behaviour is
> intended to be.
Please, try use this:
https://github.com/w3c/web-platform-tests/blob/master/pointerevents/pointerevent_pointercancel_touch-manual.html
https://github.com/w3c/web-platform-tests/blob/master/pointerevents/pointerevent_releasepointercapture_onpointercancel_touch-manual.html
I hope that tests will be in m-c not so far. (Bug 1000870).

Comment 5

[Kartikaya Gupta (email:[e-mail])]

So those tests don't really explain what I was looking for. I read through the pointer events spec though and found what I was looking for. As I understand it, the ONLY dependency on the APZC code of the pointer events spec is that you somehow need to know when the input events are being consumed for panning/zooming purposes. If that starts to happen then you have to send a pointercancel event to content if there was a pointerdown previously sent and the we're in an active buttons state.

If I understand correctly, before bug 998025 you were using the return value of the APZCTreeManager::ReceiveInputEvent to determine if the event was being consumed for panning/zooming. However as we start moving input events off the main thread I don't think this is a viable route forward. Instead, I think you should try to use the NotifyAPZStateChange callback [1] to find this out, and then send the pointercancel event accordingly. The metro implementation of this callback [2] already exists and is used for switching between precise/imprecise input mode and/or showing the selection monocles.

Does that sound reasonable?

[1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/public/GeckoContentController.h?rev=dd4a1c9b02fa#151
[2] http://mxr.mozilla.org/mozilla-central/source/widget/windows/winrt/APZController.cpp#306

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Also to clarify: the reason I don't think using the return value of ReceiveInputEvent is a viable route forward is because ReceiveInputEvent will be running on the input-receiving thread, but it may still have to wait for web content to do a preventDefault on the corresponding touch events. This means that at the time ReceiveInputEvent is called we won't necessarily know if the events are going to be used for panning/zooming or not. We will only know that at some point in the future.

We could hijack the ContentReceivedTouch callback and stick in some code there to pick up the prevent-default notification but I think that's going to be unnecessarily brittle. Even if content doesn't cancel the events we might still not pan/zoom for some other reason, so you would have to duplicate APZC logic in widget code to know for sure. With the NotifyAPZStateChange callback you will be guaranteed to know what the APZ is doing with its stream of input events. The only tricky part will be making sure that everything is thread-safe and all the events are dealt with in the correct order.

Comment 7

[Maksim Lebedev]

Attachment: pointercancel_behavior_ver1.diff

Comments and suggestions are welcome.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8459649 [details] [diff] [review]
pointercancel_behavior_ver1.diff

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

::: widget/windows/winrt/APZController.cpp
@@ +355,5 @@
>    }
>  }
>  
> +void APZController::DispatchTouchCancelEvent(const MultiTouchInput* aEvent)
> +{

you're not using aEvent here at all, so it shouldn't be needed. Most of the changes in this patch go away if you remove this.

Comment 9

[Maksim Lebedev]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #8)
> you're not using aEvent here at all, so it shouldn't be needed. Most of the
> changes in this patch go away if you remove this.
Unfortunately, I cannot remove this code, because I need get information about touches.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

(In reply to Maksim Lebedev from comment #9)
> Unfortunately, I cannot remove this code, because I need get information
> about touches.

But you're not actually using aEvent in the patch. What information are you getting from it?

Comment 11

[Maksim Lebedev]

Attachment: pointercancel_behavior_ver2.diff

+ Changes: Added restoring WidgetTouchEvent
+ Changes: Added getting access to Widget

Comments and suggestion are very welcome.

Comment 12

[Maksim Lebedev]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #10)
> But you're not actually using aEvent in the patch. What information are you
> getting from it?
I need information for synthetize correct TOUCH_CANCEL event.

Comment 13

[Olli Pettay [:smaug][[e-mail]](vacation until July 14)]

Comment on attachment 8460268 [details] [diff] [review]
pointercancel_behavior_ver2.diff

This is really for kats.

Comment 14

[Kartikaya Gupta (email:[e-mail])]

Sorry for the delay here. I spent some time looking over the code and the spec and I think I understand what you're trying to do. However I also think I probably broke some stuff in metro when I landed bug 1009733, and that unless that is fixed this fix doesn't make sense. I'm going get my metro build going again to verify the current status and see if I can fix things up again.

Comment 15

[Maksim Lebedev]

Attachment: pointercancel_behavior_ver3.diff

+ Changes: Dispatch event via Widget with to correct frame

Comment 16

[Maksim Lebedev]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #14)
> Sorry for the delay here. I spent some time looking over the code and the
> spec and I think I understand what you're trying to do. However I also think
> I probably broke some stuff in metro when I landed bug 1009733, and that
> unless that is fixed this fix doesn't make sense. I'm going get my metro
> build going again to verify the current status and see if I can fix things
> up again.
Nice to hear it. This is very important bug because now it is the latest bug,
which get fails by MetroFF with tests of pointerevents specification.

Comment 17

[Maksim Lebedev]

Test: release capture after pointercancel.
Seems patch resolve issue, when immediately after touch down we send touch up event.
Current issue: after touch down send touch move event. In this case pointercancel is not generated.

Comment 18

[Maksim Lebedev]

Attachment: pointercancel_behavior_ver4.diff

+ Changes: Added discarding flag convertToPointer to reuse in pointerCancel event

Comment 19

[Maksim Lebedev]

Current state: I doubt that we need use APZController::DispatchTouchCancelEvent

Comment 20

[Jim Mathies [:jimm]]

Comment on attachment 8461532 [details] [diff] [review]
pointercancel_behavior_ver4.diff

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

Mostly just nits. kats needs to give final r+ for this, I haven't worked with our apz code in a while.

::: widget/InputData.h
@@ +182,5 @@
>    // SingleTouchData. It also sends garbage for the identifier, radius, force
>    // and rotation angle.
>    MultiTouchInput(const WidgetMouseEvent& aMouseEvent);
>  
> +  void RestoreWidgetTouchEvent(WidgetTouchEvent& aEvent, bool restore_type = true) const;

nit - this needs a comment explaining what this method does.

::: widget/windows/winrt/APZController.h
@@ +71,5 @@
>    // todo: make this a member variable as prep for multiple views
>    static nsRefPtr<mozilla::layers::APZCTreeManager> sAPZC;
>  
>  private:
> +  nsIWidget* mWidget;

Should this be a ref ptr?

::: widget/windows/winrt/MetroInput.cpp
@@ +1470,5 @@
>        continue;
>      }
> +    // This touch could was used with previous event,
> +    // that's why we should discard this flag
> +    // to correctly generate corresponding pointer event.

nit - this comment doesn't make a lot of sense. Can you clean it up and make it more clear please?

::: widget/xpwidgets/InputData.cpp
@@ +67,5 @@
>      mTouches.AppendElement(data);
>    }
>  }
>  
> +void MultiTouchInput::RestoreWidgetTouchEvent(WidgetTouchEvent& aTouchEvent, bool restore_type) const

nit -

void
MultiTouchInput::Restore...

Comment 21

[Maksim Lebedev]

Attachment: pointercancel_behavior_ver5.diff

+ Changes: Changed comment
- Changes: Removed unnecessary code

Comment 22

[Maksim Lebedev]

https://tbpl.mozilla.org/?tree=Try&rev=2a3cee3dd519

Comment 23

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8462441 [details] [diff] [review]
pointercancel_behavior_ver5.diff

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

This patch is significantly different from the last one, and the changes to nsPresShell.cpp are just moving code around. It seems like this is a one-line fix? Unflagging for review until you confirm this is the right patch because it looks like you just uploaded the wrong thing.

Also please keep an eye on bug 1043608 which I marked as blocking this. It rewrites all of the touch-handling code in MetroInput so that it conforms to the behaviour expected by APZC. As part of that I got rid of this DispatchTouchCancel function but I can certainly put it back for dispatching cancel events to content if that's what is needed here.

I'm still thinking about the different possible codepaths that can be executed here and trying to find a way to send this cancel event that guarantees it will be sent at the right time.

Comment 24

[Kartikaya Gupta (email:[e-mail])]

(In reply to Maksim Lebedev from comment #4)
> Please, try use this:
> https://github.com/w3c/web-platform-tests/blob/master/pointerevents/
> pointerevent_pointercancel_touch-manual.html
> https://github.com/w3c/web-platform-tests/blob/master/pointerevents/
> pointerevent_releasepointercapture_onpointercancel_touch-manual.html

Is there a place that these tests are hosted that I can run them directly?

Comment 25

[Kartikaya Gupta (email:[e-mail])]

Attachment: WIP

The attached patch, applied on top of the patches in bug 1043608, makes those two tests pass. However it's not technically a correct fix because in some cases it will cancel the wrong touches. I think the same is true of your earlier patch (attachment 8461532 [details] [diff] [review]). There's some inherent difficulties because the events are queued up both in the metro widget code and in the APZC code, so by the time we know for sure that we're panning, we might have already dispatched some of the move events to web content. I'm still thinking about this...

Comment 26

[Maksim Lebedev]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #23)
> This patch is significantly different from the last one, and the changes to
> nsPresShell.cpp are just moving code around. It seems like this is a
> one-line fix? Unflagging for review until you confirm this is the right
> patch because it looks like you just uploaded the wrong thing.
I created build with DispatchTouchCancelEvent and without it.
And in both case MetroFF show the same result in our tests.
That's why I solve that the DispatchTouchCancelEvent is extra code (and remove it)

Comment 27

[Maksim Lebedev]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #24)
> Is there a place that these tests are hosted that I can run them directly?
You can get patches from Bug 1000870. This bug contain patches with all w3c tests.

Comment 28

[Maksim Lebedev]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #25)
> The attached patch, applied on top of the patches in bug 1043608, makes
> those two tests pass. However it's not technically a correct fix because in
> some cases it will cancel the wrong touches. I think the same is true of
> your earlier patch (attachment 8461532 [details] [diff] [review]). There's
> some inherent difficulties because the events are queued up both in the
> metro widget code and in the APZC code, so by the time we know for sure that
> we're panning, we might have already dispatched some of the move events to
> web content. I'm still thinking about this...
I need time to test your patch, but "it will cancel the wrong touches" seems unwanted.

Comment 29

[Maksim Lebedev]

(In reply to Maksim Lebedev from comment #28)
> I need time to test your patch, but "it will cancel the wrong touches" seems
> unwanted.
I created version with attachments from bug 1043608 and WIP patch and I tested our tests.
I got several fail results during testing. Also I got strange behavior:
pointercancel event doesn't cancel firing event to target (at first)
pointermove were fired a lot of times to target (at second)
pointerout/leave were generated on target like I use mouse, (at third), but I use touch.

Comment 30

[Maksim Lebedev]

Comment on attachment 8462441 [details] [diff] [review]
pointercancel_behavior_ver5.diff

Opposite, patch pointercancel_behavior_ver5.diff have minimal changes.
And all tests were passed successfully with normal behavior.
Also this patch doesn't "cancel wrong touches".

Comment 31

[Kartikaya Gupta (email:[e-mail])]

(In reply to Maksim Lebedev from comment #29)
> I created version with attachments from bug 1043608 and WIP patch and I
> tested our tests.
> I got several fail results during testing.

Can you list which tests were failing?

> Also I got strange behavior:
> pointercancel event doesn't cancel firing event to target (at first)
> pointermove were fired a lot of times to target (at second)
> pointerout/leave were generated on target like I use mouse, (at third), but
> I use touch.

I don't really understand what you mean here. Can you describe this in more detail, and provide steps to reproduce?

Comment 32

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8462441 [details] [diff] [review]
pointercancel_behavior_ver5.diff

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

This patch is a one-line change to MetroInput.cpp so my review is not needed here. If Jim accepts my patches in bug 1043608 then I can write a proper fix for this bug and the other issues you mentioned above.

Comment 33

[Maksim Lebedev]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #31)
> Can you list which tests were failing?
Build with patches: https://tbpl.mozilla.org/?tree=Try&rev=4c7a9d0a7435

Test: release capture on pointercancel
Action: touch down on target. Wait some seconds. Move touch out of target.
Actual: pointercancel doesnot trigger at target.
Additional strange behavior: A lot of pointermove event were fired at target.

Test: pointerout after pointercancel.
Action: touch down on target. Wait some seconds. Move touch out of target.
Expected: pointercancel received before pointerout
Actual: pointercancel doesn't fire at target.
Additional strange behavior: A lot of pointermove event were fired at target.

Comment 34

[Kartikaya Gupta (email:[e-mail])]

Attachment: IRC conversation about pointercancel

Comment 35

[Kartikaya Gupta (email:[e-mail])]

Attachment: IRC conversation about pointercancel

(In reply to Maksim Lebedev from comment #28)
> I need time to test your patch, but "it will cancel the wrong touches" seems
> unwanted.

Yeah, it is.

I discussed this further with mbrubeck on IRC to clarify when the pointercancel should be sent. For better compatibility with touchevents and for ease of implementation (particularly in the case where we have a dedicated input thread) we came to the conclusion that the pointercancel event dispatching should be independent of the preventDefault behaviour from touchevents. I'm attaching a copy of our conversation for posterity.

I'll write a new patch to go on top of the stuff I landed for bug 1043608 that dispatches pointercancel events based on this.

Comment 36

[Maksim Lebedev]

Testing of our patches shows that:
 - "pointercancel_behavior_ver5.diff" give us correct behavior of pointer events according with corresponding specification. Tests with pointercancel events will be passed succesfully. Patch resolve the main issue (which breaks pointercancel behavior). (and looks pretty simplier)
 - "WIP" with attachments from 1043608 give us incorrect behavior. Pointercancel event doesn't break sequence of pointermove events, and all this events fired at target.

Comment 37

[Maksim Lebedev]

Comment on attachment 8462441 [details] [diff] [review]
pointercancel_behavior_ver5.diff

(In reply to Olli Pettay [:smaug] from comment #13)
> Comment on attachment 8460268 [details] [diff] [review]
> pointercancel_behavior_ver2.diff
> This is really for kats.
Unfortunately, I should return You back. We need your opinion.

Comment 38

[Olli Pettay [:smaug][[e-mail]](vacation until July 14)]

Sure, v5 is something I could review. v2 was all about APZC

Comment 39

[Olli Pettay [:smaug][[e-mail]](vacation until July 14)]

Comment on attachment 8462441 [details] [diff] [review]
pointercancel_behavior_ver5.diff

># HG changeset patch
># Parent c02d0c194999d0d2009219d8c3dd3cf7a779103c
># User Lebedev Maksim <[e-mail]>
>Bug 1036985 - Add correct behavior of pointercancel event
>
>diff --git a/layout/base/nsPresShell.cpp b/layout/base/nsPresShell.cpp
>--- a/layout/base/nsPresShell.cpp
>+++ b/layout/base/nsPresShell.cpp
>@@ -6679,59 +6679,59 @@ DispatchPointerFromMouseOrTouch(PresShel
>   uint32_t pointerMessage = 0;
>   if (aEvent->eventStructType == NS_MOUSE_EVENT) {
>     WidgetMouseEvent* mouseEvent = aEvent->AsMouseEvent();
>     // if it is not mouse then it is likely will come as touch event
>     if (!mouseEvent->convertToPointer) {
>       return NS_OK;
>     }
>     int16_t button = mouseEvent->button;
>     switch (mouseEvent->message) {
>+    case NS_MOUSE_BUTTON_DOWN:
>+      pointerMessage = NS_POINTER_DOWN;
>+      break;
>     case NS_MOUSE_MOVE:
>       if (mouseEvent->buttons == 0) {
>         button = -1;
>       }
>       pointerMessage = NS_POINTER_MOVE;
>       break;
>     case NS_MOUSE_BUTTON_UP:
>       pointerMessage = NS_POINTER_UP;
>       break;
>-    case NS_MOUSE_BUTTON_DOWN:
>-      pointerMessage = NS_POINTER_DOWN;
>-      break;
Unrelated changes just make reviewing harder.
Code cleanups should be done in separate patches (in separate bugs).

>     WidgetTouchEvent* touchEvent = aEvent->AsTouchEvent();
>     // loop over all touches and dispatch pointer events on each touch
>     // copy the event
>     switch (touchEvent->message) {
>+    case NS_TOUCH_START:
>+      pointerMessage = NS_POINTER_DOWN;
>+      break;
>     case NS_TOUCH_MOVE:
>       pointerMessage = NS_POINTER_MOVE;
>       break;
>+    case NS_TOUCH_CANCEL:
>+      pointerMessage = NS_POINTER_CANCEL;
>+      break;
>     case NS_TOUCH_END:
>       pointerMessage = NS_POINTER_UP;
>       break;
>-    case NS_TOUCH_START:
>-      pointerMessage = NS_POINTER_DOWN;
>-      break;
>-    case NS_TOUCH_CANCEL:
>-      pointerMessage = NS_POINTER_CANCEL;
>-      break;
ditto.


>diff --git a/widget/windows/winrt/MetroInput.cpp b/widget/windows/winrt/MetroInput.cpp
>--- a/widget/windows/winrt/MetroInput.cpp
>+++ b/widget/windows/winrt/MetroInput.cpp
>@@ -1463,18 +1463,22 @@ MetroInput::DispatchTouchCancel(WidgetTo
I don't see MetroInput::DispatchTouchCancel anywhere in the tree.
So hard to review.

Comment 40

[Kartikaya Gupta (email:[e-mail])]

As I mentioned in comment 23 I removed that function as part of bug 1043608. It's no longer in m-c, and won't be in metro either once I merge the changes over. I'll write a new patch to fix this bug on top of those changes and based on the discussion I had with matt yesterday.

Comment 41

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 1 - Make the return value of ReceiveInputEvent provide the necessary information

Comment 42

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 2 - Remove mApzConsumingTouch flag which is now wrong

Comment 43

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 3 - Send pointercancel events when appropriate

Comment 44

[Kartikaya Gupta (email:[e-mail])]

https://tbpl.mozilla.org/?tree=Try&rev=b2bfa48a0f3d

Comment 45

[Botond Ballo [:botond]]

Comment on attachment 8466299 [details] [diff] [review]
Part 1 - Make the return value of ReceiveInputEvent provide the necessary information

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

::: gfx/layers/apz/src/APZCTreeManager.h
@@ +129,5 @@
>     * by the caller if it wants to do this.
>     *
> +   * The following values may be returned by this function:
> +   * nsEventStatus_eConsumeNoDefault is returned to indicate the
> +   *   caller should discard the event with extreme prejudice.

:)

::: gfx/layers/apz/src/AsyncPanZoomController.h
@@ +545,5 @@
>    /**
> +   * Given the number of touch points in an input event and touch block they
> +   * belong to, check if the event can result in a panning/zooming behavior.
> +   * This is primarily used to figure out when to dispatch the pointercancel
> +   * event for the pointer events spec.

It's not clear to me how "nsEventStatus" maps to "if it can result in panning/zooming behavior". I would either document the mapping (Ignore -> No, ConsumeDoDefault -> Yes, ConsumeNoDefault -> Unused), or change the return type to 'bool' and have the callers map it to nsEventStatus.

Comment 46

[Kartikaya Gupta (email:[e-mail])]

(In reply to Botond Ballo [:botond] from comment #45)
> It's not clear to me how "nsEventStatus" maps to "if it can result in
> panning/zooming behavior". I would either document the mapping (Ignore ->
> No, ConsumeDoDefault -> Yes, ConsumeNoDefault -> Unused), or change the
> return type to 'bool' and have the callers map it to nsEventStatus.

Yeah, good point. I'll change it to a bool.

Comment 47

[Kartikaya Gupta (email:[e-mail])]

Btw, I posted some small updates to the W3C test suite: https://github.com/w3c/web-platform-tests/pull/1138

Comment 48

[Jim Mathies [:jimm]]

Comment on attachment 8466301 [details] [diff] [review]
Part 3 - Send pointercancel events when appropriate

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

Thanks for updating this code!

Comment 49

[Kartikaya Gupta (email:[e-mail])]

No problem.

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/217c103a4b75
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/9333a7a2f125
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/faaf13afb149

Comment 50

[Maksim Lebedev]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #44)
> https://tbpl.mozilla.org/?tree=Try&rev=b2bfa48a0f3d
Unfortunately, this build have no ability to work as Metro.
As I correctly re-build this, I create this: https://tbpl.mozilla.org/?tree=Try&rev=4200f04c4686
During testing this version I got very strange behavior of tests:
In one of test I can see stable pointercancel event after first pointermove event.
But in another test this behavior could detected in only about 5% of test iterations.

Comment 51

[Kartikaya Gupta (email:[e-mail])]

If there's something here you want me to look at please let me know what it is and how i can reproduce it. Just saying that some test is failing intwrmittently doesn't provide any actionable information and is a waste of time.

Comment 52

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/217c103a4b75
https://hg.mozilla.org/mozilla-central/rev/9333a7a2f125
https://hg.mozilla.org/mozilla-central/rev/faaf13afb149

Comment 53

[Maksim Lebedev]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #51)
> If there's something here you want me to look at please let me know what it
> is and how i can reproduce it. Just saying that some test is failing
> intwrmittently doesn't provide any actionable information and is a waste of time.
I will try to provide information, but in any case, I would be grateful,
if this patches will be copyed from m-c to project/metro branch.

Comment 54

[Kartikaya Gupta (email:[e-mail])]

Maksim sent me an email with details on the failures. I believe they should be resolved by applying the min-height patch that is among the fixes I linked to in comment 47.

I'll build another merge from m-c to the metro branch later today and push it if it looks good.

Comment 55

[Kartikaya Gupta (email:[e-mail])]

I pushed the changes to the metro branch; merge cset http://hg.mozilla.org/projects/metro/rev/74f31dec75b1