Merge lp:~osomon/webbrowser-app/handle-new-view into lp:webbrowser-app

Proposed by Olivier Tilloy
Status: Merged
Merged at revision: 500
Proposed branch: lp:~osomon/webbrowser-app/handle-new-view
Merge into: lp:webbrowser-app
Diff against target: 207 lines (+19/-49)
5 files modified
src/Ubuntu/Components/Extras/Browser/UbuntuWebView02.qml (+1/-29)
src/app/webbrowser/Browser.qml (+15/-8)
src/app/webbrowser/webbrowser-app.qml (+1/-1)
src/app/webcontainer/WebViewImplOxide.qml (+2/-6)
tests/autopilot/webbrowser_app/tests/test_tabs.py (+0/-5)
To merge this branch: bzr merge lp:~osomon/webbrowser-app/handle-new-view
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Needs Fixing
Alexandre Abreu (community) Approve
Review via email: mp+215831@code.launchpad.net

Commit message

Handle new view requests in the browser.

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

FAILED: Continuous integration, rev:496
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/753/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4841
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/4171/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-amd64-ci/255
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/255
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/255/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-i386-ci/255
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/4184
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4978
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4978/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4418
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4418/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/6403/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6040

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Olivier Tilloy (osomon) wrote :

Note: the two autopilot failures on touch are expected, a fix has been committed in lp:oxide, a release will be requested at the same time as requesting a release for this one to ensure that everything passes.

Revision history for this message
Alexandre Abreu (abreu-alexandre) wrote :

LGTM

review: Approve
497. By Olivier Tilloy

Force focusing the address bar only when actually needed.

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

FAILED: Continuous integration, rev:497
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/755/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4851
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/4173/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-amd64-ci/257
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/257
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/257/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-i386-ci/257
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/4192
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4989
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4989/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4426
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4426/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/6405/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6051

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/755/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 'src/Ubuntu/Components/Extras/Browser/UbuntuWebView02.qml'
2--- src/Ubuntu/Components/Extras/Browser/UbuntuWebView02.qml 2014-04-10 10:29:35 +0000
3+++ src/Ubuntu/Components/Extras/Browser/UbuntuWebView02.qml 2014-04-15 15:51:10 +0000
4@@ -26,9 +26,6 @@
5 WebView {
6 id: _webview
7
8- // TODO Should be renamed to newViewRequested
9- signal newTabRequested(var request)
10-
11 //interactive: !selection.visible
12
13 /**
14@@ -97,36 +94,13 @@
15 }
16 ]
17
18- onNewViewRequested: {
19- // TODO: at this point request.disposition should be
20- // NewViewRequest.DispositionNewPopup (all dispositions are coerced to it),
21- // not sure if it is safer to add an extra level of filtering to avoid
22- // e.g. NewViewRequest.DispositionCurrentTab being opened
23- // as new tabs (although should never happen)
24- newTabRequested(request)
25- }
26-
27 onNavigationRequested: {
28 request.action = NavigationRequest.ActionAccept;
29-
30 navigationRequestedDelegate(request);
31-
32- /*
33- if (request.action === NavigationRequest.ActionReject)
34- return;
35-
36- var staticUA = _webview.getUAString()
37- if (staticUA === undefined) {
38- _webview.experimental.userAgent = userAgent.getUAString(request.url)
39- } else {
40- _webview.experimental.userAgent = staticUA
41- }
42- */
43 }
44
45 /*experimental.preferences.navigatorQtObjectEnabled: true
46- experimental.userScripts: [Qt.resolvedUrl("hyperlinks.js"),
47- Qt.resolvedUrl("selection.js")]
48+ experimental.userScripts: [Qt.resolvedUrl("selection.js")]
49 experimental.onMessageReceived: {
50 var data = null
51 try {
52@@ -169,8 +143,6 @@
53 selection.mimedata.urls = data.images
54 }
55 selection.show(data.left, data.top, data.width, data.height)
56- } else if (data.event === 'newtab') {
57- newTabRequested(data.url)
58 }
59 }
60 }*/
61
62=== modified file 'src/app/webbrowser/Browser.qml'
63--- src/app/webbrowser/Browser.qml 2014-03-17 22:16:59 +0000
64+++ src/app/webbrowser/Browser.qml 2014-04-15 15:51:10 +0000
65@@ -49,7 +49,7 @@
66 onTriggered: bookmarksModel.add(currentWebview.url, currentWebview.title, "")//currentWebview.icon)
67 },
68 Actions.NewTab {
69- onTriggered: newTab("", true)
70+ onTriggered: openUrlInNewTab("", true)
71 },
72 Actions.ClearHistory {
73 onTriggered: _historyModel.clearAll()
74@@ -88,7 +88,7 @@
75
76 function onNewTabRequested() {
77 toggleActivityView()
78- newTab("", true)
79+ openUrlInNewTab("", true)
80 }
81
82 function onSwitchToTabRequested(index) {
83@@ -220,7 +220,7 @@
84 contextualActions: ActionList {
85 Actions.OpenLinkInNewTab {
86 enabled: contextualData.href.toString()
87- onTriggered: newTab(contextualData.href, true)
88+ onTriggered: openUrlInNewTab(contextualData.href, true)
89 }
90 Actions.BookmarkLink {
91 enabled: contextualData.href.toString()
92@@ -232,7 +232,7 @@
93 }
94 Actions.OpenImageInNewTab {
95 enabled: contextualData.img.toString()
96- onTriggered: newTab(contextualData.img, true)
97+ onTriggered: openUrlInNewTab(contextualData.img, true)
98 }
99 Actions.CopyImage {
100 enabled: contextualData.img.toString()
101@@ -240,7 +240,10 @@
102 }
103 }
104
105- onNewTabRequested: newTab(url, true)
106+ onNewViewRequested: {
107+ var webview = webviewComponent.createObject(webviewContainer, {"request": request})
108+ addTab(webview, true, false)
109+ }
110
111 onLoadingChanged: {
112 if (lastLoadSucceeded) {
113@@ -250,13 +253,12 @@
114 }
115 }
116
117- function newTab(url, setCurrent) {
118- var webview = webviewComponent.createObject(webviewContainer, {"url": url})
119+ function addTab(webview, setCurrent, focusAddressBar) {
120 var index = tabsModel.add(webview)
121 if (setCurrent) {
122 tabsModel.currentIndex = index
123 if (!chromeless) {
124- if (!url) {
125+ if (focusAddressBar) {
126 panel.chrome.addressBar.forceActiveFocus()
127 panel.open()
128 }
129@@ -264,6 +266,11 @@
130 }
131 }
132
133+ function openUrlInNewTab(url, setCurrent) {
134+ var webview = webviewComponent.createObject(webviewContainer, {"url": url})
135+ addTab(webview, setCurrent, !url.toString())
136+ }
137+
138 function closeTab(index) {
139 var webview = tabsModel.remove(index)
140 if (webview) {
141
142=== modified file 'src/app/webbrowser/webbrowser-app.qml'
143--- src/app/webbrowser/webbrowser-app.qml 2014-03-12 22:45:12 +0000
144+++ src/app/webbrowser/webbrowser-app.qml 2014-04-15 15:51:10 +0000
145@@ -48,7 +48,7 @@
146 }
147
148 function newTab(url, setCurrent) {
149- return browser.newTab(url, setCurrent)
150+ return browser.openUrlInNewTab(url, setCurrent)
151 }
152
153 // Handle runtime requests to open urls as defined
154
155=== modified file 'src/app/webcontainer/WebViewImplOxide.qml'
156--- src/app/webcontainer/WebViewImplOxide.qml 2014-04-09 01:25:03 +0000
157+++ src/app/webcontainer/WebViewImplOxide.qml 2014-04-15 15:51:10 +0000
158@@ -143,17 +143,13 @@
159 webview.navigationRequestedDelegate(request)
160 }
161
162- onNewTabRequested: {
163- webview.createPopupWindow(request)
164- }
165+ onNewViewRequested: webview.createPopupWindow(request)
166 }
167 Component.onCompleted: popup.show()
168 }
169 }
170
171- onNewTabRequested: {
172- createPopupWindow(request)
173- }
174+ onNewViewRequested: createPopupWindow(request)
175
176 preferences.localStorageEnabled: true
177
178
179=== modified file 'tests/autopilot/webbrowser_app/tests/test_tabs.py'
180--- tests/autopilot/webbrowser_app/tests/test_tabs.py 2014-03-17 22:02:18 +0000
181+++ tests/autopilot/webbrowser_app/tests/test_tabs.py 2014-04-15 15:51:10 +0000
182@@ -17,7 +17,6 @@
183 from __future__ import absolute_import
184
185 import time
186-import unittest
187 from testtools.matchers import Equals
188 from autopilot.matchers import Eventually
189
190@@ -141,8 +140,6 @@
191 address_bar = self.main_window.get_address_bar()
192 self.assertThat(address_bar.activeFocus, Eventually(Equals(True)))
193
194- # FIXME: re-enable when implemented in oxide
195- @unittest.skip("not implemented in oxide yet")
196 def test_open_target_blank_in_new_tab(self):
197 url = self.base_url + "/blanktargetlink"
198 self.go_to_url(url)
199@@ -152,8 +149,6 @@
200 self.assertThat(self.main_window.currentIndex, Eventually(Equals(1)))
201 self.assert_current_url(self.base_url + "/aleaiactaest")
202
203- # FIXME: re-enable when implemented in oxide
204- @unittest.skip("not implemented in oxide yet")
205 def test_open_iframe_target_blank_in_new_tab(self):
206 url = self.base_url + "/fulliframewithblanktargetlink"
207 self.go_to_url(url)

Subscribers

People subscribed via source and target branches

to status/vote changes: