webbrowser-app

Merge lp:~osomon/webbrowser-app/no-loadingChanged into lp:webbrowser-app

  1. no-loadingChanged
  2. Merge into trunk
Proposed by Olivier Tilloy
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
Reviewer Review Type Date Requested Status
Alexandre Abreu (community) Approve
PS Jenkins bot continuous-integration Needs Fixing
Review via email: mp+283997@code.launchpad.net

Commit message

Do not connect to the loadingChanged signal, which has been deprecated for a good while.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1336
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2579/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/6208
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1332
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1332
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1332/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1332
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-touch/4786
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6219
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6219/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/27055

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2579/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Chris Coulson (chrisccoulson) wrote :

I've added a comment to this

lp:~osomon/webbrowser-app/no-loadingChanged updated
1337. By Olivier Tilloy

Use the loadingStateChanged signal.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1337
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2582/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/6213/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1335
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1335
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1335/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1335
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-touch/4791/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6224
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6224/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/27067

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2582/rebuild

review: Needs Fixing (continuous-integration)
lp:~osomon/webbrowser-app/no-loadingChanged updated
1338. By Olivier Tilloy

Add unit tests for ChromeController.

Revision history for this message
Olivier Tilloy (osomon) wrote :

Addressed, and added unit tests for ChromeController.
Thanks for the review!

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1338
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2583/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/6217
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1336
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1336
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1336/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1336
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-touch/4793
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6228
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6228/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/27071

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2583/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Alexandre Abreu (abreu-alexandre) wrote :

one small comment, but otherwise +1

review: Approve
lp:~osomon/webbrowser-app/no-loadingChanged updated
1339. By Olivier Tilloy

Cosmetics.

Revision history for this message
Olivier Tilloy (osomon) wrote :

Thanks, I applied your suggestion.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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+}

Subscribers

People subscribed via source and target branches

to status/vote changes: