Merge lp:~josharenson/unity8/fix_trust_store_focus into lp:unity8

Proposed by Josh Arenson
Status: Merged
Approved by: Michael Zanetti
Approved revision: 1371
Merged at revision: 1436
Proposed branch: lp:~josharenson/unity8/fix_trust_store_focus
Merge into: lp:unity8
Diff against target: 73 lines (+40/-1)
3 files modified
qml/Stages/SessionContainer.qml (+4/-0)
tests/mocks/Unity/Application/Session.h (+1/-1)
tests/qmltests/Stages/tst_SessionContainer.qml (+35/-0)
To merge this branch: bzr merge lp:~josharenson/unity8/fix_trust_store_focus
Reviewer Review Type Date Requested Status
Michael Zanetti (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+239567@code.launchpad.net

Commit message

Return focus to the application after a tusted session overlay is closing.

Description of the change

* 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?
N/A

Fix lp:1381292

This works around an issue where a trusted session dialog would steal focus from an underlying application/session and never return it.

To post a comment you must log in.
Michael Zanetti (mzanetti) wrote :

With the current way we're doing focus I guess this is the way to go, not really a hack. What is the reason why you think this is a hack? What would you say is the proper fix?

I guess I'm fine with approving this, maybe we can even remove the FIXME comment.

review: Needs Information
Josh Arenson (josharenson) wrote :

> With the current way we're doing focus I guess this is the way to go, not
> really a hack. What is the reason why you think this is a hack? What would you
> say is the proper fix?
>
> I guess I'm fine with approving this, maybe we can even remove the FIXME
> comment.

If focus worked correctly, this wouldn't be a hack. I'll remove the FIXME because any focus changes we make should work with this anyway.

1371. By Josh Arenson on 2014-11-12

Remove FIXME

Michael Zanetti (mzanetti) wrote :

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

yep

 * Did CI run pass? If not, please explain why.

it did

 * Did you make sure that the branch does not contain spurious tags?

yeah

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'qml/Stages/SessionContainer.qml'
2--- qml/Stages/SessionContainer.qml 2014-10-01 13:20:32 +0000
3+++ qml/Stages/SessionContainer.qml 2014-11-12 01:10:20 +0000
4@@ -69,6 +69,10 @@
5 target: item; when: item
6 property: "orientation"; value: root.orientation
7 }
8+
9+ Component.onDestruction: {
10+ root.session.surface.forceActiveFocus();
11+ }
12 }
13 }
14
15
16=== modified file 'tests/mocks/Unity/Application/Session.h'
17--- tests/mocks/Unity/Application/Session.h 2014-09-01 16:07:55 +0000
18+++ tests/mocks/Unity/Application/Session.h 2014-11-12 01:10:20 +0000
19@@ -56,7 +56,7 @@
20
21 Q_INVOKABLE void addChildSession(Session* session);
22 void insertChildSession(uint index, Session* session);
23- void removeChildSession(Session* session);
24+ Q_INVOKABLE void removeChildSession(Session* session);
25
26 Q_SIGNALS:
27 void applicationChanged(ApplicationInfo*);
28
29=== modified file 'tests/qmltests/Stages/tst_SessionContainer.qml'
30--- tests/qmltests/Stages/tst_SessionContainer.qml 2014-09-12 18:09:44 +0000
31+++ tests/qmltests/Stages/tst_SessionContainer.qml 2014-11-12 01:10:20 +0000
32@@ -185,6 +185,41 @@
33 tryCompare(sessionSpy, "count", data.count);
34 }
35
36+ function test_childSessionDestructionReturnsFocusToSiblingOrParent() {
37+ sessionCheckbox.checked = true;
38+ var sessionContainer = sessionContainerLoader.item;
39+ compare(sessionContainer.childSessions.count(), 0);
40+
41+ sessionContainer.interactive = true;
42+
43+ var i;
44+ var sessions = [];
45+ // 3 sessions should cover all edge cases
46+ for(i = 0; i < 3; i++) {
47+ var a_session = ApplicationTest.addChildSession(
48+ sessionContainer.session, "gallery"
49+ )
50+
51+ a_session.createSurface();
52+ sessionContainer.session.addChildSession(a_session);
53+ compare(sessionContainer.childSessions.count(), i + 1);
54+
55+ sessions.push(a_session);
56+ }
57+
58+ var a_session;
59+ while(a_session = sessions.pop()) {
60+ a_session.surface.forceActiveFocus();
61+ compare(a_session.surface.activeFocus, true);
62+
63+ var parentSession = a_session.parentSession;
64+ sessionContainer.session.removeChildSession(a_session);
65+ compare(a_session.surface.activeFocus, false);
66+
67+ compare(parentSession.surface.activeFocus, true);
68+ }
69+ }
70+
71 function test_nestedChildSessions_data() {
72 return [ { tag: "depth=2", depth: 2 },
73 { tag: "depth=8", depth: 8 }

Subscribers

People subscribed via source and target branches