971006 - Remove a sync reflow when showing the utility tray

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Same fix than the one for edge swipe detector.

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: bug971006.patch

Same BrowserElementPanning.js issue that creates a reflow to found a pannable in the system app.

Comment 2

[Etienne Segonzac (:etienne)]

Comment on attachment 8374146 [details] [diff] [review]
bug971006.patch

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

Awesome! But you're breaking unit tests :)

We can either :
* keep doing what fakeTouches does, but make sure we sent fake objects with a preventDefault() method
* change it to forge real touch events like we do in other system app test suites

In both case we should also add a test to check that the default was prevented.

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Etienne Segonzac (:etienne) from comment #2)
> Comment on attachment 8374146 [details] [diff] [review]
> bug971006.patch
> 
> Review of attachment 8374146 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Awesome! But you're breaking unit tests :)
> 
> We can either :
> * keep doing what fakeTouches does, but make sure we sent fake objects with
> a preventDefault() method

It makes me sad that we don't use real events :(

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: bug971006.patch

Sent to Travis as well.

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: bug971006.patch

With some tests.

Comment 6

[Etienne Segonzac (:etienne)]

Comment on attachment 8376363 [details] [diff] [review]
bug971006.patch

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

r=me with a green travis, but I'm confident it will be :)

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Etienne Segonzac (:etienne) from comment #6)
> Comment on attachment 8376363 [details] [diff] [review]
> bug971006.patch
> 
> Review of attachment 8376363 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me with a green travis, but I'm confident it will be :)

As green as a zombie.

https://github.com/mozilla-b2g/gaia/commit/19304fe63b26630f95c6f3a7da49349b433d476e

Comment 8

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Reopening. Backed out bug 971006 as of

https://github.com/mozilla-b2g/gaia/pull/16539

Commit

https://github.com/mozilla-b2g/gaia/commit/b6b2f10d4784f478878b3cfdabc4bd66bf3da8ec

In order to fix regression in Bug 975018.

Comment 9

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Send a new version on Travis: https://github.com/mozilla-b2g/gaia/pull/16770

This one only preventDefault the events is they are targetting the statusbar or the grippy.

Comment 10

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: bug971006.v2.diff

Etienne here is a new version on top of bug 978687

Comment 11

[Etienne Segonzac (:etienne)]

Comment on attachment 8384686 [details] [diff] [review]
bug971006.v2.diff

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

r=me with the nits in the test.

I'm pretty impressed :)

::: apps/system/test/unit/utility_tray_test.js
@@ +241,5 @@
>        assert.ok(stub.calledOnce);
>      });
>  
> +    test('Dont preventDefault if the target is the overlay', function() {
> +      assert.ok(UtilityTray.overlay.dispatchEvent(fakeEvt));

nit: assert.isTrue

@@ +245,5 @@
> +      assert.ok(UtilityTray.overlay.dispatchEvent(fakeEvt));
> +    });
> +
> +    test('preventDefault if the target is the statusbar', function() {
> +      assert.ok(!UtilityTray.statusbar.dispatchEvent(fakeEvt));

assert.isFalse

@@ +249,5 @@
> +      assert.ok(!UtilityTray.statusbar.dispatchEvent(fakeEvt));
> +    });
> +
> +    test('preventDefault if the target is the grippy', function() {
> +      assert.ok(!UtilityTray.grippy.dispatchEvent(fakeEvt));

assert.isFalse

@@ +268,5 @@
>        UtilityTray.active = true;
> +    });
> +
> +    test('Dont preventDefault if the target is the overlay', function() {
> +      assert.ok(UtilityTray.overlay.dispatchEvent(fakeEvt));

same

@@ +272,5 @@
> +      assert.ok(UtilityTray.overlay.dispatchEvent(fakeEvt));
> +    });
> +
> +    test('preventDefault if the target is the statusbar', function() {
> +      assert.ok(!UtilityTray.statusbar.dispatchEvent(fakeEvt));

same

@@ +276,5 @@
> +      assert.ok(!UtilityTray.statusbar.dispatchEvent(fakeEvt));
> +    });
> +
> +    test('preventDefault if the target is the grippy', function() {
> +      assert.ok(!UtilityTray.grippy.dispatchEvent(fakeEvt));

same

Comment 12

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://github.com/mozilla-b2g/gaia/commit/73a4755535fa6051c7ed7c11bf44c1f62cdabe1b

Landed with nits addressed.