929377 - [LockScreen][UtilityTray] Sleep the phone when UtilityTray shows would make it overlay on lockscreen

Status: RESOLVED FIXED

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

Description

[Greg Weng [:snowmantw][:gweng][:λ]]

STR:

1. Pull down the UtilityTray to the half of the screen (not pull to the end)
2. Do not release the finger and press the power button to make the phone sleep
3. Press the power button to wake the phone up

Actual Results:

* The UtilityTray will overlay on the lockscreen

Expected: 

* The UtilityTray will disappear and the lockscreen should be the only one component on the screen.

Tested on:

* Unagi, with pvt build 27.0a, gaia master

Comment 1

[Greg Weng [:snowmantw][:gweng][:λ]]

I want to test this on more devices.

Comment 2

[Greg Weng [:snowmantw][:gweng][:λ]]

This would be fixed by change one flag in the UtilityTray component: when we pull it to the half of screen, it's now shown yet. So we should adjust the flag to adapt the condition.

And due to it's a rare but simple bug, I want to tag it as a first-good-bug. If anyone feels it's a bug emergent bug, I'll fix it soon.

Comment 3

[Veck Hsiao]

Hi, everyone!

I'm going to study this bug now.

Thanks for any advise.

Comment 4

[Veck Hsiao]

Attachment: Patch

This is my first patch.
I modified the apps/systems/js/utility_tray.js.
I created a new variable 'isScreenchange' recording if the screen is changed.

Comment 5

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 824689 [details] [review]
Patch

Hi!

This is not bad, but I think we should just check Lockscreen.isLocked as https://github.com/fbukevin/gaia/blob/79c55ce68f72bb7a01632b76edcea6686ee5eac9/apps/system/js/utility_tray.js#L77 does. Make it simple and consistent.

Comment 6

[Veck Hsiao]

Attachment: New patch to fixed this bug.

Comment 7

[Veck Hsiao]

Hi!

I've removed all new variables 'isScreenchange' that created by me and replaced it by 'Lockscreen.locked'.
Please take a look, thanks!

Comment 8

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 825362 [details] [review]
New patch to fixed this bug.

Cannot r+ because of touching LockScreen property improperly. But's it's not far from the target.

Comment 9

[Veck Hsiao]

I removed the line "LockScreen.locked = true;".

As Alive implied, I think when the screenchange, screen is locked and the value of property "LockScreen.locked" might be set by screenchange. I need not to update it manually.

It still work!

Comment 10

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 825362 [details] [review]
New patch to fixed this bug.

Let me know when you are ready to merge

Comment 11

[Veck Hsiao]

I remove the change of eliminating one line.
I'm ready to merge, but I don't have authority doing merge.
Please do that for me, thanks!

Comment 12

[(inactive after 6/18) Alive Kuo [:alive]]

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

Comment 13

[Anthony Ricaud (:rik)]

I had to revert this for failing tests: ef152f3e039b8beb294950b9bd17a8ba4751ac70

1) [system] system/UtilityTray onTouch showing should be shown by a drag from the top:
Error: expected false to equal true
at chaiAssert (http://test-agent.gaiamobile.org:8080/common/test/helper.js:33)
at equal (http://test-agent.gaiamobile.org:8080/common/vendor/chai/chai.js:1250)
at (anonymous) (http://system.gaiamobile.org:8080/test/unit/utility_tray_test.js:88)
at wrapper (http://test-agent.gaiamobile.org:8080/common/test/mocha_generators.js:62)
at run (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:3709)
at runTest (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4081)
at (anonymous) (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4127)
at next (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4007)
at (anonymous) (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4016)
at next (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:3964)
at (anonymous) (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:3984)
at (anonymous) (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4932)

2) [system] system/UtilityTray onTouch hiding should not be hidden by a tap:
Error: expected false to equal true
at chaiAssert (http://test-agent.gaiamobile.org:8080/common/test/helper.js:33)
at equal (http://test-agent.gaiamobile.org:8080/common/vendor/chai/chai.js:1250)
at (anonymous) (http://system.gaiamobile.org:8080/test/unit/utility_tray_test.js:99)
at wrapper (http://test-agent.gaiamobile.org:8080/common/test/mocha_generators.js:62)
at run (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:3709)
at runTest (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4081)
at (anonymous) (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4127)
at next (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4007)
at (anonymous) (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4016)
at next (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:3964)
at (anonymous) (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:3979)
at done (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:3700)
at (anonymous) (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:3712)
at (anonymous) (http://test-agent.gaiamobile.org:8080/common/test/mocha_generators.js:46)
at wrapper (http://test-agent.gaiamobile.org:8080/common/test/mocha_generators.js:73)
at run (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:3709)
at next (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:3973)
at (anonymous) (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:3984)
at (anonymous) (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4932)

Comment 14

[Veck Hsiao]

Hi!

I tested the same test on my computer, but there was no failure happening in utility_tray_test.

I think just use LockScreen.locked makes this happened in travis testing.

Do you have any advice or I think I could use my original solution (create a new variable recording screen lock)?


The following is the link of tests I ran on travis:
https://travis-ci.org/mozilla-b2g/gaia/jobs/14862463

Comment 15

[(inactive after 6/18) Alive Kuo [:alive]]

Try to use MockLockscreen instead of var Lockscreen = { locked: false } in the unit test.

(In reply to Veck Hsiao from comment #14)
> Hi!
> 
> I tested the same test on my computer, but there was no failure happening in
> utility_tray_test.
> 
> I think just use LockScreen.locked makes this happened in travis testing.
> 
> Do you have any advice or I think I could use my original solution (create a
> new variable recording screen lock)?
> 
> 
> The following is the link of tests I ran on travis:
> https://travis-ci.org/mozilla-b2g/gaia/jobs/14862463

Comment 16

[Veck Hsiao]

I modified the content of related unit test file with your advice and ran my new patch on Travis.
Although there still exist some error, it seems that those new error are not my problem?

The following is my new test result:
https://travis-ci.org/fbukevin/gaia/jobs/15923316

Comment 17

[(inactive after 6/18) Alive Kuo [:alive]]

Update the code and send a new pull request please.

Comment 18

[Veck Hsiao]

Attachment: New patch that fixed this bug and mentioned unit test problem

New patch that fixed this bug and mentioned unit test problem

Comment 19

[(inactive after 6/18) Alive Kuo [:alive]]

Rebase your branch. You are seeing graying out merge button that means your branch is outdated. No test will run if your branch is outdated.

Comment 20

[Veck Hsiao]

Comment on attachment 8356472 [details] [review]
New patch that fixed this bug and mentioned unit test problem

Hi Alive!
I've rebased my branch and ran the test on Travis without error.
This is the result on TravisCI: https://travis-ci.org/mozilla-b2g/gaia/builds/16927887

Thanks!

Comment 21

[(inactive after 6/18) Alive Kuo [:alive]]

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