Merge lp:~mzanetti/unity8/catch-osk into lp:unity8
- catch-osk
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Michał Sawicz |
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 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Michał Sawicz | ci | Approve | |
Unity8 CI Bot | continuous-integration | Approve | |
Lukáš Tinkl (community) | Approve | ||
Daniel d'Andrada (community) | Approve | ||
PS Jenkins bot (community) | continuous-integration | Needs Fixing | |
Review via email:
|
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 surfaceAddedSig
Description of the change
* Are there any related MPs required for this MP to build/function as expected? Please list.
https:/
* 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
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Daniel d'Andrada (dandrader) wrote : | # |
s/simply/simplify
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Lukáš Tinkl (lukas-kde) wrote : | # |
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.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2182
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michael Zanetti (mzanetti) wrote : | # |
> 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
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:2182
http://
Executed test runs:
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Daniel d'Andrada (dandrader) wrote : | # |
Please update the mock SurfaceManager.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2183
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:2183
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
SUCCESS: http://
FAILURE: http://
FAILURE: http://
SUCCESS: http://
SUCCESS: http://
FAILURE: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Daniel d'Andrada (dandrader) wrote : | # |
There's a conflict with the new trunk:
"""
Text conflict in tests/mocks/
1 conflicts encountered.
"""
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Daniel d'Andrada (dandrader) wrote : | # |
And please, in the SurfaceManager mock, also set m_virtualKeyboard back to nullptr when it's destroyed.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2185
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:2185
http://
Executed test runs:
UNSTABLE: http://
FAILURE: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
UNSTABLE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Daniel d'Andrada (dandrader) wrote : | # |
* 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
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Lukáš Tinkl (lukas-kde) wrote : | # |
It fixes the issue with missing OSK in the OOBE wizard too, yay
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michał Sawicz (saviq) wrote : | # |
Unfortunately the test still is not stable:
FAIL! : qmltestrunner:
Actual (): false
Expected (): true
Loc: [/tmp/adt-
This is from a failed adt-run in britney:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2186
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2186
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
- 2187. By Michael Zanetti
-
property parent the OSK surface in mocks
- 2188. By Michael Zanetti
-
register MirSurfaceInterface as a qml datatype
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2188
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
- 2189. By Michael Zanetti
-
don't do it properly... too late for that :(
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:2189
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michał Sawicz (saviq) : | # |
Preview Diff
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); |
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.