webbrowser-app

Merge lp:~fboucault/webbrowser-app/simplify_tab_opening into lp:webbrowser-app/staging

  1. simplify_tab_opening
  2. Merge into staging
Proposed by Florian Boucault
Status: Needs review
Proposed branch: lp:~fboucault/webbrowser-app/simplify_tab_opening
Merge into: lp:webbrowser-app/staging
Prerequisite: lp:~fboucault/webbrowser-app/switch_to_parent_tab_on_close
Diff against target: 275 lines (+47/-48)
3 files modified
src/app/webbrowser/Browser.qml (+32/-27)
src/app/webbrowser/TabComponent.qml (+7/-11)
src/app/webbrowser/webbrowser-app.qml (+8/-10)
To merge this branch: bzr merge lp:~fboucault/webbrowser-app/simplify_tab_opening
Reviewer Review Type Date Requested Status
Ubuntu Phablet Team Pending
Review via email: mp+317983@code.launchpad.net

Commit message

Simplified tab opening, reduced number of entry points.

Moved Browser.internal.openUrlInNewTab to Browser.openUrlInNewTab and made it the main entry point to create tabs.
New Browser.openRequestInNewTab that takes in a 'request' instead of a 'url'.

TabComponent.WebViewImpl.onNewViewRequested to use Browser.openRequestInNewTab instead of BrowserWindow.addTab
Removed BrowserWindow.addTab
Browser.restoreTabState to use internal.createTabHelper instead of Browser.createTab
Removed Browser.createTab

Description of the change

Simplified tab opening, reduced number of entry points.

Moved Browser.internal.openUrlInNewTab to Browser.openUrlInNewTab and made it the main entry point to create tabs.
New Browser.openRequestInNewTab that takes in a 'request' instead of a 'url'.

TabComponent.WebViewImpl.onNewViewRequested to use Browser.openRequestInNewTab instead of BrowserWindow.addTab
Removed BrowserWindow.addTab
Browser.restoreTabState to use internal.createTabHelper instead of Browser.createTab
Removed Browser.createTab

To post a comment you must log in.

Unmerged revisions

1624. By Florian Boucault

Simplified tab opening, reduced number of entry points.

Moved Browser.internal.openUrlInNewTab to Browser.openUrlInNewTab and made it the main entry point to create tabs.
New Browser.openRequestInNewTab that takes in a 'request' instead of a 'url'.

TabComponent.WebViewImpl.onNewViewRequested to use Browser.openRequestInNewTab instead of BrowserWindow.addTab
Removed BrowserWindow.addTab
Browser.restoreTabState to use internal.createTabHelper instead of Browser.createTab
Removed Browser.createTab

1623. By Florian Boucault

Added corresponding AP tests

1622. By Florian Boucault

Restore focus on parent tab when closing child tab

1621. By Florian Boucault

AP test: simplified tab closing

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/app/webbrowser/Browser.qml'
2--- src/app/webbrowser/Browser.qml 2017-02-22 14:44:59 +0000
3+++ src/app/webbrowser/Browser.qml 2017-02-22 14:44:59 +0000
4@@ -49,7 +49,7 @@
5 readonly property int selectedIndex: currentIndex
6
7 function addTab() {
8- internal.openUrlInNewTab("", true, true, count)
9+ browser.openUrlInNewTab("", true, true, count)
10 }
11
12 function addExistingTab(tab) {
13@@ -102,10 +102,6 @@
14 'parentUniqueId': state.parentUniqueId,
15 'preview': state.preview, 'restoreState': state.savedState,
16 'restoreType': Oxide.WebView.RestoreLastSessionExitedCleanly}
17- return createTab(properties)
18- }
19-
20- function createTab(properties) {
21 return internal.createTabHelper(properties)
22 }
23
24@@ -123,6 +119,28 @@
25 internal.switchToTab(tabsModel.count - 1, true);
26 }
27
28+ function openUrlInNewTab(url, setCurrent, load, index, parentUniqueId) {
29+ load = typeof load !== 'undefined' ? load : true
30+ var tab = internal.createTabHelper({"initialUrl": url,
31+ "parentUniqueId": parentUniqueId})
32+ internal.addTab(tab, setCurrent, index)
33+ if (load) {
34+ tab.load()
35+ }
36+ if (!url || !url.toString()) {
37+ internal.maybeFocusAddressBar()
38+ }
39+ return tab
40+ }
41+
42+ function openRequestInNewTab(request, index, parentUniqueId) {
43+ var tab = internal.createTabHelper({"request": request,
44+ "parentUniqueId": parentUniqueId})
45+ var setCurrent = (request.disposition == Oxide.NewViewRequest.DispositionNewForegroundTab)
46+ internal.addTab(tab, setCurrent, index)
47+ return tab
48+ }
49+
50 signal newWindowRequested(bool incognito)
51 signal newWindowFromTab(var tab, var callback)
52 signal openLinkInWindowRequested(url url, bool incognito)
53@@ -195,7 +213,7 @@
54 onTriggered: internal.addBookmark(currentWebview.url, currentWebview.title, currentWebview.icon)
55 },
56 Actions.NewTab {
57- onTriggered: internal.openUrlInNewTab("", true)
58+ onTriggered: browser.openUrlInNewTab("", true)
59 },
60 Actions.ClearHistory {
61 onTriggered: HistoryModel.clearAll()
62@@ -493,7 +511,7 @@
63
64 onClicked: {
65 recentView.reset()
66- internal.openUrlInNewTab("", true)
67+ browser.openUrlInNewTab("", true)
68 }
69 }
70 }
71@@ -571,7 +589,7 @@
72 }
73
74 onSwitchToTab: internal.switchToTab(index, true)
75- onRequestNewTab: internal.openUrlInNewTab("", makeCurrent, true, index)
76+ onRequestNewTab: browser.openUrlInNewTab("", makeCurrent, true, index)
77 onTabClosed: internal.closeTab(index, moving)
78
79 onFindInPageModeChanged: {
80@@ -880,12 +898,12 @@
81 target: bookmarksViewLoader.item
82
83 onBookmarkEntryClicked: {
84- internal.openUrlInNewTab(url, true)
85+ browser.openUrlInNewTab(url, true)
86 bookmarksViewLoader.active = false
87 }
88 onBack: bookmarksViewLoader.active = false
89 onNewTabClicked: {
90- internal.openUrlInNewTab("", true)
91+ browser.openUrlInNewTab("", true)
92 bookmarksViewLoader.active = false
93 }
94 }
95@@ -912,11 +930,11 @@
96 target: historyViewLoader.item
97 onHistoryEntryClicked: {
98 historyViewLoader.active = false
99- internal.openUrlInNewTab(url, true)
100+ browser.openUrlInNewTab(url, true)
101 }
102 onNewTabRequested: {
103 historyViewLoader.active = false
104- internal.openUrlInNewTab("", true)
105+ browser.openUrlInNewTab("", true)
106 }
107 onDone: {
108 historyViewLoader.active = false
109@@ -1049,19 +1067,6 @@
110 if (share) share.shareText(text)
111 }
112
113- function openUrlInNewTab(url, setCurrent, load, index, parentUniqueId) {
114- load = typeof load !== 'undefined' ? load : true
115- var tab = internal.createTabHelper({"initialUrl": url,
116- "parentUniqueId": parentUniqueId})
117- addTab(tab, setCurrent, index)
118- if (load) {
119- tab.load()
120- }
121- if (!url.toString()) {
122- maybeFocusAddressBar()
123- }
124- }
125-
126 function addTab(tab, setCurrent, index) {
127 if (index === undefined) index = tabsModel.add(tab)
128 else index = tabsModel.insert(tab, index)
129@@ -1125,7 +1130,7 @@
130 tabsModel.currentTab.load()
131 }
132 if (tabsModel.count === 0) {
133- internal.openUrlInNewTab("", true)
134+ browser.openUrlInNewTab("", true)
135 recentView.reset()
136 }
137 }
138@@ -1334,7 +1339,7 @@
139 enabled: contentsContainer.visible || recentView.visible ||
140 bookmarksViewLoader.active || historyViewLoader.active
141 onActivated: {
142- internal.openUrlInNewTab("", true)
143+ browser.openUrlInNewTab("", true)
144 if (recentView.visible) recentView.reset()
145 bookmarksViewLoader.active = false
146 historyViewLoader.active = false
147
148=== modified file 'src/app/webbrowser/TabComponent.qml'
149--- src/app/webbrowser/TabComponent.qml 2017-02-22 14:44:59 +0000
150+++ src/app/webbrowser/TabComponent.qml 2017-02-22 14:44:59 +0000
151@@ -85,14 +85,14 @@
152 Actions.OpenLinkInNewTab {
153 objectName: "OpenLinkInNewTabContextualAction"
154 enabled: contextModel && contextModel.linkUrl.toString()
155- onTriggered: internal.openUrlInNewTab(contextModel.linkUrl, true,
156+ onTriggered: browser.openUrlInNewTab(contextModel.linkUrl, true,
157 true, tabsModel.indexOf(browserTab) + 1,
158 tab.uniqueId)
159 }
160 Actions.OpenLinkInNewBackgroundTab {
161 objectName: "OpenLinkInNewBackgroundTabContextualAction"
162 enabled: contextModel && contextModel.linkUrl.toString()
163- onTriggered: internal.openUrlInNewTab(contextModel.linkUrl, false,
164+ onTriggered: browser.openUrlInNewTab(contextModel.linkUrl, false,
165 true, tabsModel.indexOf(browserTab) + 1,
166 tab.uniqueId)
167 }
168@@ -147,7 +147,7 @@
169 enabled: contextModel &&
170 (contextModel.mediaType === Oxide.WebView.MediaTypeImage) &&
171 contextModel.srcUrl.toString()
172- onTriggered: internal.openUrlInNewTab(contextModel.srcUrl, true,
173+ onTriggered: browser.openUrlInNewTab(contextModel.srcUrl, true,
174 true, tabsModel.indexOf(browserTab) + 1,
175 tab.uniqueId)
176 }
177@@ -172,7 +172,7 @@
178 enabled: contextModel &&
179 (contextModel.mediaType === Oxide.WebView.MediaTypeVideo) &&
180 contextModel.srcUrl.toString()
181- onTriggered: internal.openUrlInNewTab(contextModel.srcUrl, true,
182+ onTriggered: browser.openUrlInNewTab(contextModel.srcUrl, true,
183 true, tabsModel.indexOf(browserTab) + 1,
184 tab.uniqueId)
185 }
186@@ -262,11 +262,7 @@
187 contextMenu: browser && browser.wide ? contextMenuWideComponent : contextMenuNarrowComponent
188
189 onNewViewRequested: {
190- var newTab = browser.createTab({"parentUniqueId": tab.uniqueId,
191- "request": request})
192- var setCurrent = (request.disposition == Oxide.NewViewRequest.DispositionNewForegroundTab)
193- internal.addTab(newTab, setCurrent, tabsModel.indexOf(browserTab) + 1)
194- if (setCurrent) tabContainer.forceActiveFocus()
195+ browser.openRequestInNewTab(request, tabsModel.indexOf(browserTab) + 1, tab.uniqueId)
196 }
197
198 onCloseRequested: prepareToClose()
199@@ -280,12 +276,12 @@
200
201 // tab.close() destroys the context so add new tab before destroy if required
202 if (tabsModel.count === 0) {
203- internal.openUrlInNewTab("", true, true)
204+ browser.openUrlInNewTab("", true, true)
205 }
206
207 tab.close()
208 } else if (tabsModel.count === 0) {
209- internal.openUrlInNewTab("", true, true)
210+ browser.openUrlInNewTab("", true, true)
211 }
212 }
213 }
214
215=== modified file 'src/app/webbrowser/webbrowser-app.qml'
216--- src/app/webbrowser/webbrowser-app.qml 2017-02-20 13:42:09 +0000
217+++ src/app/webbrowser/webbrowser-app.qml 2017-02-22 14:44:59 +0000
218@@ -39,11 +39,11 @@
219 }
220 var window = allWindows[allWindows.length - 1]
221 for (var i in urls) {
222- window.addTab(urls[i]).load()
223+ window.openUrlInNewTab(urls[i]).load()
224 window.tabsModel.currentIndex = window.tabsModel.count - 1
225 }
226 if (window.tabsModel.count == 0) {
227- window.addTab(incognito ? "" : settings.homepage).load()
228+ window.openUrlInNewTab(incognito ? "" : settings.homepage).load()
229 window.tabsModel.currentIndex = 0
230 }
231 for (var w in allWindows) {
232@@ -75,10 +75,10 @@
233 window = windowFactory.createObject(null, {"incognito": incognito})
234 }
235 for (var i in urls) {
236- window.addTab(urls[i]).load()
237+ window.openUrlInNewTab(urls[i]).load()
238 }
239 if (window.tabsModel.count == 0) {
240- window.addTab().load()
241+ window.openUrlInNewTab().load()
242 }
243 window.tabsModel.currentIndex = window.tabsModel.count - 1
244 window.show()
245@@ -197,7 +197,7 @@
246 "width": parent.width,
247 }
248 )
249- window.addTab()
250+ window.openUrlInNewTab()
251 window.tabsModel.currentIndex = 0
252 window.tabsModel.currentTab.load()
253 window.show()
254@@ -217,7 +217,7 @@
255 }
256 )
257 }
258- window.addTab(url)
259+ window.openUrlInNewTab(url)
260 window.tabsModel.currentIndex = window.tabsModel.count - 1
261 window.tabsModel.currentTab.load()
262 window.show()
263@@ -266,10 +266,8 @@
264 return browser.restoreTabState(state)
265 }
266
267- function addTab(url) {
268- var tab = browser.createTab({"initialUrl": url || ""})
269- tabsModel.add(tab)
270- return tab
271+ function openUrlInNewTab(url, setCurrent, load, index, parentUniqueId) {
272+ return browser.openUrlInNewTab(url, setCurrent, load, index, parentUniqueId)
273 }
274 }
275 }

Subscribers

People subscribed via source and target branches