webbrowser-app

Merge lp:~osomon/webbrowser-app/locationBarController-show-hide into lp:webbrowser-app

  1. locationBarController-show-hide
  2. Merge into trunk
Proposed by Olivier Tilloy
Status: Merged
Approved by: Olivier Tilloy
Approved revision: 973
Merged at revision: 1007
Proposed branch: lp:~osomon/webbrowser-app/locationBarController-show-hide
Merge into: lp:webbrowser-app
Diff against target: 162 lines (+51/-46)
4 files modified
debian/control (+1/-1)
src/app/ChromeController.qml (+39/-30)
src/app/webbrowser/Browser.qml (+9/-7)
src/app/webcontainer/WebApp.qml (+2/-8)
To merge this branch: bzr merge lp:~osomon/webbrowser-app/locationBarController-show-hide
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Needs Fixing
Alexandre Abreu (community) Approve
Review via email: mp+256679@code.launchpad.net

Commit message

Use the new Oxide APIs to better control visibility of the chrome.
This bumps the runtime dependency on liboxideqt-qmlplugin to 1.7.

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

FAILED: Continuous integration, rev:968
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1668/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2319/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/425
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/425
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/425/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/425
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/2034/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2317
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2317/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19761

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:968
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1686/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2439/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/443
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/443
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/443/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/443
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/2121/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2437
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2437/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19908

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:968
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1687/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2441/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/444/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/444
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/444/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/444
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/2122/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2439
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2439/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19910

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:968
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1688/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2445/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/445
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/445
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/445/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/445
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/2124/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2443
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2443/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19919

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

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

A few comments inline

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

Answered inline.

Related to visibility/behavior of the chrome, I just filed bug #1453908. I don’t think its resolution should block merging this in (as the issue already exists with webbrowser-app trunk).

Revision history for this message
Alexandre Abreu (abreu-alexandre) :
review: Approve
lp:~osomon/webbrowser-app/locationBarController-show-hide updated
969. By Olivier Tilloy

One ChromeController for all webviews is enough.

970. By Olivier Tilloy

Default to ModeShown on desktop, where we don’t want the chrome to auto-hide when scrolling anyway.

971. By Olivier Tilloy

Work around bug #1453908 by forcing ModeShown while loading until the load is committed or stopped.

972. By Olivier Tilloy

Merge the latest changes from trunk.

973. By Olivier Tilloy

Also set the default mode in the webapp container.

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

Updated with further changes, would you mind giving it another round of review?

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

FAILED: Continuous integration, rev:973
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1741/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2718/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/498
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/498
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/498/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/498
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/2346/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2716
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2716/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/20375

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

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/control'
2--- debian/control 2015-05-06 21:50:57 +0000
3+++ debian/control 2015-05-11 19:26:41 +0000
4@@ -33,7 +33,7 @@
5 Depends: ${misc:Depends},
6 ${shlibs:Depends},
7 fonts-liberation,
8- liboxideqt-qmlplugin (>= 1.5),
9+ liboxideqt-qmlplugin (>= 1.7),
10 libqt5sql5-sqlite,
11 qml-module-qt-labs-folderlistmodel,
12 qml-module-qt-labs-settings,
13
14=== modified file 'src/app/ChromeController.qml'
15--- src/app/ChromeController.qml 2015-03-30 18:29:58 +0000
16+++ src/app/ChromeController.qml 2015-05-11 19:26:41 +0000
17@@ -17,48 +17,57 @@
18 */
19
20 import QtQuick 2.0
21-import com.canonical.Oxide 1.5 as Oxide
22+import com.canonical.Oxide 1.7 as Oxide
23
24 Item {
25 visible: false
26
27 property var webview
28 property bool forceHide: false
29-
30- readonly property int mode: {
31- if (forceHide || webview.fullscreen) {
32- return Oxide.LocationBarController.ModeHidden
33- } else if (internal.forceShow) {
34- return Oxide.LocationBarController.ModeShown
35- } else {
36- return Oxide.LocationBarController.ModeAuto
37- }
38- }
39-
40- // Work around the lack of a show() method on the location bar controller
41- // (https://launchpad.net/bugs/1422920) by forcing its mode to ModeShown
42- // for long enough (500ms) to allow the animation to be committed.
43- QtObject {
44- id: internal
45- property bool forceShow: false
46- }
47- Timer {
48- id: delayedResetMode
49- interval: 500
50- onTriggered: internal.forceShow = false
51- }
52+ property int defaultMode: Oxide.LocationBarController.ModeAuto
53+
54+ onForceHideChanged: {
55+ if (!webview) {
56+ return
57+ }
58+ webview.locationBarController.animated = false
59+ if (forceHide) {
60+ webview.locationBarController.mode = Oxide.LocationBarController.ModeHidden
61+ } else if (!webview.fullscreen) {
62+ webview.locationBarController.mode = defaultMode
63+ if (webview.locationBarController.mode == Oxide.LocationBarController.ModeAuto) {
64+ webview.locationBarController.show(false)
65+ }
66+ }
67+ webview.locationBarController.animated = true
68+ }
69+
70 Connections {
71 target: webview
72 onFullscreenChanged: {
73- if (!webview.fullscreen) {
74- internal.forceShow = true
75- delayedResetMode.restart()
76+ if (webview.fullscreen) {
77+ webview.locationBarController.mode = Oxide.LocationBarController.ModeHidden
78+ } else if (!forceHide) {
79+ webview.locationBarController.mode = defaultMode
80+ if (webview.locationBarController.mode == Oxide.LocationBarController.ModeAuto) {
81+ webview.locationBarController.show(true)
82+ }
83 }
84 }
85 onLoadingChanged: {
86- if (webview.loading && !webview.fullscreen) {
87- internal.forceShow = true
88- delayedResetMode.restart()
89+ if (webview.loading && !webview.fullscreen && !forceHide &&
90+ (webview.locationBarController.mode == Oxide.LocationBarController.ModeAuto)) {
91+ webview.locationBarController.show(true)
92+ }
93+ }
94+ onLoadEvent: {
95+ // When loading, force ModeShown until the load is committed or stopped,
96+ // to work around https://launchpad.net/bugs/1453908.
97+ if (event.type == Oxide.LoadEvent.TypeStarted) {
98+ webview.locationBarController.mode = Oxide.LocationBarController.ModeShown
99+ } else if ((event.type == Oxide.LoadEvent.TypeCommitted) ||
100+ (event.type == Oxide.LoadEvent.TypeStopped)) {
101+ webview.locationBarController.mode = defaultMode
102 }
103 }
104 }
105
106=== modified file 'src/app/webbrowser/Browser.qml'
107--- src/app/webbrowser/Browser.qml 2015-05-06 21:51:16 +0000
108+++ src/app/webbrowser/Browser.qml 2015-05-11 19:26:41 +0000
109@@ -256,6 +256,14 @@
110 ]
111 }
112
113+ ChromeController {
114+ id: chromeController
115+ webview: browser.currentWebview
116+ forceHide: recentView.visible
117+ defaultMode: (formFactor == "desktop") ? Oxide.LocationBarController.ModeShown
118+ : Oxide.LocationBarController.ModeAuto
119+ }
120+
121 Suggestions {
122 id: suggestionsList
123 opacity: ((chrome.state == "shown") && chrome.activeFocus && (count > 0) && !chrome.drawerOpen) ? 1.0 : 0.0
124@@ -588,15 +596,9 @@
125
126 enabled: visible && !bottomEdgeHandle.dragging && !recentView.visible
127
128- ChromeController {
129- id: chromeController
130- webview: webviewimpl
131- forceHide: recentView.visible
132- }
133-
134 locationBarController {
135 height: webviewimpl.visible ? chrome.height : 0
136- mode: chromeController.mode
137+ mode: chromeController.defaultMode
138 }
139
140 //experimental.preferences.developerExtrasEnabled: developerExtrasEnabled
141
142=== modified file 'src/app/webcontainer/WebApp.qml'
143--- src/app/webcontainer/WebApp.qml 2015-04-08 13:20:57 +0000
144+++ src/app/webcontainer/WebApp.qml 2015-05-11 19:26:41 +0000
145@@ -143,16 +143,10 @@
146 }
147
148 ChromeController {
149- id: oxideChromeController
150 webview: webapp.currentWebview
151 forceHide: webapp.chromeless
152- }
153-
154- Binding {
155- when: webapp.currentWebview
156- target: webapp.currentWebview ? webapp.currentWebview.locationBarController : null
157- property: 'mode'
158- value: oxideChromeController.mode
159+ defaultMode: (formFactor == "desktop") ? Oxide.LocationBarController.ModeShown
160+ : Oxide.LocationBarController.ModeAuto
161 }
162 }
163

Subscribers

People subscribed via source and target branches

to status/vote changes: