unity-mir

Merge lp://qastaging/~mterry/unity-mir/no-focus into lp://qastaging/unity-mir

no-focus
Merge into trunk

Proposed by Michael Terry on 2014-05-22

Status:

Merged

Approved by:

Michael Terry on 2014-05-26

Approved revision:

226

Merged at revision:

225

Proposed branch:

lp://qastaging/~mterry/unity-mir/no-focus

Merge into:

lp://qastaging/unity-mir

Diff against target:

53 lines (+32/-0)

2 files modified

src/modules/Unity/Application/application_manager.cpp (+2/-0)
tests/application_manager_test.cpp (+30/-0)

To merge this branch:

bzr merge lp://qastaging/~mterry/unity-mir/no-focus

Undecided

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Gerry Boland (community)			Approve on 2014-05-26
PS Jenkins bot (community)	continuous-integration		Approve on 2014-05-26
Michał Sawicz		2014-05-22	Needs Information on 2014-05-23
Review via email: mp+220718@code.qastaging.launchpad.net

Commit message

If no app has focus when we get a un-actionable onSessionFocused event, still re-set focus to the shell.

Description of the change

If no app has focus when we get a un-actionable onSessionFocused event, still re-set focus to the shell.

During testing of split greeter, Saviq noticed that the volume up/down buttons didn't work. Some digging discovered that the android input layer thought nothing was focused and that it only happens after maliit-server is running.

Digging into unity-mir, it seems we discard onSessionFocused events that don't relate to an application (like maliit-server was generating). But apparently, android-input is a little confused at that point, and there is code in the function for re-setting the focus for the focused app.

But if no app has focus, we did nothing. In the case where no app has focus, we should also re-set the focus onto the shell. This fixed the volume bug for me.

== Checklist ==

* Are there any related MPs required for this MP to build/function as expected? Please list.
- No

* Did you perform an exploratory manual test run of your code change and any related functionality?
- Yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
- NA

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-22:

PASSED: Continuous integration, rev:224
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/349/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-amd64-ci/13
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/13
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/13/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-i386-ci/13

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-mir-ci/349/rebuild

review: Approve (continuous-integration)

Revision history for this message

Michał Sawicz (saviq) wrote on 2014-05-23:

Could we have a test for this?

review: Needs Information

Revision history for this message

Gerry Boland (gerboland) wrote on 2014-05-23:

+1 on the test request

lp://qastaging/~mterry/unity-mir/no-focus updated on 2014-05-23

225. By Michael Terry on 2014-05-23: Add tests

Revision history for this message

Michael Terry (mterry) wrote on 2014-05-23:

Tests added.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-23:

PASSED: Continuous integration, rev:225
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/357/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-amd64-ci/21
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/21
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/21/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-i386-ci/21

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-mir-ci/357/rebuild

review: Approve (continuous-integration)

Revision history for this message

Gerry Boland (gerboland) wrote on 2014-05-26:

DLOG("Invalid application focused, discarding the event");
not correct any more, please update comment

Please add the checklist too
https://wiki.ubuntu.com/Process/Merges/Checklists/Unity-Mir

review: Needs Fixing

Revision history for this message

Gerry Boland (gerboland) wrote on 2014-05-26:

"Oo" <- as before please just make empty string

Revision history for this message

Gerry Boland (gerboland) wrote on 2014-05-26:

Testing this, everything seems ok

lp://qastaging/~mterry/unity-mir/no-focus updated on 2014-05-26

226. By Michael Terry on 2014-05-26: simplify test

Revision history for this message

Michael Terry (mterry) wrote on 2014-05-26:

> DLOG("Invalid application focused, discarding the event");
> not correct any more, please update comment

I disagree. It means discarding the request to focus the new app. The code that follows is just rolling back any side effects of handling the request. I only added a special case to that code anyway, rather than change its meaning.

I could change the comment to "refocusing previous focus" or something like that. But it sounds like a wordier version of the "not doing anything" meaning we already have.

> Please add the checklist too
> https://wiki.ubuntu.com/Process/Merges/Checklists/Unity-Mir

Done.

> "Oo" <- as before please just make empty string

Done.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-26:

PASSED: Continuous integration, rev:226
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/362/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-amd64-ci/26
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/26
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/26/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-i386-ci/26

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-mir-ci/362/rebuild

review: Approve (continuous-integration)

Revision history for this message

Gerry Boland (gerboland) wrote on 2014-05-26:

> > DLOG("Invalid application focused, discarding the event");
> > not correct any more, please update comment
>
> I disagree. It means discarding the request to focus the new app.

Ok. Approved!

review: Approve

Revision history for this message

Gerry Boland (gerboland) wrote on 2014-05-26:

* Did you perform an exploratory manual test run of the code change and any related functionality?
Y
* Did CI run pass? If not, please explain why.
Y