Merge lp:~uriboni/webbrowser-app/search-suggestions into lp:webbrowser-app

Proposed by Ugo Riboni
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
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.

To post a comment you must log in.
Revision history for this message
Ugo Riboni (uriboni) wrote : Posted in a previous version of this proposal

Added inline comments

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
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.

review: Needs Fixing
Revision history for this message
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 ?

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
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://duckduckgo.com/opensearch.xml) and making it the default search engine, but the suggestions URL doesn’t seem to be used, instead the default suggestions from Google are used.

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

review: Needs Fixing
Revision history for this message
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.

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Olivier Tilloy (osomon) : Posted in a previous version of this proposal
review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Olivier Tilloy (osomon) wrote : Posted in a previous version of this proposal

A few minor additional remarks:

In SearchSuggestions.qml:
  - 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_suggestions.py:
  - in setup_suggestions_source(), you should be able to replace "http://localhost:{}/" by "http://test/", as there is automatic mapping of test:80 requests to localhost:port
  - 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>{}</html>".format(re.sub(term, lambda m: self.highlight(m.group(0)), text))
  - in test_search_suggestions(), is the sleep really necessary?

Revision history for this message
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.

Revision history for this message
Ugo Riboni (uriboni) wrote : Posted in a previous version of this proposal

All suggestions implemented except for the following:

> In SearchSuggestions.qml:
> - 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_suggestions.py:
> - in setup_suggestions_source(), you should be able to replace
> "http://localhost:{}/" by "http://test/", as there is automatic mapping of
> 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>{}</html>".format(re.sub(term, lambda m:
> self.highlight(m.group(0)), text))

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

Revision history for this message
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.WebContext.hostMappingRules), so indeed it won’t work with XmlHttpRequest. Sorry for putting you on a false trail, I hadn’t realized the requests were not going through the WebView.

> 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.

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

That looks all good now.
I have only one minor concern: in TestSuggestions.setUp(), you’re assigning to self.server, which is later overridden when calling the parent’s setUp(). I understand you’re doing that because the the ping_server() method relies on self.server, but I think this is confusing and makes the code harder to maintain in the long run. Can you modify ping_server() to take the server object as parameter, and assign the search suggestions server instance to a different member variable?

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

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

Preview Diff

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

Subscribers

People subscribed via source and target branches

to status/vote changes: