Merge lp:~uriboni/webbrowser-app/search-suggestions into lp:webbrowser-app
- search-suggestions
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Olivier Tilloy |
Approved revision: | 995 |
Merged at revision: | 1006 |
Proposed branch: | lp:~uriboni/webbrowser-app/search-suggestions |
Merge into: | lp:webbrowser-app |
Prerequisite: | lp:~osomon/webbrowser-app/autopilot-temp-profile-config |
Diff against target: |
659 lines (+272/-54) 11 files modified
CMakeLists.txt (+2/-0) src/app/config.h.in (+5/-0) src/app/webbrowser/Browser.qml (+26/-5) src/app/webbrowser/SearchSuggestions.qml (+78/-0) src/app/webbrowser/Suggestion.qml (+5/-2) src/app/webbrowser/Suggestions.qml (+22/-11) src/app/webbrowser/searchengine.cpp (+13/-1) src/app/webbrowser/searchengine.h (+4/-0) tests/autopilot/webbrowser_app/tests/__init__.py (+6/-6) tests/autopilot/webbrowser_app/tests/http_server.py (+14/-1) tests/autopilot/webbrowser_app/tests/test_suggestions.py (+97/-28) |
To merge this branch: | bzr merge lp:~uriboni/webbrowser-app/search-suggestions |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Olivier Tilloy | Approve | ||
PS Jenkins bot | continuous-integration | Needs Fixing | |
Review via email: mp+258856@code.launchpad.net |
This proposal supersedes a proposal from 2015-04-22.
Commit message
Add suggestions from search engines in the suggestions list.
Description of the change
Add suggestions from search engines in the suggestions list.
Ugo Riboni (uriboni) wrote : Posted in a previous version of this proposal | # |
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:972
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:975
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://
Olivier Tilloy (osomon) wrote : Posted in a previous version of this proposal | # |
From a very quick functional test, this seems to work nicely (I’ll do more in-depth testing and a code review later).
One thing that I noticed is that the terms are not highlighted in search suggestions, as they are for history entries and bookmarks. According to the visual spec they should be.
Olivier Tilloy (osomon) wrote : Posted in a previous version of this proposal | # |
« This API is not official, but it is what Google products and everyone else uses to implement search suggestions. The "client=firefox" is there to request JSON output. »
Can you add this as a comment in config.h.in ?
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:976
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://
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:978
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:979
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://
Olivier Tilloy (osomon) wrote : Posted in a previous version of this proposal | # |
I’m seeing one funny behaviour: I’m adding a custom search engine definition (https:/
There are two things to fix here:
- that XML description file seems to have a valid suggestions URL, for some reason it’s not being parsed/used, it should be
- when loading a valid XML description file that doesn’t have a suggestions URL, we shouldn’t fall back to Google, instead there shouldn’t be any search suggestions
Olivier Tilloy (osomon) wrote : Posted in a previous version of this proposal | # |
When launching the app, the address bar is not focused, so the suggestions list is not shown, yet I’m seeing a query issued to the suggestions URL with the current URL. There shouldn’t be any queries issued until the address bar has focus and its contents change.
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:980
http://
Executed test runs:
FAILURE: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
FAILURE: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:980
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://
Olivier Tilloy (osomon) : Posted in a previous version of this proposal | # |
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:981
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://
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:982
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://
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:983
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://
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:984
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://
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:983
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://
Olivier Tilloy (osomon) wrote : Posted in a previous version of this proposal | # |
A few minor additional remarks:
In SearchSuggestio
- QQuickItem already has an 'enabled' property, I don’t think it’s necessary to override its definition
- in resetSearch(), the request (if any) should be aborted even if !enabled
A suggestion (pun not intended): in Browser.qml, in the two instances of LimitProxyModel that populate the list of suggestions for history entries and bookmarks, properties 'icon' and 'displayUrl' can be marked 'readonly'.
Similarly, in Suggestions.qml, the new properties added can be marked readonly.
In test_suggestion
- in setup_suggestio
- In highlight_term(), this function won’t work if there’s more than one occurrence of term in text. This is arguably unimportant for the scope of the tests, but the function could be simplified (and made to work in all cases) by simply returning "<html>
- in test_search_
Olivier Tilloy (osomon) wrote : Posted in a previous version of this proposal | # |
There is now a (trivial) conflict when merging this branch into the latest trunk.
test_suggestions.py will require a bit of refactoring to make use of the newly-introduced self.data_location (revision 994) to write test search engine descriptions.
Ugo Riboni (uriboni) wrote : Posted in a previous version of this proposal | # |
All suggestions implemented except for the following:
> In SearchSuggestio
> - QQuickItem already has an 'enabled' property, I don’t think it’s necessary
> to override its definition
Item.enabled has a completely different meaning tied to keyboard focus, and re-using it to do something else is wrong (and actually does not ever work since the property is toggled on and off various times by QML itself).
It was my mistake to redefine it though, as the original can't be accessed any more, so I defined a new "active" property that does the same thing, while leaving Item.enabled alone and with its original meaning.
> In test_suggestion
> - in setup_suggestio
> "http://
> test:80 requests to localhost:port
Is this a DNS mapping or a redirect of some sort ? Because it does not seem to work when using XmlHttpRequest from QML to get the list of suggestions. The responseText comes out empty, which is the way XmlHttpRequest signals errors, but I can't tell more details on why.
I spent way too much time on this already, so I will leave it as it was for now.
> - In highlight_term(), this function won’t work if there’s more than one
> occurrence of term in text. This is arguably unimportant for the scope of the
> tests, but the function could be simplified (and made to work in all cases) by
> simply returning "<html>
> self.highlight(
This does not work. In some cases I get a complain from re.py of "unbalanced parenthesis". Unless you have a quick fix I would leave it as it is.
- 991. By Ugo Riboni
-
Add project name in main cmake file, for convenience in qtcreator
Olivier Tilloy (osomon) wrote : | # |
> It was my mistake to redefine it though, as the original can't be accessed
> any more, so I defined a new "active" property that does the same thing,
> while leaving Item.enabled alone and with its original meaning.
Yeah, that’s better and safer indeed.
> Is this a DNS mapping or a redirect of some sort ? Because it does not seem
> to work when using XmlHttpRequest from QML to get the list of suggestions.
This is oxide-specific (see Oxide.WebContex
> This does not work. In some cases I get a complain from re.py of
> "unbalanced parenthesis". Unless you have a quick fix I would leave
> it as it is.
Yeah, that’s fine.
Olivier Tilloy (osomon) wrote : | # |
That looks all good now.
I have only one minor concern: in TestSuggestions
- 992. By Ugo Riboni
-
Keep the server in a temporary variable, instead of in the test instance
- 993. By Ugo Riboni
-
More safely store the server in an instance variable
- 994. By Ugo Riboni
-
Do the same in the package main
- 995. By Ugo Riboni
-
Better names
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:991
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://
Olivier Tilloy (osomon) : | # |
Preview Diff
1 | === modified file 'CMakeLists.txt' |
2 | --- CMakeLists.txt 2015-02-04 10:56:38 +0000 |
3 | +++ CMakeLists.txt 2015-05-12 13:40:34 +0000 |
4 | @@ -1,6 +1,8 @@ |
5 | cmake_minimum_required(VERSION 2.8.9) |
6 | set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake") |
7 | |
8 | +project("webbrowser") |
9 | + |
10 | find_program(INTLTOOL_MERGE intltool-merge) |
11 | if(NOT INTLTOOL_MERGE) |
12 | message(FATAL_ERROR "Could not find intltool-merge, please install the intltool package") |
13 | |
14 | === modified file 'src/app/config.h.in' |
15 | --- src/app/config.h.in 2015-03-23 22:35:12 +0000 |
16 | +++ src/app/config.h.in 2015-05-12 13:40:34 +0000 |
17 | @@ -30,6 +30,11 @@ |
18 | #define DEFAULT_SEARCH_DESC "Use google.com to search the Web" |
19 | #define DEFAULT_SEARCH_TEMPLATE "https://google.com/search?client=ubuntu&q={searchTerms}&ie=utf-8&oe=utf-8" |
20 | |
21 | +// This API is not official, but it is what Google products and everyone else |
22 | +// use to implement search suggestions. The "client=firefox" is there to request |
23 | +// JSON output. |
24 | +#define DEFAULT_SEARCH_SUGGESTIONS_TEMPLATE "http://suggestqueries.google.com/complete/search?client=firefox&q={searchTerms}" |
25 | + |
26 | inline bool isRunningInstalled() |
27 | { |
28 | static bool installed = (QCoreApplication::applicationDirPath() == QDir("@CMAKE_INSTALL_FULL_BINDIR@").canonicalPath()); |
29 | |
30 | === modified file 'src/app/webbrowser/Browser.qml' |
31 | --- src/app/webbrowser/Browser.qml 2015-05-06 21:51:16 +0000 |
32 | +++ src/app/webbrowser/Browser.qml 2015-05-12 13:40:34 +0000 |
33 | @@ -26,6 +26,8 @@ |
34 | import "../actions" as Actions |
35 | import ".." |
36 | import "../UrlUtils.js" as UrlUtils |
37 | +import "urlManagement.js" as UrlManagement |
38 | + |
39 | |
40 | BrowserView { |
41 | id: browser |
42 | @@ -272,12 +274,15 @@ |
43 | |
44 | searchTerms: chrome.text.split(/\s+/g).filter(function(term) { return term.length > 0 }) |
45 | |
46 | - models: [historySuggestions, bookmarksSuggestions] |
47 | + models: [historySuggestions, |
48 | + bookmarksSuggestions, |
49 | + searchSuggestions.limit(4)] |
50 | |
51 | LimitProxyModel { |
52 | id: historySuggestions |
53 | - limit: 4 |
54 | - property string icon: "history" |
55 | + limit: 2 |
56 | + readonly property string icon: "history" |
57 | + readonly property bool displayUrl: true |
58 | sourceModel: SuggestionsFilterModel { |
59 | sourceModel: browser.historyModel |
60 | terms: suggestionsList.searchTerms |
61 | @@ -287,8 +292,9 @@ |
62 | |
63 | LimitProxyModel { |
64 | id: bookmarksSuggestions |
65 | - limit: 4 |
66 | - property string icon: "non-starred" |
67 | + limit: 2 |
68 | + readonly property string icon: "non-starred" |
69 | + readonly property bool displayUrl: true |
70 | sourceModel: SuggestionsFilterModel { |
71 | sourceModel: browser.bookmarksModel |
72 | terms: suggestionsList.searchTerms |
73 | @@ -296,6 +302,21 @@ |
74 | } |
75 | } |
76 | |
77 | + SearchSuggestions { |
78 | + id: searchSuggestions |
79 | + terms: suggestionsList.searchTerms |
80 | + searchEngine: currentSearchEngine |
81 | + active: chrome.activeFocus && |
82 | + !UrlManagement.looksLikeAUrl(chrome.text.replace(/ /g, "+")) |
83 | + |
84 | + function limit(number) { |
85 | + var slice = results.slice(0, number) |
86 | + slice.icon = 'search' |
87 | + slice.displayUrl = false |
88 | + return slice |
89 | + } |
90 | + } |
91 | + |
92 | onSelected: { |
93 | browser.currentWebview.url = url |
94 | browser.currentWebview.forceActiveFocus() |
95 | |
96 | === added file 'src/app/webbrowser/SearchSuggestions.qml' |
97 | --- src/app/webbrowser/SearchSuggestions.qml 1970-01-01 00:00:00 +0000 |
98 | +++ src/app/webbrowser/SearchSuggestions.qml 2015-05-12 13:40:34 +0000 |
99 | @@ -0,0 +1,78 @@ |
100 | +/* |
101 | + * Copyright 2015 Canonical Ltd. |
102 | + * |
103 | + * This file is part of webbrowser-app. |
104 | + * |
105 | + * webbrowser-app is free software; you can redistribute it and/or modify |
106 | + * it under the terms of the GNU General Public License as published by |
107 | + * the Free Software Foundation; version 3. |
108 | + * |
109 | + * webbrowser-app is distributed in the hope that it will be useful, |
110 | + * but WITHOUT ANY WARRANTY; without even the implied warranty of |
111 | + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
112 | + * GNU General Public License for more details. |
113 | + * |
114 | + * You should have received a copy of the GNU General Public License |
115 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
116 | + */ |
117 | + |
118 | +import QtQuick 2.0 |
119 | +import webbrowserapp.private 0.1 |
120 | + |
121 | +Item { |
122 | + property var terms |
123 | + property SearchEngine searchEngine |
124 | + property var results: [] |
125 | + property bool active: false |
126 | + |
127 | + property var _request: new XMLHttpRequest() |
128 | + onSearchEngineChanged: resetSearch() |
129 | + onTermsChanged: resetSearch() |
130 | + onActiveChanged: resetSearch() |
131 | + |
132 | + Component.onCompleted: { |
133 | + _request.onreadystatechange = function() { |
134 | + if (_request.readyState === XMLHttpRequest.DONE) { |
135 | + results = parseResponse(_request.responseText) |
136 | + } |
137 | + } |
138 | + } |
139 | + |
140 | + Timer { |
141 | + id: limiter |
142 | + interval: 250 |
143 | + onTriggered: { |
144 | + if (_request && terms.length > 0 && searchEngine) { |
145 | + var url = searchEngine.suggestionsUrlTemplate |
146 | + url = url.replace("{searchTerms}", encodeURIComponent(terms.join(" "))) |
147 | + |
148 | + _request.open("GET", url); |
149 | + _request.send(); |
150 | + } |
151 | + } |
152 | + } |
153 | + |
154 | + function parseResponse(response) { |
155 | + try { |
156 | + var data = JSON.parse(response) |
157 | + } catch (error) { |
158 | + return [] |
159 | + } |
160 | + |
161 | + if (data.length > 1) { |
162 | + return data[1].map(function(result) { |
163 | + return { |
164 | + title: result, |
165 | + url: searchEngine.urlTemplate.replace("{searchTerms}", |
166 | + encodeURIComponent(result)) |
167 | + } |
168 | + }) |
169 | + } else return [] |
170 | + } |
171 | + |
172 | + function resetSearch() { |
173 | + results = [] |
174 | + if (_request) _request.abort() |
175 | + if (active) limiter.restart() |
176 | + } |
177 | +} |
178 | |
179 | === modified file 'src/app/webbrowser/Suggestion.qml' |
180 | --- src/app/webbrowser/Suggestion.qml 2015-04-16 16:42:04 +0000 |
181 | +++ src/app/webbrowser/Suggestion.qml 2015-05-12 13:40:34 +0000 |
182 | @@ -41,7 +41,7 @@ |
183 | right: parent.right |
184 | verticalCenter: parent.verticalCenter |
185 | } |
186 | - height: label.height + subLabel.height |
187 | + height: subLabel.visible ? label.height + subLabel.height : icon.height |
188 | |
189 | Icon { |
190 | id: icon |
191 | @@ -56,11 +56,13 @@ |
192 | Label { |
193 | id: label |
194 | anchors { |
195 | - top: parent.top |
196 | + top: subLabel.visible ? parent.top : undefined |
197 | + verticalCenter: subLabel.visible ? undefined : parent.verticalCenter |
198 | left: icon.right |
199 | leftMargin: units.gu(2) |
200 | right: parent.right |
201 | } |
202 | + |
203 | elide: Text.ElideRight |
204 | } |
205 | |
206 | @@ -74,6 +76,7 @@ |
207 | } |
208 | fontSize: "small" |
209 | elide: Text.ElideRight |
210 | + visible: text !== "" |
211 | } |
212 | } |
213 | |
214 | |
215 | === modified file 'src/app/webbrowser/Suggestions.qml' |
216 | --- src/app/webbrowser/Suggestions.qml 2015-04-16 16:41:31 +0000 |
217 | +++ src/app/webbrowser/Suggestions.qml 2015-05-12 13:40:34 +0000 |
218 | @@ -24,8 +24,8 @@ |
219 | |
220 | property var searchTerms |
221 | property var models |
222 | - property int count: models.reduce(countItems, 0) |
223 | - property alias contentHeight: suggestionsList.contentHeight |
224 | + readonly property int count: models.reduce(countItems, 0) |
225 | + readonly property alias contentHeight: suggestionsList.contentHeight |
226 | |
227 | signal selected(url url) |
228 | |
229 | @@ -43,25 +43,34 @@ |
230 | |
231 | model: suggestions.models |
232 | delegate: Column { |
233 | + id: suggestionsSection |
234 | width: suggestionsList.width |
235 | height: childrenRect.height |
236 | |
237 | + property string icon: models[index].icon |
238 | + property bool displayUrl: models[index].displayUrl |
239 | + property int firstItemIndex: models.slice(0, index).reduce(countItems, 0) |
240 | + |
241 | Repeater { |
242 | id: suggestionsSource |
243 | model: modelData |
244 | - property int firstItemIndex: models.slice(0, index).reduce(countItems, 0) |
245 | |
246 | delegate: Suggestion { |
247 | + id: suggestion |
248 | width: suggestionsList.width |
249 | - showDivider: suggestionsSource.firstItemIndex + index < |
250 | + showDivider: suggestionsSection.firstItemIndex + index < |
251 | suggestions.count - 1 |
252 | |
253 | - title: highlightTerms(model.title) |
254 | - subtitle: highlightTerms(model.url) |
255 | - url: model.url |
256 | - icon: suggestionsSource.model.icon |
257 | - |
258 | - onSelected: suggestions.selected(url) |
259 | + // Necessary to support both using objects inheriting from |
260 | + // QAbstractItemModel and JS arrays as models, since they |
261 | + // expose their data differently |
262 | + property var item: (model.modelData) ? model.modelData : model |
263 | + |
264 | + title: highlightTerms(item.title) |
265 | + subtitle: suggestionsSection.displayUrl ? highlightTerms(item.url) : "" |
266 | + icon: suggestionsSection.icon |
267 | + |
268 | + onSelected: suggestions.selected(item.url) |
269 | } |
270 | } |
271 | } |
272 | @@ -97,5 +106,7 @@ |
273 | return highlighted |
274 | } |
275 | |
276 | - function countItems(total, model) { return total + model.count } |
277 | + function countItems(total, model) { |
278 | + return total + (model.hasOwnProperty("length") ? model.length : model.count); |
279 | + } |
280 | } |
281 | |
282 | === modified file 'src/app/webbrowser/searchengine.cpp' |
283 | --- src/app/webbrowser/searchengine.cpp 2015-03-19 11:58:33 +0000 |
284 | +++ src/app/webbrowser/searchengine.cpp 2015-05-12 13:40:34 +0000 |
285 | @@ -30,6 +30,7 @@ |
286 | , m_name(DEFAULT_SEARCH_NAME) |
287 | , m_description(DEFAULT_SEARCH_DESC) |
288 | , m_template(DEFAULT_SEARCH_TEMPLATE) |
289 | + , m_suggestionsTemplate(DEFAULT_SEARCH_SUGGESTIONS_TEMPLATE) |
290 | { |
291 | } |
292 | |
293 | @@ -47,6 +48,7 @@ |
294 | m_name = DEFAULT_SEARCH_NAME; |
295 | m_description = DEFAULT_SEARCH_DESC; |
296 | m_template = DEFAULT_SEARCH_TEMPLATE; |
297 | + m_suggestionsTemplate = DEFAULT_SEARCH_SUGGESTIONS_TEMPLATE; |
298 | |
299 | if (!filename.isEmpty()) { |
300 | QString filepath = QStandardPaths::locate(QStandardPaths::DataLocation, |
301 | @@ -66,9 +68,13 @@ |
302 | } else if (name == "Description") { |
303 | m_description = parser.readElementText(); |
304 | } else if (name == "Url") { |
305 | - if (parser.attributes().value("type") == "text/html") { |
306 | + QStringRef type = parser.attributes().value("type"); |
307 | + if (type == "text/html") { |
308 | m_template = parser.attributes().value("template").toString(); |
309 | + } else if (type == "application/x-suggestions+json") { |
310 | + m_suggestionsTemplate = parser.attributes().value("template").toString(); |
311 | } |
312 | + |
313 | } |
314 | } |
315 | } |
316 | @@ -79,6 +85,7 @@ |
317 | Q_EMIT nameChanged(); |
318 | Q_EMIT descriptionChanged(); |
319 | Q_EMIT urlTemplateChanged(); |
320 | + Q_EMIT suggestionsUrlTemplateChanged(); |
321 | } |
322 | } |
323 | |
324 | @@ -96,3 +103,8 @@ |
325 | { |
326 | return m_template; |
327 | } |
328 | + |
329 | +const QString& SearchEngine::suggestionsUrlTemplate() const |
330 | +{ |
331 | + return m_suggestionsTemplate; |
332 | +} |
333 | |
334 | === modified file 'src/app/webbrowser/searchengine.h' |
335 | --- src/app/webbrowser/searchengine.h 2015-03-19 11:58:33 +0000 |
336 | +++ src/app/webbrowser/searchengine.h 2015-05-12 13:40:34 +0000 |
337 | @@ -31,6 +31,7 @@ |
338 | Q_PROPERTY(QString name READ name NOTIFY nameChanged) |
339 | Q_PROPERTY(QString description READ description NOTIFY descriptionChanged) |
340 | Q_PROPERTY(QString urlTemplate READ urlTemplate NOTIFY urlTemplateChanged) |
341 | + Q_PROPERTY(QString suggestionsUrlTemplate READ suggestionsUrlTemplate NOTIFY suggestionsUrlTemplateChanged) |
342 | |
343 | public: |
344 | SearchEngine(QObject* parent=0); |
345 | @@ -41,18 +42,21 @@ |
346 | const QString& name() const; |
347 | const QString& description() const; |
348 | const QString& urlTemplate() const; |
349 | + const QString& suggestionsUrlTemplate() const; |
350 | |
351 | Q_SIGNALS: |
352 | void filenameChanged() const; |
353 | void nameChanged() const; |
354 | void descriptionChanged() const; |
355 | void urlTemplateChanged() const; |
356 | + void suggestionsUrlTemplateChanged() const; |
357 | |
358 | private: |
359 | QString m_filename; |
360 | QString m_name; |
361 | QString m_description; |
362 | QString m_template; |
363 | + QString m_suggestionsTemplate; |
364 | }; |
365 | |
366 | #endif // __SEARCH_ENGINE_H__ |
367 | |
368 | === modified file 'tests/autopilot/webbrowser_app/tests/__init__.py' |
369 | --- tests/autopilot/webbrowser_app/tests/__init__.py 2015-05-12 13:40:34 +0000 |
370 | +++ tests/autopilot/webbrowser_app/tests/__init__.py 2015-05-12 13:40:34 +0000 |
371 | @@ -166,8 +166,8 @@ |
372 | self.assertThat(lambda: len(self.main_window.get_webviews()), |
373 | Eventually(Equals(count))) |
374 | |
375 | - def ping_server(self): |
376 | - url = "http://localhost:{}/ping".format(self.server.port) |
377 | + def ping_server(self, server): |
378 | + url = "http://localhost:{}/ping".format(server.port) |
379 | ping = urllib.request.urlopen(url) |
380 | self.assertThat(ping.read(), Equals(b"pong")) |
381 | |
382 | @@ -185,12 +185,12 @@ |
383 | """ |
384 | |
385 | def setUp(self): |
386 | - self.server = http_server.HTTPServerInAThread() |
387 | - self.ping_server() |
388 | - self.addCleanup(self.server.cleanup) |
389 | + self.http_server = http_server.HTTPServerInAThread() |
390 | + self.ping_server(self.http_server) |
391 | + self.addCleanup(self.http_server.cleanup) |
392 | self.useFixture(fixtures.EnvironmentVariable( |
393 | 'UBUNTU_WEBVIEW_HOST_MAPPING_RULES', |
394 | - "MAP test:80 localhost:{}".format(self.server.port))) |
395 | + "MAP test:80 localhost:{}".format(self.http_server.port))) |
396 | self.base_url = "http://test" |
397 | self.url = self.base_url + "/test1" |
398 | self.ARGS = self.ARGS + [self.url] |
399 | |
400 | === modified file 'tests/autopilot/webbrowser_app/tests/http_server.py' |
401 | --- tests/autopilot/webbrowser_app/tests/http_server.py 2015-03-20 15:08:55 +0000 |
402 | +++ tests/autopilot/webbrowser_app/tests/http_server.py 2015-05-12 13:40:34 +0000 |
403 | @@ -7,6 +7,7 @@ |
404 | # by the Free Software Foundation. |
405 | |
406 | import http.server as http |
407 | +import json |
408 | import logging |
409 | import threading |
410 | import time |
411 | @@ -19,6 +20,7 @@ |
412 | """ |
413 | A custom HTTP request handler that serves GET resources. |
414 | """ |
415 | + suggestions_data = {} |
416 | |
417 | def make_html(self, title, body): |
418 | html = "<html><title>{}</title><body>{}</body></html>" |
419 | @@ -130,6 +132,14 @@ |
420 | html += '<div style="height: 100%"></div>' |
421 | html += '</a></body></html>' |
422 | self.send_html(html) |
423 | + elif self.path.startswith("/suggest"): |
424 | + self.send_response(200) |
425 | + self.send_header("Content-Type", "text/x-suggestions+json") |
426 | + self.end_headers() |
427 | + query = self.path[len("/suggest?q="):] |
428 | + if query in self.suggestions_data: |
429 | + suggestions = self.suggestions_data[query] |
430 | + self.wfile.write(json.dumps(suggestions).encode()) |
431 | else: |
432 | self.send_error(404) |
433 | |
434 | @@ -145,10 +155,13 @@ |
435 | """ |
436 | A simple custom HTTP server run in a separate thread. |
437 | """ |
438 | + def set_suggestions_data(self, data): |
439 | + self.handler.suggestions_data = data |
440 | |
441 | def __init__(self): |
442 | # port == 0 will assign a random free port |
443 | - self.server = http.HTTPServer(("", 0), HTTPRequestHandler) |
444 | + self.handler = HTTPRequestHandler |
445 | + self.server = http.HTTPServer(("", 0), self.handler) |
446 | self.server.allow_reuse_address = True |
447 | self.server_thread = threading.Thread(target=self.server.serve_forever) |
448 | self.server_thread.start() |
449 | |
450 | === modified file 'tests/autopilot/webbrowser_app/tests/test_suggestions.py' |
451 | --- tests/autopilot/webbrowser_app/tests/test_suggestions.py 2015-04-27 03:23:00 +0000 |
452 | +++ tests/autopilot/webbrowser_app/tests/test_suggestions.py 2015-05-12 13:40:34 +0000 |
453 | @@ -23,6 +23,7 @@ |
454 | from autopilot.matchers import Eventually |
455 | |
456 | from webbrowser_app.tests import StartOpenRemotePageTestCaseBase |
457 | +from . import http_server |
458 | |
459 | |
460 | class PrepopulatedDatabaseTestCaseBase(StartOpenRemotePageTestCaseBase): |
461 | @@ -31,6 +32,7 @@ |
462 | |
463 | def setUp(self): |
464 | self.create_temporary_profile() |
465 | + |
466 | self.populate_history() |
467 | self.populate_bookmarks() |
468 | super(PrepopulatedDatabaseTestCaseBase, self).setUp() |
469 | @@ -56,7 +58,6 @@ |
470 | "Ubuntu (philosophy) - Wikipedia, the free encyclopedia"), |
471 | (search_uri.format("example"), "google.com", |
472 | "example - Google Search"), |
473 | - ("http://example.iana.org/", "iana.org", "Example Domain"), |
474 | ("http://www.iana.org/domains/special", "iana.org", |
475 | "IANA — Special Use Domains"), |
476 | ("http://doc.qt.io/qt-5/qtqml-index.html", "qt.io", |
477 | @@ -91,8 +92,6 @@ |
478 | "Samarium - Element Information"), |
479 | ("http://en.wikipedia.org/wiki/Linux", |
480 | "Linux - Wikipedia, the free encyclopedia"), |
481 | - ("https://www.linux.com/", |
482 | - "Linux.com | The source for Linux information"), |
483 | ("http://doc.qt.io/qt-5/qtqml-index.html", |
484 | "Qt QML 5.4 - Qt Documentation") |
485 | ] |
486 | @@ -112,10 +111,54 @@ |
487 | |
488 | """Test the address bar suggestions (based on history and bookmarks).""" |
489 | |
490 | + def setup_suggestions_source(self, server): |
491 | + search_engines_path = os.path.join(self.data_location, "searchengines") |
492 | + os.makedirs(search_engines_path, exist_ok=True) |
493 | + with open(os.path.join(search_engines_path, "test.xml"), "w") as f: |
494 | + f.write(""" |
495 | + <OpenSearchDescription> |
496 | + <Url type="application/x-suggestions+json" |
497 | + template="http://localhost:{}/suggest?q={searchTerms}"/> |
498 | + <Url type="text/html" |
499 | + template="http://aserver.somewhere/search?q={searchTerms}"/> |
500 | + </OpenSearchDescription> |
501 | + """.replace("{}", str(server.port))) |
502 | + |
503 | + with open(os.path.join(self.config_location, "webbrowser-app.conf"), |
504 | + "w") as f: |
505 | + f.write(""" |
506 | + [General] |
507 | + searchEngine=test |
508 | + """) |
509 | + server.set_suggestions_data({ |
510 | + "high": ["high", ["highlight"]], |
511 | + "foo": ["foo", ["food", "foot", "fool", "foobar", "foo five"]], |
512 | + "QML": ["QML", ["qt qml", "qml documentation", "qml rocks"]] |
513 | + }) |
514 | + |
515 | def setUp(self): |
516 | + self.suggest_http_server = http_server.HTTPServerInAThread() |
517 | + self.ping_server(self.suggest_http_server) |
518 | + self.addCleanup(self.suggest_http_server.cleanup) |
519 | + |
520 | + self.create_temporary_profile() |
521 | + self.setup_suggestions_source(self.suggest_http_server) |
522 | + |
523 | super(TestSuggestions, self).setUp() |
524 | + |
525 | self.address_bar = self.main_window.address_bar |
526 | |
527 | + def highlight_term(self, text, term): |
528 | + parts = text.split(term) |
529 | + if len(parts) < 2: |
530 | + return text |
531 | + else: |
532 | + pattern = '<html>{}{}{}</html>' |
533 | + return pattern.format(parts[0], self.highlight(term), parts[1]) |
534 | + |
535 | + def highlight(self, text): |
536 | + return '<b><font color="#dd4814">{}</font></b>'.format(text) |
537 | + |
538 | def assert_suggestions_eventually_shown(self): |
539 | suggestions = self.main_window.get_suggestions() |
540 | self.assertThat(suggestions.opacity, Eventually(Equals(1))) |
541 | @@ -136,38 +179,47 @@ |
542 | |
543 | def test_list_of_suggestions_case_insensitive(self): |
544 | suggestions = self.main_window.get_suggestions() |
545 | - self.address_bar.write('xaMPL') |
546 | - self.assertThat(suggestions.count, Eventually(Equals(2))) |
547 | + self.address_bar.write('SpEciAl') |
548 | + self.assertThat(suggestions.count, Eventually(Equals(1))) |
549 | |
550 | def test_list_of_suggestions_history_limits(self): |
551 | suggestions = self.main_window.get_suggestions() |
552 | self.address_bar.write('ubuntu') |
553 | self.assert_suggestions_eventually_shown() |
554 | - self.assertThat(suggestions.count, Eventually(Equals(4))) |
555 | + self.assertThat(suggestions.count, Eventually(Equals(2))) |
556 | self.address_bar.write('bleh', clear=False) |
557 | self.assertThat(suggestions.count, Eventually(Equals(0))) |
558 | self.address_bar.write('iana') |
559 | - self.assertThat(suggestions.count, Eventually(Equals(2))) |
560 | + self.assertThat(suggestions.count, Eventually(Equals(1))) |
561 | |
562 | def test_list_of_suggestions_bookmark_limits(self): |
563 | suggestions = self.main_window.get_suggestions() |
564 | self.address_bar.write('element') |
565 | self.assert_suggestions_eventually_shown() |
566 | + self.assertThat(suggestions.count, Eventually(Equals(2))) |
567 | + self.address_bar.write('bleh', clear=False) |
568 | + self.assertThat(suggestions.count, Eventually(Equals(0))) |
569 | + self.address_bar.write('linux') |
570 | + self.assertThat(suggestions.count, Eventually(Equals(1))) |
571 | + |
572 | + def test_list_of_suggestions_search_limits(self): |
573 | + suggestions = self.main_window.get_suggestions() |
574 | + self.address_bar.write('foo') |
575 | + self.assert_suggestions_eventually_shown() |
576 | self.assertThat(suggestions.count, Eventually(Equals(4))) |
577 | self.address_bar.write('bleh', clear=False) |
578 | self.assertThat(suggestions.count, Eventually(Equals(0))) |
579 | - self.address_bar.write('linux') |
580 | - self.assertThat(suggestions.count, Eventually(Equals(2))) |
581 | |
582 | def test_list_of_suggestions_order(self): |
583 | suggestions = self.main_window.get_suggestions() |
584 | self.address_bar.write('QML') |
585 | self.assert_suggestions_eventually_shown() |
586 | - self.assertThat(suggestions.count, Eventually(Equals(2))) |
587 | + self.assertThat(suggestions.count, Eventually(Equals(5))) |
588 | entries = suggestions.get_ordered_entries() |
589 | - self.assertThat(len(entries), Equals(2)) |
590 | + self.assertThat(len(entries), Equals(5)) |
591 | self.assertThat(entries[0].icon, Equals("history")) |
592 | self.assertThat(entries[1].icon, Equals("non-starred")) |
593 | + self.assertThat(entries[2].icon, Equals("search")) |
594 | |
595 | def test_clear_address_bar_dismisses_suggestions(self): |
596 | self.address_bar.focus() |
597 | @@ -208,21 +260,29 @@ |
598 | self.address_bar.focus() |
599 | self.assert_suggestions_eventually_shown() |
600 | self.address_bar.clear() |
601 | - self.address_bar.write('ubuntu') |
602 | - self.assert_suggestions_eventually_shown() |
603 | - self.assertThat(suggestions.count, Eventually(Equals(4))) |
604 | - entries = suggestions.get_ordered_entries() |
605 | - highlight = '<b><font color="#dd4814">Ubuntu</font></b>' |
606 | - url = "http://en.wikipedia.org/wiki/{}_(operating_system)" |
607 | - url = url.format(highlight) |
608 | - entries = [entry for entry in entries if url in entry.subtitle] |
609 | - entry = entries[0] if len(entries) == 1 else None |
610 | - self.assertIsNotNone(entry) |
611 | - self.pointing_device.click_object(entry) |
612 | - webview = self.main_window.get_current_webview() |
613 | - url = "wikipedia.org/wiki/Ubuntu_(operating_system)" |
614 | + self.address_bar.write('linux') |
615 | + self.assert_suggestions_eventually_shown() |
616 | + self.assertThat(suggestions.count, Eventually(Equals(1))) |
617 | + entries = suggestions.get_ordered_entries() |
618 | + url = "http://en.wikipedia.org/wiki/Linux" |
619 | + self.pointing_device.click_object(entries[0]) |
620 | + webview = self.main_window.get_current_webview() |
621 | + self.assertThat(webview.url, Eventually(Equals(url))) |
622 | + self.assert_suggestions_eventually_hidden() |
623 | + |
624 | + def test_select_search_suggestion(self): |
625 | + suggestions = self.main_window.get_suggestions() |
626 | + self.address_bar.focus() |
627 | + self.assert_suggestions_eventually_shown() |
628 | + self.address_bar.clear() |
629 | + self.address_bar.write('high') |
630 | + self.assert_suggestions_eventually_shown() |
631 | + self.assertThat(suggestions.count, Eventually(Equals(1))) |
632 | + entries = suggestions.get_ordered_entries() |
633 | + self.pointing_device.click_object(entries[0]) |
634 | + webview = self.main_window.get_current_webview() |
635 | + url = "aserver.somewhere/search?q=highlight" |
636 | self.assertThat(webview.url, Eventually(Contains(url))) |
637 | - self.assert_suggestions_eventually_hidden() |
638 | |
639 | def test_special_characters(self): |
640 | self.address_bar.clear() |
641 | @@ -231,6 +291,15 @@ |
642 | suggestions = self.main_window.get_suggestions() |
643 | self.assertThat(suggestions.count, Eventually(Equals(1))) |
644 | entry = suggestions.get_ordered_entries()[0] |
645 | - highlight = '<b><font color="#dd4814">(phil</font></b>' |
646 | - url = "http://en.wikipedia.org/wiki/Ubuntu_{}osophy)".format(highlight) |
647 | - self.assertThat(entry.subtitle, Contains(url)) |
648 | + url = "http://en.wikipedia.org/wiki/Ubuntu_(philosophy)" |
649 | + highlighted = self.highlight_term(url, "(phil") |
650 | + self.assertThat(entry.subtitle, Equals(highlighted)) |
651 | + |
652 | + def test_search_suggestions(self): |
653 | + self.address_bar.write('high') |
654 | + suggestions = self.main_window.get_suggestions() |
655 | + self.assertThat(suggestions.count, Eventually(Equals(1))) |
656 | + entries = suggestions.get_ordered_entries() |
657 | + highlighted = self.highlight_term("highlight", "high") |
658 | + self.assertThat(entries[0].title, Equals(highlighted)) |
659 | + self.assertThat(entries[0].subtitle, Equals('')) |
Added inline comments