Merge lp:~osomon/webbrowser-app/no-loadingChanged into lp:webbrowser-app
- no-loadingChanged
- Merge into trunk
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Olivier Tilloy | ||||
Approved revision: | 1339 | ||||
Merged at revision: | 1338 | ||||
Proposed branch: | lp:~osomon/webbrowser-app/no-loadingChanged | ||||
Merge into: | lp:webbrowser-app | ||||
Diff against target: |
386 lines (+316/-13) 2 files modified
src/app/ChromeController.qml (+24/-13) tests/unittests/qml/tst_ChromeController.qml (+292/-0) |
||||
To merge this branch: | bzr merge lp:~osomon/webbrowser-app/no-loadingChanged | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Alexandre Abreu (community) | Approve | ||
PS Jenkins bot | continuous-integration | Needs Fixing | |
Review via email:
|
Commit message
Do not connect to the loadingChanged signal, which has been deprecated for a good while.
Description of the change
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Chris Coulson (chrisccoulson) wrote : | # |
I've added a comment to this
- 1337. By Olivier Tilloy
-
Use the loadingStateChanged signal.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1337
http://
Executed test runs:
FAILURE: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 1338. By Olivier Tilloy
-
Add unit tests for ChromeController.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Olivier Tilloy (osomon) wrote : | # |
Addressed, and added unit tests for ChromeController.
Thanks for the review!
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1338
http://
Executed test runs:
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
UNSTABLE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Alexandre Abreu (abreu-alexandre) wrote : | # |
one small comment, but otherwise +1
- 1339. By Olivier Tilloy
-
Cosmetics.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Olivier Tilloy (osomon) wrote : | # |
Thanks, I applied your suggestion.
Preview Diff
1 | === modified file 'src/app/ChromeController.qml' |
2 | --- src/app/ChromeController.qml 2015-11-04 16:47:37 +0000 |
3 | +++ src/app/ChromeController.qml 2016-01-28 07:59:06 +0000 |
4 | @@ -1,5 +1,5 @@ |
5 | /* |
6 | - * Copyright 2015 Canonical Ltd. |
7 | + * Copyright 2015-2016 Canonical Ltd. |
8 | * |
9 | * This file is part of webbrowser-app. |
10 | * |
11 | @@ -25,7 +25,7 @@ |
12 | property var webview |
13 | property bool forceHide: false |
14 | property bool forceShow: false |
15 | - property int defaultMode: Oxide.LocationBarController.ModeAuto |
16 | + property int defaultMode: internal.modeAuto |
17 | |
18 | onWebviewChanged: internal.updateVisibility() |
19 | onForceHideChanged: internal.updateVisibility() |
20 | @@ -34,18 +34,22 @@ |
21 | QtObject { |
22 | id: internal |
23 | |
24 | + readonly property int modeAuto: Oxide.LocationBarController.ModeAuto |
25 | + readonly property int modeShown: Oxide.LocationBarController.ModeShown |
26 | + readonly property int modeHidden: Oxide.LocationBarController.ModeHidden |
27 | + |
28 | function updateVisibility() { |
29 | if (!webview) { |
30 | return |
31 | } |
32 | webview.locationBarController.animated = false |
33 | if (forceHide) { |
34 | - webview.locationBarController.mode = Oxide.LocationBarController.ModeHidden |
35 | + webview.locationBarController.mode = internal.modeHidden |
36 | } else if (forceShow) { |
37 | - webview.locationBarController.mode = Oxide.LocationBarController.ModeShown |
38 | + webview.locationBarController.mode = internal.modeShown |
39 | } else if (!webview.fullscreen) { |
40 | webview.locationBarController.mode = defaultMode |
41 | - if (webview.locationBarController.mode == Oxide.LocationBarController.ModeAuto) { |
42 | + if (webview.locationBarController.mode == internal.modeAuto) { |
43 | webview.locationBarController.show(false) |
44 | } |
45 | } |
46 | @@ -55,28 +59,35 @@ |
47 | |
48 | Connections { |
49 | target: webview |
50 | + |
51 | onFullscreenChanged: { |
52 | if (webview.fullscreen) { |
53 | - webview.locationBarController.mode = Oxide.LocationBarController.ModeHidden |
54 | - } else if (!forceHide && !forceShow) { |
55 | - webview.locationBarController.mode = defaultMode |
56 | - if (webview.locationBarController.mode == Oxide.LocationBarController.ModeAuto) { |
57 | - webview.locationBarController.show(true) |
58 | + webview.locationBarController.mode = internal.modeHidden |
59 | + } else if (!forceHide) { |
60 | + if (forceShow) { |
61 | + webview.locationBarController.mode = internal.modeShown |
62 | + } else { |
63 | + webview.locationBarController.mode = defaultMode |
64 | + if (webview.locationBarController.mode == internal.modeAuto) { |
65 | + webview.locationBarController.show(true) |
66 | + } |
67 | } |
68 | } |
69 | } |
70 | - onLoadingChanged: { |
71 | + |
72 | + onLoadingStateChanged: { |
73 | if (webview.loading && !webview.fullscreen && !forceHide && !forceShow && |
74 | - (webview.locationBarController.mode == Oxide.LocationBarController.ModeAuto)) { |
75 | + (webview.locationBarController.mode == internal.modeAuto)) { |
76 | webview.locationBarController.show(true) |
77 | } |
78 | } |
79 | + |
80 | onLoadEvent: { |
81 | // When loading, force ModeShown until the load is committed or stopped, |
82 | // to work around https://launchpad.net/bugs/1453908. |
83 | if (forceHide || forceShow) return |
84 | if (event.type == Oxide.LoadEvent.TypeStarted) { |
85 | - webview.locationBarController.mode = Oxide.LocationBarController.ModeShown |
86 | + webview.locationBarController.mode = internal.modeShown |
87 | } else if ((event.type == Oxide.LoadEvent.TypeCommitted) || |
88 | (event.type == Oxide.LoadEvent.TypeStopped)) { |
89 | webview.locationBarController.mode = defaultMode |
90 | |
91 | === added file 'tests/unittests/qml/tst_ChromeController.qml' |
92 | --- tests/unittests/qml/tst_ChromeController.qml 1970-01-01 00:00:00 +0000 |
93 | +++ tests/unittests/qml/tst_ChromeController.qml 2016-01-28 07:59:06 +0000 |
94 | @@ -0,0 +1,292 @@ |
95 | +/* |
96 | + * Copyright 2016 Canonical Ltd. |
97 | + * |
98 | + * This file is part of webbrowser-app. |
99 | + * |
100 | + * webbrowser-app is free software; you can redistribute it and/or modify |
101 | + * it under the terms of the GNU General Public License as published by |
102 | + * the Free Software Foundation; version 3. |
103 | + * |
104 | + * webbrowser-app is distributed in the hope that it will be useful, |
105 | + * but WITHOUT ANY WARRANTY; without even the implied warranty of |
106 | + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
107 | + * GNU General Public License for more details. |
108 | + * |
109 | + * You should have received a copy of the GNU General Public License |
110 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
111 | + */ |
112 | + |
113 | +import QtQuick 2.4 |
114 | +import QtTest 1.0 |
115 | +import com.canonical.Oxide 1.7 as Oxide |
116 | +import "../../../src/app" |
117 | + |
118 | +ChromeController { |
119 | + id: controller |
120 | + |
121 | + Item { |
122 | + id: webviewMock |
123 | + property bool loading: false |
124 | + readonly property bool loadingState: loading |
125 | + property bool fullscreen: false |
126 | + property var locationBarController: QtObject { |
127 | + property bool animated: false |
128 | + property int mode: controller.defaultMode |
129 | + signal show(bool animate) |
130 | + } |
131 | + signal loadEvent(var event) |
132 | + } |
133 | + |
134 | + SignalSpy { |
135 | + id: showSpy |
136 | + target: webviewMock.locationBarController |
137 | + signalName: "show" |
138 | + } |
139 | + |
140 | + webview: webviewMock |
141 | + |
142 | + TestCase { |
143 | + name: "ChromeController" |
144 | + |
145 | + readonly property int modeAuto: Oxide.LocationBarController.ModeAuto |
146 | + readonly property int modeShown: Oxide.LocationBarController.ModeShown |
147 | + readonly property int modeHidden: Oxide.LocationBarController.ModeHidden |
148 | + |
149 | + function init() { |
150 | + controller.forceHide = false |
151 | + controller.forceShow = false |
152 | + controller.defaultMode = modeAuto |
153 | + webviewMock.loading = false |
154 | + webviewMock.fullscreen = false |
155 | + webviewMock.locationBarController.animated = false |
156 | + webviewMock.locationBarController.mode = controller.defaultMode |
157 | + showSpy.clear() |
158 | + } |
159 | + |
160 | + function test_change_webview_data() { |
161 | + return [ |
162 | + {forceHide: false, forceShow: false, fullscreen: false, |
163 | + mode: modeAuto, shown: true}, |
164 | + {forceHide: false, forceShow: true, fullscreen: false, |
165 | + mode: modeShown, shown: false}, |
166 | + {forceHide: false, forceShow: false, fullscreen: true, |
167 | + mode: modeAuto, shown: false}, |
168 | + {forceHide: false, forceShow: true, fullscreen: true, |
169 | + mode: modeShown, shown: false}, |
170 | + {forceHide: true, forceShow: false, fullscreen: true, |
171 | + mode: modeHidden, shown: false}, |
172 | + {forceHide: true, forceShow: true, fullscreen: false, |
173 | + mode: modeHidden, shown: false}, |
174 | + {forceHide: true, forceShow: true, fullscreen: true, |
175 | + mode: modeHidden, shown: false}, |
176 | + ] |
177 | + } |
178 | + |
179 | + function test_change_webview(data) { |
180 | + controller.webview = null |
181 | + controller.forceHide = data.forceHide |
182 | + controller.forceShow = data.forceShow |
183 | + webviewMock.fullscreen = data.fullscreen |
184 | + showSpy.clear() |
185 | + controller.webview = webviewMock |
186 | + compare(webviewMock.locationBarController.mode, data.mode) |
187 | + compare(showSpy.count, data.shown ? 1 : 0) |
188 | + } |
189 | + |
190 | + function test_change_forceHide_data() { |
191 | + return [ |
192 | + {forceShow: false, fullscreen: false, |
193 | + modes: [modeAuto, modeHidden, modeAuto], shown: 1}, |
194 | + {forceShow: true, fullscreen: false, |
195 | + modes: [modeShown, modeHidden, modeShown], shown: 0}, |
196 | + {forceShow: false, fullscreen: true, |
197 | + modes: [modeHidden, modeHidden, modeHidden], shown: 0}, |
198 | + {forceShow: true, fullscreen: true, |
199 | + modes: [modeHidden, modeHidden, modeShown], shown: 0}, |
200 | + ] |
201 | + } |
202 | + |
203 | + function test_change_forceHide(data) { |
204 | + controller.forceShow = data.forceShow |
205 | + webviewMock.fullscreen = data.fullscreen |
206 | + showSpy.clear() |
207 | + controller.forceHide = false |
208 | + compare(webviewMock.locationBarController.mode, data.modes[0]) |
209 | + controller.forceHide = true |
210 | + compare(webviewMock.locationBarController.mode, data.modes[1]) |
211 | + controller.forceHide = false |
212 | + compare(webviewMock.locationBarController.mode, data.modes[2]) |
213 | + compare(showSpy.count, data.shown) |
214 | + } |
215 | + |
216 | + function test_change_forceShow_data() { |
217 | + return [ |
218 | + {forceHide: false, fullscreen: false, |
219 | + modes: [modeAuto, modeShown, modeAuto], shown: 1}, |
220 | + {forceHide: true, fullscreen: false, |
221 | + modes: [modeHidden, modeHidden, modeHidden], shown: 0}, |
222 | + {forceHide: false, fullscreen: true, |
223 | + modes: [modeHidden, modeShown, modeShown], shown: 0}, |
224 | + {forceHide: true, fullscreen: true, |
225 | + modes: [modeHidden, modeHidden, modeHidden], shown: 0}, |
226 | + ] |
227 | + } |
228 | + |
229 | + function test_change_forceShow(data) { |
230 | + controller.forceHide = data.forceHide |
231 | + webviewMock.fullscreen = data.fullscreen |
232 | + showSpy.clear() |
233 | + controller.forceShow = false |
234 | + compare(webviewMock.locationBarController.mode, data.modes[0]) |
235 | + controller.forceShow = true |
236 | + compare(webviewMock.locationBarController.mode, data.modes[1]) |
237 | + controller.forceShow = false |
238 | + compare(webviewMock.locationBarController.mode, data.modes[2]) |
239 | + compare(showSpy.count, data.shown) |
240 | + } |
241 | + |
242 | + function test_change_fullscreen_data() { |
243 | + return [ |
244 | + {forceHide: false, forceShow: false, defaultMode: modeAuto, |
245 | + mode: modeAuto, shown: true}, |
246 | + {forceHide: false, forceShow: false, defaultMode: modeShown, |
247 | + mode: modeShown, shown: false}, |
248 | + {forceHide: false, forceShow: false, defaultMode: modeHidden, |
249 | + mode: modeHidden, shown: false}, |
250 | + {forceHide: true, forceShow: false, defaultMode: modeAuto, |
251 | + mode: modeHidden, shown: false}, |
252 | + {forceHide: true, forceShow: false, defaultMode: modeShown, |
253 | + mode: modeHidden, shown: false}, |
254 | + {forceHide: true, forceShow: false, defaultMode: modeHidden, |
255 | + mode: modeHidden, shown: false}, |
256 | + {forceHide: false, forceShow: true, defaultMode: modeAuto, |
257 | + mode: modeShown, shown: false}, |
258 | + {forceHide: false, forceShow: true, defaultMode: modeShown, |
259 | + mode: modeShown, shown: false}, |
260 | + {forceHide: false, forceShow: true, defaultMode: modeHidden, |
261 | + mode: modeShown, shown: false}, |
262 | + {forceHide: true, forceShow: true, defaultMode: modeAuto, |
263 | + mode: modeHidden, shown: false}, |
264 | + {forceHide: true, forceShow: true, defaultMode: modeShown, |
265 | + mode: modeHidden, shown: false}, |
266 | + {forceHide: true, forceShow: true, defaultMode: modeHidden, |
267 | + mode: modeHidden, shown: false}, |
268 | + ] |
269 | + } |
270 | + |
271 | + function test_change_fullscreen(data) { |
272 | + webviewMock.fullscreen = false |
273 | + controller.forceHide = data.forceHide |
274 | + controller.forceShow = data.forceShow |
275 | + controller.defaultMode = data.defaultMode |
276 | + showSpy.clear() |
277 | + webviewMock.fullscreen = true |
278 | + compare(webviewMock.locationBarController.mode, modeHidden) |
279 | + compare(showSpy.count, 0) |
280 | + webviewMock.fullscreen = false |
281 | + compare(webviewMock.locationBarController.mode, data.mode) |
282 | + compare(showSpy.count, data.shown ? 1 : 0) |
283 | + } |
284 | + |
285 | + function test_loading_state_changed_data() { |
286 | + return [ |
287 | + {forceHide: false, forceShow: false, fullscreen: false, |
288 | + mode: modeAuto, shown: true}, |
289 | + {forceHide: false, forceShow: false, fullscreen: false, |
290 | + mode: modeShown, shown: false}, |
291 | + {forceHide: false, forceShow: false, fullscreen: false, |
292 | + mode: modeHidden, shown: false}, |
293 | + {forceHide: true, forceShow: false, fullscreen: false, |
294 | + mode: modeHidden, shown: false}, |
295 | + {forceHide: false, forceShow: true, fullscreen: false, |
296 | + mode: modeShown, shown: false}, |
297 | + {forceHide: false, forceShow: false, fullscreen: true, |
298 | + mode: modeHidden, shown: false}, |
299 | + {forceHide: true, forceShow: true, fullscreen: false, |
300 | + mode: modeHidden, shown: false}, |
301 | + {forceHide: true, forceShow: false, fullscreen: true, |
302 | + mode: modeHidden, shown: false}, |
303 | + {forceHide: false, forceShow: true, fullscreen: true, |
304 | + mode: modeShown, shown: false}, |
305 | + {forceHide: true, forceShow: true, fullscreen: true, |
306 | + mode: modeHidden, shown: false}, |
307 | + ] |
308 | + } |
309 | + |
310 | + function test_loading_state_changed(data) { |
311 | + controller.forceHide = data.forceHide |
312 | + controller.forceShow = data.forceShow |
313 | + webviewMock.fullscreen = data.fullscreen |
314 | + webviewMock.locationBarController.mode = data.mode |
315 | + showSpy.clear() |
316 | + webviewMock.loading = true |
317 | + compare(showSpy.count, data.shown ? 1 : 0) |
318 | + compare(webviewMock.locationBarController.mode, data.mode) |
319 | + showSpy.clear() |
320 | + webviewMock.loading = false |
321 | + compare(showSpy.count, 0) |
322 | + compare(webviewMock.locationBarController.mode, data.mode) |
323 | + } |
324 | + |
325 | + function test_load_event_data() { |
326 | + var data = [] |
327 | + var booleanValues = [false, true] |
328 | + var modeValues = [modeAuto, modeHidden, modeShown] |
329 | + for (var i in booleanValues) { |
330 | + for (var j in booleanValues) { |
331 | + for (var k in modeValues) { |
332 | + for (var l in modeValues) { |
333 | + data.push({forceHide: booleanValues[i], forceShow: booleanValues[j], |
334 | + initialMode: modeValues[k], defaultMode: modeValues[l]}) |
335 | + } |
336 | + } |
337 | + } |
338 | + } |
339 | + return data |
340 | + } |
341 | + |
342 | + function test_load_event(data) { |
343 | + // event types |
344 | + var started = Oxide.LoadEvent.TypeStarted |
345 | + var committed = Oxide.LoadEvent.TypeCommitted |
346 | + var succeeded = Oxide.LoadEvent.TypeSucceeded |
347 | + var stopped = Oxide.LoadEvent.TypeStopped |
348 | + var failed = Oxide.LoadEvent.TypeFailed |
349 | + var redirected = Oxide.LoadEvent.TypeRedirected |
350 | + |
351 | + controller.forceHide = data.forceHide |
352 | + controller.forceShow = data.forceShow |
353 | + controller.defaultMode = data.defaultMode |
354 | + webviewMock.locationBarController.mode = data.initialMode |
355 | + showSpy.clear() |
356 | + |
357 | + function test_sequence(sequence, modes) { |
358 | + for (var i in sequence) { |
359 | + webviewMock.loadEvent({type: sequence[i]}) |
360 | + if (data.forceHide || data.forceShow) { |
361 | + compare(webviewMock.locationBarController.mode, data.initialMode) |
362 | + } else { |
363 | + compare(webviewMock.locationBarController.mode, modes[i]) |
364 | + } |
365 | + compare(showSpy.count, 0) |
366 | + } |
367 | + } |
368 | + |
369 | + var sequence = [started, committed, succeeded] |
370 | + var modes = [modeShown, data.defaultMode, data.defaultMode] |
371 | + test_sequence(sequence, modes) |
372 | + |
373 | + sequence = [started, stopped] |
374 | + modes = [modeShown, data.defaultMode] |
375 | + test_sequence(sequence, modes) |
376 | + |
377 | + sequence = [started, failed, committed] |
378 | + modes = [modeShown, modeShown, data.defaultMode] |
379 | + test_sequence(sequence, modes) |
380 | + |
381 | + sequence = [started, redirected, committed, succeeded] |
382 | + modes = [modeShown, modeShown, data.defaultMode, data.defaultMode] |
383 | + test_sequence(sequence, modes) |
384 | + } |
385 | + } |
386 | +} |
FAILED: Continuous integration, rev:1336 jenkins. qa.ubuntu. com/job/ webbrowser- app-ci/ 2579/ jenkins. qa.ubuntu. com/job/ generic- deb-autopilot- vivid-touch/ 6208 jenkins. qa.ubuntu. com/job/ webbrowser- app-vivid- amd64-ci/ 1332 jenkins. qa.ubuntu. com/job/ webbrowser- app-vivid- armhf-ci/ 1332 jenkins. qa.ubuntu. com/job/ webbrowser- app-vivid- armhf-ci/ 1332/artifact/ work/output/ *zip*/output. zip jenkins. qa.ubuntu. com/job/ webbrowser- app-vivid- i386-ci/ 1332 jenkins. qa.ubuntu. com/job/ generic- deb-autopilot- runner- vivid-touch/ 4786 jenkins. qa.ubuntu. com/job/ generic- mediumtests- builder- vivid-armhf/ 6219 jenkins. qa.ubuntu. com/job/ generic- mediumtests- builder- vivid-armhf/ 6219/artifact/ work/output/ *zip*/output. zip s-jenkins. ubuntu- ci:8080/ job/touch- flash-device/ 27055
http://
Executed test runs:
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
UNSTABLE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/webbrowser- app-ci/ 2579/rebuild
http://