Merge lp:~unity-team/unity8/fix-1238232 into lp:unity8

Proposed by Andrea Cimitan on 2013-10-16
Status: Rejected
Rejected by: Michael Zanetti on 2013-11-13
Proposed branch: lp:~unity-team/unity8/fix-1238232
Merge into: lp:unity8
Diff against target: 12 lines (+2/-0)
1 file modified
Shell.qml (+2/-0)
To merge this branch: bzr merge lp:~unity-team/unity8/fix-1238232
Reviewer Review Type Date Requested Status
Michael Zanetti (community) Disapprove on 2013-10-24
Michael Terry 2013-10-16 Needs Information on 2013-10-21
PS Jenkins bot (community) continuous-integration Needs Fixing on 2013-10-16
Review via email: mp+191424@code.launchpad.net

Commit message

Return on showHome() if screen is off

To post a comment you must log in.
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:467
http://jenkins.qa.ubuntu.com/job/unity8-ci/1431/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/5061
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/3000
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-saucy/2299
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-saucy-amd64-ci/454
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-saucy-armhf-ci/1431
        deb: http://jenkins.qa.ubuntu.com/job/unity8-saucy-armhf-ci/1431/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-saucy-i386-ci/1430
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-saucy/1230
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-amd64/936
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-amd64/936/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/3002
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/3002/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/2509
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/2553
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/29
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/26

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1431/rebuild

review: Needs Fixing (continuous-integration)
Michael Terry (mterry) wrote :

Could use a comment in the code explaining why we are bailing if power is off (or at least, why this bit of code cares about it).

Doesn't this bug happen for all the launcher items, not just the Home one?

review: Needs Information
Michael Zanetti (mzanetti) wrote :

I'm confused by this: first of all yes, as mterry pointed out already this happens with every launcher item, not only with the home button. So this fix is really not the way to go.

Now, my question is this:

how does the scenario described in the bug differ from:

* Press the power button to lock the device
* Press the power button to wake the device up and have the greeter visible
* Click on the home icon

Why would that be a problem? This obviously differs if there is a Lockscreen enabled. On the phone that's a non-issue as the lockscreen is one level below the Greeter and is not affected by this. However, this might very well be an issue on the tablet. Right now however, we disable panel and swipe gesture in the locked tablet scenario.

review: Disapprove

Unmerged revisions

467. By Andrea Cimitan on 2013-10-16

Fixes lp 1238232

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Shell.qml'
2--- Shell.qml 2013-10-14 20:20:18 +0000
3+++ Shell.qml 2013-10-16 14:28:21 +0000
4@@ -558,6 +558,8 @@
5 }
6
7 function showHome() {
8+ if (powerConnection.status == Powerd.off) return
9+
10 var animate = !greeter.shown && !stages.shown
11 greeter.hide()
12 dash.setCurrentScope("home.scope", animate, false)

Subscribers

People subscribed via source and target branches