Closed Bug 929377 Opened 8 years ago Closed 8 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: gweng, Assigned: fbukevin)

Details

(Whiteboard: [good first bug][mentor=alive][mentor-lang=zh])

Attachments

(1 file, 2 obsolete files)

46 bytes, text/x-github-pull-request
alive
: review+
Details | Review
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
Assignee: nobody → gweng
I want to test this on more devices.
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.
Assignee: gweng → nobody
Whiteboard: [good-first-bug]
Hi, everyone!

I'm going to study this bug now.

Thanks for any advise.
Assignee: nobody → fbukevin
Attached file Patch (obsolete) —
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.
Attachment #824689 - Flags: review?(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.
Attachment #824689 - Flags: review?(alive)
Attached file New patch to fixed this bug. (obsolete) —
Attachment #824689 - Attachment is obsolete: true
Hi!

I've removed all new variables 'isScreenchange' that created by me and replaced it by 'Lockscreen.locked'.
Please take a look, thanks!
Whiteboard: [good-first-bug] → [good first bug]
Attachment #825362 - Flags: review?(alive)
Attachment #825362 - Flags: review?(alive)
Attachment #825362 - Flags: review?(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.
Attachment #825362 - Flags: review?(alive)
Attachment #825362 - Flags: review?(alive)
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 on attachment 825362 [details] [review]
New patch to fixed this bug.

Let me know when you are ready to merge
Attachment #825362 - Flags: review?(alive) → review+
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!
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)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [good first bug] → [good first bug][mentor=alive][mentor-lang=zh]
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
Flags: needinfo?(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
Flags: needinfo?(alive)
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
Flags: needinfo?(alive)
Update the code and send a new pull request please.
Flags: needinfo?(alive)
New patch that fixed this bug and mentioned unit test problem
Attachment #825362 - Attachment is obsolete: true
Flags: needinfo?(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.
Flags: needinfo?(alive)
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!
Attachment #8356472 - Flags: review?(alive)
Attachment #8356472 - Flags: review?(alive) → review+
https://github.com/mozilla-b2g/gaia/commit/d3d802e7855fef9b181cfad7dd6efa42ed153a89
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.