Unity 8

Merge lp:~mzanetti/unity8/catch-osk into lp:unity8

catch-osk
Merge into trunk

Proposed by Michael Zanetti on 2016-02-15

Status:

Merged

Approved by:

Michał Sawicz on 2016-03-09

Approved revision:

2189

Merged at revision:

2245

Proposed branch:

lp:~mzanetti/unity8/catch-osk

Merge into:

lp:unity8

Diff against target:

198 lines (+25/-30)

10 files modified

qml/Components/InputMethod.qml (+1/-9)
tests/mocks/Unity/Application/MirSurface.cpp (+3/-2)
tests/mocks/Unity/Application/MirSurface.h (+2/-1)
tests/mocks/Unity/Application/SurfaceManager.cpp (+8/-9)
tests/mocks/Unity/Application/SurfaceManager.h (+3/-3)
tests/mocks/Unity/Application/VirtualKeyboard.cpp (+3/-2)
tests/mocks/Unity/Application/VirtualKeyboard.h (+1/-1)
tests/mocks/Unity/Application/plugin.cpp (+2/-1)
tests/qmltests/tst_OrientedShell.qml (+1/-1)
tests/qmltests/tst_Shell.qml (+1/-1)

To merge this branch:

bzr merge lp:~mzanetti/unity8/catch-osk

Critical

In Progress

Link a bug report

Reviewer	Review Type	Date Requested	Status
Michał Sawicz	ci		Approve on 2016-03-09
Unity8 CI Bot	continuous-integration		Approve on 2016-03-03
Lukáš Tinkl (community)			Approve on 2016-02-17
Daniel d'Andrada (community)		2016-02-15	Approve on 2016-02-17
PS Jenkins bot	continuous-integration		Needs Fixing on 2016-02-16
Review via email: mp+286076@code.launchpad.net

Commit Message

Read inputMethod surface from the new property int QtMir

The previous signal had issues if the InputMethod in unity was
created after the surfaceAddedSignal() was emitted.

Description of the Change

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

https://code.launchpad.net/~mzanetti/qtmir/surfacemanager-getters/+merge/286074

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

yes

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

yes

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

n/a

* If you changed the UI, has there been a design review?

n/a

Daniel d'Andrada (dandrader) wrote on 2016-02-15:

What about adding a simple property to SurfaceManager that contains the osk surface?

I mean, the general idea is that qtmir should provide us with a QML API that would simply unity8's life as much as possible.

review: Needs Information

Daniel d'Andrada (dandrader) wrote on 2016-02-15:

s/simply/simplify

Lukáš Tinkl (lukas-kde) wrote on 2016-02-15:

12 + surfaceItem.surface = surface;
13 + return;

This should probably "break", not "return", in case we might add some stuff below the loop in the future.

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-02-15:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/355/rebuild

review: Needs Fixing (continuous-integration)

Michael Zanetti (mzanetti) wrote on 2016-02-15:

> What about adding a simple property to SurfaceManager that contains the osk
> surface?
>
> I mean, the general idea is that qtmir should provide us with a QML API that
> would simply unity8's life as much as possible.

done

PS Jenkins bot (ps-jenkins) wrote on 2016-02-15:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/7293/rebuild

review: Needs Fixing (continuous-integration)

Daniel d'Andrada (dandrader) wrote on 2016-02-15:

Please update the mock SurfaceManager.

review: Needs Fixing

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-02-15:

FAILED: Continuous integration, rev:2183
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/359/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/491/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/514
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/532
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/532
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/528/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/528/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/528/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/528/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/528/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/528/console

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/359/rebuild

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-15:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/7297/rebuild

review: Needs Fixing (continuous-integration)

Daniel d'Andrada (dandrader) wrote on 2016-02-16:

There's a conflict with the new trunk:

"""
Text conflict in tests/mocks/Unity/Application/SurfaceManager.h
1 conflicts encountered.
"""

review: Needs Fixing

Daniel d'Andrada (dandrader) wrote on 2016-02-16:

And please, in the SurfaceManager mock, also set m_virtualKeyboard back to nullptr when it's destroyed.

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-02-16:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/370/rebuild

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-16:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/7306/rebuild

review: Needs Fixing (continuous-integration)

Daniel d'Andrada (dandrader) wrote on 2016-02-17:

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

* Did CI run pass? If not, please explain why.
Mostly yeah. There's one failure I suspect will be fixed by lp:~dandrader/unity8/ownMockMirSurface

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

review: Approve

Lukáš Tinkl (lukas-kde) wrote on 2016-02-17:

It fixes the issue with missing OSK in the OOBE wizard too, yay

review: Approve

Michał Sawicz (saviq) wrote on 2016-02-29:

Unfortunately the test still is not stable:

FAIL! : qmltestrunner::Shell::test_showInputMethod() function returned unexpected result
   Actual (): false
   Expected (): true
   Loc: [/tmp/adt-run.dd2sFU/build.VbD/unity8-8.11+15.04.20160229/tests/qmltests/tst_Shell.qml(788)]

This is from a failed adt-run in britney:
https://objectstorage.prodstack4-5.canonical.com/v1/AUTH_77e2ada1e7a84929a74ba3b87153c0ac/autopkgtest-vivid-ci-train-ppa-service-landing-064/vivid/amd64/u/unity8/20160229_200055@/log.gz

review: Needs Fixing

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-01:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/540/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-01:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/543/rebuild

review: Needs Fixing (continuous-integration)

lp:~mzanetti/unity8/catch-osk updated on 2016-03-02

2187. By Michael Zanetti on 2016-03-01: property parent the OSK surface in mocks
2188. By Michael Zanetti on 2016-03-02: register MirSurfaceInterface as a qml datatype

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-02:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/558/rebuild

review: Needs Fixing (continuous-integration)

lp:~mzanetti/unity8/catch-osk updated on 2016-03-02

2189. By Michael Zanetti on 2016-03-02: don't do it properly... too late for that :(

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-03:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/567/rebuild

review: Approve (continuous-integration)

Michał Sawicz (saviq) on 2016-03-09:

review: Approve (ci)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Michael Zanetti

Mohammad Rakibul Hasan

Unity Team

1	=== modified file 'qml/Components/InputMethod.qml'
2	--- qml/Components/InputMethod.qml 2015-12-02 10:34:31 +0000
3	+++ qml/Components/InputMethod.qml 2016-03-02 23:42:46 +0000
4	@@ -22,15 +22,6 @@
5	Item {
6	id: root
7
8	- Connections {
9	- target: SurfaceManager
10	- onSurfaceCreated: {
11	- if (surface.type == Mir.InputMethodType) {
12	- surfaceItem.surface = surface;
13	- }
14	- }
15	- }
16	-
17	property int transitionDuration: UbuntuAnimation.FastDuration
18
19	MirSurfaceItem {
20	@@ -41,6 +32,7 @@
21
22	surfaceWidth: width
23	surfaceHeight: height
24	+ surface: SurfaceManager.inputMethodSurface
25
26	onLiveChanged: {
27	if (surface !== null && !live) {
28
29	=== modified file 'tests/mocks/Unity/Application/MirSurface.cpp'
30	--- tests/mocks/Unity/Application/MirSurface.cpp 2015-11-30 18:25:47 +0000
31	+++ tests/mocks/Unity/Application/MirSurface.cpp 2016-03-02 23:42:46 +0000
32	@@ -22,8 +22,9 @@
33	Mir::Type type,
34	Mir::State state,
35	const QUrl& screenshot,
36	- const QUrl &qmlFilePath)
37	- : unity::shell::application::MirSurfaceInterface(nullptr)
38	+ const QUrl &qmlFilePath,
39	+ QObject *parent)
40	+ : unity::shell::application::MirSurfaceInterface(parent)
41	, m_name(name)
42	, m_type(type)
43	, m_state(state)
44
45	=== modified file 'tests/mocks/Unity/Application/MirSurface.h'
46	--- tests/mocks/Unity/Application/MirSurface.h 2015-11-30 18:25:47 +0000
47	+++ tests/mocks/Unity/Application/MirSurface.h 2016-03-02 23:42:46 +0000
48	@@ -41,7 +41,8 @@
49	Mir::Type type,
50	Mir::State state,
51	const QUrl& screenshot,
52	- const QUrl &qmlFilePath = QUrl());
53	+ const QUrl &qmlFilePath = QUrl(),
54	+ QObject *parent = nullptr);
55	virtual ~MirSurface();
56
57	////
58
59	=== modified file 'tests/mocks/Unity/Application/SurfaceManager.cpp'
60	--- tests/mocks/Unity/Application/SurfaceManager.cpp 2015-11-30 18:25:47 +0000
61	+++ tests/mocks/Unity/Application/SurfaceManager.cpp 2016-03-02 23:42:46 +0000
62	@@ -34,6 +34,13 @@
63	QObject(parent)
64	, m_virtualKeyboard(nullptr)
65	{
66	+ m_virtualKeyboard = new VirtualKeyboard(this);
67	+ connect(m_virtualKeyboard, &QObject::destroyed, this, [this](QObject *obj) {
68	+ MirSurface* surface = qobject_cast<MirSurface*>(obj);
69	+ m_virtualKeyboard = nullptr;
70	+ Q_EMIT inputMethodSurfaceChanged();
71	+ Q_EMIT surfaceDestroyed(surface);
72	+ });
73	}
74
75	MirSurface *SurfaceManager::createSurface(const QString& name,
76	@@ -58,16 +65,8 @@
77	return surface;
78	}
79
80	-MirSurface *SurfaceManager::inputMethodSurface()
81	+MirSurface *SurfaceManager::inputMethodSurface() const
82	{
83	- if (!m_virtualKeyboard) {
84	- m_virtualKeyboard = new VirtualKeyboard;
85	- connect(m_virtualKeyboard, &QObject::destroyed, this, [this](QObject *obj) {
86	- MirSurface* surface = qobject_cast<MirSurface*>(obj);
87	- Q_EMIT surfaceDestroyed(surface);
88	- });
89	- Q_EMIT surfaceCreated(m_virtualKeyboard);
90	- }
91	return m_virtualKeyboard;
92	}
93
94
95	=== modified file 'tests/mocks/Unity/Application/SurfaceManager.h'
96	--- tests/mocks/Unity/Application/SurfaceManager.h 2015-11-30 18:25:47 +0000
97	+++ tests/mocks/Unity/Application/SurfaceManager.h 2016-03-02 23:42:46 +0000
98	@@ -25,7 +25,7 @@
99	class SurfaceManager : public QObject
100	{
101	Q_OBJECT
102	-
103	+ Q_PROPERTY(MirSurface* inputMethodSurface READ inputMethodSurface NOTIFY inputMethodSurfaceChanged)
104	Q_PROPERTY(int newSurfaceMinimumWidth READ newSurfaceMinimumWidth WRITE setNewSurfaceMinimumWidth NOTIFY newSurfaceMinimumWidthChanged)
105	Q_PROPERTY(int newSurfaceMaximumWidth READ newSurfaceMaximumWidth WRITE setNewSurfaceMaximumWidth NOTIFY newSurfaceMaximumWidthChanged)
106	Q_PROPERTY(int newSurfaceMinimumHeight READ newSurfaceMinimumHeight WRITE setNewSurfaceMinimumHeight NOTIFY newSurfaceMinimumHeightChanged)
107	@@ -43,8 +43,7 @@
108	Mir::State state,
109	const QUrl& screenshot);
110
111	- // To be used in the tests
112	- Q_INVOKABLE MirSurface* inputMethodSurface();
113	+ MirSurface* inputMethodSurface() const;
114
115	int newSurfaceMinimumWidth() const { return m_newSurfaceMinimumWidth; }
116	void setNewSurfaceMinimumWidth(int value);
117	@@ -65,6 +64,7 @@
118	void setNewSurfaceHeightIncrement(int);
119
120	Q_SIGNALS:
121	+ void inputMethodSurfaceChanged();
122	void countChanged();
123	void surfaceCreated(MirSurface *surface);
124	void surfaceDestroyed(MirSurface*surface);
125
126	=== modified file 'tests/mocks/Unity/Application/VirtualKeyboard.cpp'
127	--- tests/mocks/Unity/Application/VirtualKeyboard.cpp 2015-09-02 10:35:16 +0000
128	+++ tests/mocks/Unity/Application/VirtualKeyboard.cpp 2016-03-02 23:42:46 +0000
129	@@ -22,12 +22,13 @@
130
131	#include <QDebug>
132
133	-VirtualKeyboard::VirtualKeyboard()
134	+VirtualKeyboard::VirtualKeyboard(QObject *parent)
135	: MirSurface("input-method",
136	Mir::InputMethodType,
137	Mir::MinimizedState,
138	QUrl("qrc:///Unity/Application/vkb_portrait.png"),
139	- QUrl("qrc:///Unity/Application/VirtualKeyboard.qml"))
140	+ QUrl("qrc:///Unity/Application/VirtualKeyboard.qml"),
141	+ parent)
142	{
143	}
144
145
146	=== modified file 'tests/mocks/Unity/Application/VirtualKeyboard.h'
147	--- tests/mocks/Unity/Application/VirtualKeyboard.h 2015-08-19 20:30:09 +0000
148	+++ tests/mocks/Unity/Application/VirtualKeyboard.h 2016-03-02 23:42:46 +0000
149	@@ -25,7 +25,7 @@
150	{
151	Q_OBJECT
152	public:
153	- explicit VirtualKeyboard();
154	+ explicit VirtualKeyboard(QObject *parent = nullptr);
155	~VirtualKeyboard();
156	};
157
158
159	=== modified file 'tests/mocks/Unity/Application/plugin.cpp'
160	--- tests/mocks/Unity/Application/plugin.cpp 2015-08-03 15:00:47 +0000
161	+++ tests/mocks/Unity/Application/plugin.cpp 2016-03-02 23:42:46 +0000
162	@@ -78,8 +78,9 @@
163	qmlRegisterUncreatableType<unity::shell::application::ApplicationInfoInterface>(uri, 0, 1, "ApplicationInfoInterface", "Abstract interface. Cannot be created in QML");
164	qmlRegisterUncreatableType<Session>(uri, 0, 1, "Session", "Session can't be instantiated from QML");
165	qmlRegisterUncreatableType<MirSurface>(uri, 0, 1, "MirSurface", "MirSurface can't be instantiated from QML");
166	+ qmlRegisterUncreatableType<unity::shell::application::MirSurfaceInterface>(
167	+ uri, 0, 1, "MirSurface", "MirSurface can't be instantiated from QML");
168	qmlRegisterType<MirSurfaceItem>(uri, 0, 1, "MirSurfaceItem");
169	-
170	qmlRegisterType<ApplicationInfo>(uri, 0, 1, "ApplicationInfo");
171
172	qmlRegisterSingletonType<ApplicationManager>(uri, 0, 1, "ApplicationManager", applicationManagerSingleton);
173
174	=== modified file 'tests/qmltests/tst_OrientedShell.qml'
175	--- tests/qmltests/tst_OrientedShell.qml 2016-01-28 11:31:48 +0000
176	+++ tests/qmltests/tst_OrientedShell.qml 2016-03-02 23:42:46 +0000
177	@@ -314,7 +314,7 @@
178	checked: false
179	activeFocusOnPress: false
180	onCheckedChanged: {
181	- var surface = SurfaceManager.inputMethodSurface();
182	+ var surface = SurfaceManager.inputMethodSurface;
183	if (checked) {
184	surface.setState(Mir.RestoredState);
185	} else {
186
187	=== modified file 'tests/qmltests/tst_Shell.qml'
188	--- tests/qmltests/tst_Shell.qml 2016-02-12 00:11:28 +0000
189	+++ tests/qmltests/tst_Shell.qml 2016-03-02 23:42:46 +0000
190	@@ -773,7 +773,7 @@
191	loadShell("phone");
192	swipeAwayGreeter();
193	var item = findChild(shell, "inputMethod");
194	- var surface = SurfaceManager.inputMethodSurface();
195	+ var surface = SurfaceManager.inputMethodSurface;
196
197	surface.setState(Mir.MinimizedState);
198	tryCompare(item, "visible", false);