webbrowser-app

Merge lp:~uriboni/webbrowser-app/bookmarks-in-suggestions into lp:webbrowser-app

  1. bookmarks-in-suggestions
  2. Merge into trunk
Proposed by Ugo Riboni
Status: Merged
Approved by: Olivier Tilloy
Approved revision: 995
Merged at revision: 973
Proposed branch: lp:~uriboni/webbrowser-app/bookmarks-in-suggestions
Merge into: lp:webbrowser-app
Diff against target: 1185 lines (+555/-335)
14 files modified
src/app/webbrowser/Browser.qml (+29/-5)
src/app/webbrowser/CMakeLists.txt (+1/-1)
src/app/webbrowser/Suggestion.qml (+81/-0)
src/app/webbrowser/Suggestions.qml (+57/-85)
src/app/webbrowser/history-matches-model.cpp (+0/-92)
src/app/webbrowser/history-matches-model.h (+0/-62)
src/app/webbrowser/suggestions-filter-model.cpp (+135/-0)
src/app/webbrowser/suggestions-filter-model.h (+72/-0)
src/app/webbrowser/webbrowser-app.cpp (+2/-2)
tests/autopilot/webbrowser_app/emulators/browser.py (+3/-5)
tests/autopilot/webbrowser_app/tests/test_suggestions.py (+93/-28)
tests/unittests/CMakeLists.txt (+1/-1)
tests/unittests/suggestions-filter-model/CMakeLists.txt (+2/-2)
tests/unittests/suggestions-filter-model/tst_SuggestionsFilterModelTests.cpp (+79/-52)
To merge this branch: bzr merge lp:~uriboni/webbrowser-app/bookmarks-in-suggestions
Reviewer Review Type Date Requested Status
Olivier Tilloy Approve
PS Jenkins bot continuous-integration Approve
Review via email: mp+256459@code.launchpad.net

Commit message

Include bookmark results in the suggestions list

Description of the change

Include bookmark results in the suggestions list.

Some notes:
- the design document specifies 8 max total suggestions, of which essentially 2 history, 2 bookmarks and 4 search results. since we don't do search results yet for now i set the limits to 4 history and 4 bookmarks.

- we had discussed not using LimitProxyModel, however it ends up being actually useful for this use case and working well. the alternative would have been to hand pick the items from the results to build a new list, but it proved very non-declarative and clunky. we might remove it in a further iteration by creating a c++ model that merges and limits the various results from suggestion sources (which would also solve the Suggestions.count property being slightly hackish), but I would leave that for later when we get the search engine suggestions as well.

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

Included a bunch of inline comments to clarify on pieces of the patch.

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

FAILED: Continuous integration, rev:981
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1653/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2274/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/410/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/410/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/410/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2272/console

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

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

I haven’t reviewed the code yet, but I’m seeing one functional issue here (it looks good and works well otherwise): testing on desktop, if I decrease the height of the window, then start typing in the address bar so that I have more results than fit of the screen, I can’t scroll them into view (I can see that they are there by dragging the list view, but when releasing they spring back out of sight).

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

PASSED: Continuous integration, rev:982
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1655/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2279
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/412
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/412
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/412/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/412
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/2002
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2277
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2277/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19700

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

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

In src/app/webbrowser/suggestions-filter-model.h:

 - updateSearchRoles is not reimplemented from QSortFilterProxyModel as the comment above and the absence of a blank line seems to imply, and it could be made private

In src/app/webbrowser/suggestions-filter-model.cpp:

 - In SuggestionsFilterModel::updateSearchRoles, the first parameter of Q_FOREACH should be a const QString&

 - In SuggestionsFilterModel::filterAcceptsRow, the implementation discards entries for which not all terms match in at least one of the roles, but I think it should be more tolerant, and match an entry if all the terms are found in at least one of the roles (i.e. for two search terms, if the first one matches only the first role and the second one matches only the second role, the entry should be included)

In tests/autopilot/webbrowser_app/emulators/browser.py:

 - In get_ordered_entries, items is effectively self.get_entries(), and I don’t think the comment is really necessary, the code is pretty self-explanatory (autopilot’s documentation makes it explicit that items are returned in no particular order)

In tests/unittests/suggestions-filter-model/tst_SuggestionsFilterModelTests.cpp:

 - In shouldMatch, there’s a leftover qDebug()

In src/app/webbrowser/Suggestions.qml:

 - Can’t you simply make models a var? If it was a javascript array instead of being a QML list, you could use the Array.reduce() function to compute the total count, thus inlining the function (and potentially making it faster to run)

In src/app/webbrowser/Suggestion.qml:

 - The call to highlightTerms() breaks encapsulation. The definition of the function should probably be moved to this file.

review: Needs Fixing
Revision history for this message
Ugo Riboni (uriboni) wrote :

Everything else not included in this reply has been fixed as requested

> - In SuggestionsFilterModel::filterAcceptsRow, the implementation discards
> entries for which not all terms match in at least one of the roles, but I
> think it should be more tolerant, and match an entry if all the terms are
> found in at least one of the roles (i.e. for two search terms, if the first
> one matches only the first role and the second one matches only the second
> role, the entry should be included)

Ok, the previous implementation was restrictive so I did not change it. Fixed as you request now.

> In src/app/webbrowser/Suggestions.qml:
>
> - Can’t you simply make models a var? If it was a javascript array instead of
> being a QML list, you could use the Array.reduce() function to compute the
> total count, thus inlining the function (and potentially making it faster to
> run)

No, if I make the model a var, and keep the same assignment code in Browser.qml, I get this: "Cannot assign multiple values to a singular property" when trying to assign a list to it.

To use a var, and thus a JS array as the list of models, with all the functional programming goodies it comes with, you have to use a slightly more cumbersome syntax on the assignment, but I think it is worth it.

Judge by yourself with the change I proposed.

> In src/app/webbrowser/Suggestion.qml:
>
> - The call to highlightTerms() breaks encapsulation. The definition of the
> function should probably be moved to this file.

Actually it was a mistake to call it from inside the Suggestion item itself, as it was already called in Suggestions.qml

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

615 + if (foundTerms.count() == m_terms.count()) {

I can see one possible corner case where this will fail: if m_terms contains doublons (e.g. if for whatever reason I typed in the address bar "ubuntu ubuntu"). This is arguably not a very important use case, but I think it’s easy enough to fix regardless, by comparing to QSet<QString>::fromList(m_terms).count() (and adding a unit test for it).

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

FAILED: Continuous integration, rev:984
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1659/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2288/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/416
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/416
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/416/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/416
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/2007/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2286
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2286/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19708

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

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

A suggestion (pun intended):

In tests/autopilot/webbrowser_app/emulators/browser.py, it seems get_entries() is not very useful, as when called from tests, swapping it for get_ordered_entries() wouldn’t make a difference.
Care to remove it, and rename get_ordered_entries() to get_entries() ?

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

I ran a coverage report on this branch, and there’s a couple of things that are not covered by the unit tests (but it’s easy to fix):

  - in SuggestionsFilterModel::updateSearchRoles(), the case of an inexistent role (role == -1)
  - SuggestionsFilterModel::count() seems to never be called from the unit tests

With the above fixed, coverage for SuggestionsFilterModel would be 100%.

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

FAILED: Continuous integration, rev:990
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1664/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2296
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/421
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/421
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/421/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/421
    None: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/2015/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2294
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2294/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19719

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

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

PASSED: Continuous integration, rev:993
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1665/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2298
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/422
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/422
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/422/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/422
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/2017
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2296
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2296/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19723

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

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

Looks good to me now, thanks!

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

When merging the latest trunk (lp:webbrowser-app) into this branch, it fails to build because includes were re-ordered in webbrowser-app.cpp, and when merging history-matches-model.h is incorrectly re-added. Can you please merge trunk and remove this spurious include?

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 2015-04-08 20:44:21 +0000
3+++ src/app/webbrowser/Browser.qml 2015-04-22 10:06:25 +0000
4@@ -257,6 +257,7 @@
5 }
6
7 Suggestions {
8+ id: suggestionsList
9 opacity: ((chrome.state == "shown") && chrome.activeFocus && (count > 0) && !chrome.drawerOpen) ? 1.0 : 0.0
10 Behavior on opacity {
11 UbuntuNumberAnimation {}
12@@ -267,11 +268,34 @@
13 horizontalCenter: parent.horizontalCenter
14 }
15 width: chrome.width - units.gu(5)
16- height: enabled ? Math.min(contentHeight, tabContainer.height - units.gu(2)) : 0
17- model: HistoryMatchesModel {
18- sourceModel: browser.historyModel
19- query: chrome.text
20- }
21+ height: enabled ? Math.min(contentHeight, tabContainer.height - chrome.height - units.gu(2)) : 0
22+
23+ searchTerms: chrome.text.split(/\s+/g).filter(function(term) { return term.length > 0 })
24+
25+ models: [historySuggestions, bookmarksSuggestions]
26+
27+ LimitProxyModel {
28+ id: historySuggestions
29+ limit: 4
30+ property string icon: "history"
31+ sourceModel: SuggestionsFilterModel {
32+ sourceModel: browser.historyModel
33+ terms: suggestionsList.searchTerms
34+ searchFields: ["url", "title"]
35+ }
36+ }
37+
38+ LimitProxyModel {
39+ id: bookmarksSuggestions
40+ limit: 4
41+ property string icon: "non-starred"
42+ sourceModel: SuggestionsFilterModel {
43+ sourceModel: browser.bookmarksModel
44+ terms: suggestionsList.searchTerms
45+ searchFields: ["url", "title"]
46+ }
47+ }
48+
49 onSelected: {
50 browser.currentWebview.url = url
51 browser.currentWebview.forceActiveFocus()
52
53=== modified file 'src/app/webbrowser/CMakeLists.txt'
54--- src/app/webbrowser/CMakeLists.txt 2015-04-09 13:59:02 +0000
55+++ src/app/webbrowser/CMakeLists.txt 2015-04-22 10:06:25 +0000
56@@ -11,10 +11,10 @@
57 history-domain-model.cpp
58 history-domainlist-chronological-model.cpp
59 history-domainlist-model.cpp
60- history-matches-model.cpp
61 history-model.cpp
62 history-timeframe-model.cpp
63 limit-proxy-model.cpp
64+ suggestions-filter-model.cpp
65 tabs-model.cpp
66 top-sites-model.cpp
67 )
68
69=== added file 'src/app/webbrowser/Suggestion.qml'
70--- src/app/webbrowser/Suggestion.qml 1970-01-01 00:00:00 +0000
71+++ src/app/webbrowser/Suggestion.qml 2015-04-22 10:06:25 +0000
72@@ -0,0 +1,81 @@
73+/*
74+ * Copyright 2015 Canonical Ltd.
75+ *
76+ * This file is part of webbrowser-app.
77+ *
78+ * webbrowser-app is free software; you can redistribute it and/or modify
79+ * it under the terms of the GNU General Public License as published by
80+ * the Free Software Foundation; version 3.
81+ *
82+ * webbrowser-app is distributed in the hope that it will be useful,
83+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
84+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
85+ * GNU General Public License for more details.
86+ *
87+ * You should have received a copy of the GNU General Public License
88+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
89+ */
90+
91+import QtQuick 2.0
92+import Ubuntu.Components 1.1
93+import Ubuntu.Components.ListItems 1.0 as ListItem
94+
95+// Not using ListItem.Subtitled because it’s not themable,
96+// and we want the subText to be on one line only.
97+ListItem.Base {
98+ property alias title: label.text
99+ property alias subtitle: subLabel.text
100+ property alias icon: icon.name
101+ property url url
102+
103+ signal selected(url url)
104+
105+ __height: Math.max(middleVisuals.height, units.gu(6))
106+ // disable focus handling
107+ activeFocusOnPress: false
108+
109+ Item {
110+ id: middleVisuals
111+ anchors {
112+ left: parent.left
113+ right: parent.right
114+ verticalCenter: parent.verticalCenter
115+ }
116+ height: label.height + subLabel.height
117+
118+ Icon {
119+ id: icon
120+ anchors {
121+ verticalCenter: parent.verticalCenter
122+ left: parent.left
123+ }
124+ width: units.gu(2)
125+ height: units.gu(2)
126+ }
127+
128+ Label {
129+ id: label
130+ anchors {
131+ top: parent.top
132+ left: icon.right
133+ leftMargin: units.gu(2)
134+ right: parent.right
135+ }
136+ elide: Text.ElideRight
137+ }
138+
139+ Label {
140+ id: subLabel
141+ anchors {
142+ top: label.bottom
143+ left: icon.right
144+ leftMargin: units.gu(2)
145+ right: parent.right
146+ }
147+ fontSize: "small"
148+ elide: Text.ElideRight
149+ }
150+ }
151+
152+ onClicked: selected(url)
153+}
154
155=== modified file 'src/app/webbrowser/Suggestions.qml'
156--- src/app/webbrowser/Suggestions.qml 2014-08-26 08:53:28 +0000
157+++ src/app/webbrowser/Suggestions.qml 2015-04-22 10:06:25 +0000
158@@ -18,14 +18,14 @@
159
160 import QtQuick 2.0
161 import Ubuntu.Components 1.1
162-import Ubuntu.Components.ListItems 1.0 as ListItem
163
164 Rectangle {
165 id: suggestions
166
167- property alias model: listview.model
168- property alias count: listview.count
169- property alias contentHeight: listview.contentHeight
170+ property var searchTerms
171+ property var models
172+ property int count: models.reduce(countItems, 0)
173+ property alias contentHeight: suggestionsList.contentHeight
174
175 signal selected(url url)
176
177@@ -38,92 +38,64 @@
178 clip: true
179
180 ListView {
181- id: listview
182-
183- anchors {
184- top: parent.top
185- left: parent.left
186- right: parent.right
187- }
188- height: parent.height
189-
190- delegate: ListItem.Base {
191- // Not using ListItem.Subtitled because it’s not themable,
192- // and we want the subText to be on one line only.
193-
194- property alias text: label.text
195- property alias subText: subLabel.text
196-
197- showDivider: index < (listview.count - 1)
198-
199- __height: Math.max(middleVisuals.height, units.gu(6))
200- // disable focus handling
201- activeFocusOnPress: false
202-
203- Item {
204- id: middleVisuals
205- anchors {
206- left: parent.left
207- right: parent.right
208- verticalCenter: parent.verticalCenter
209- }
210- height: childrenRect.height + label.anchors.topMargin + subLabel.anchors.bottomMargin
211-
212- Label {
213- id: label
214- anchors {
215- top: parent.top
216- left: parent.left
217- right: parent.right
218- }
219- elide: Text.ElideRight
220- text: highlightTerms(title, suggestions.model.terms)
221- }
222-
223- Label {
224- id: subLabel
225- anchors {
226- left: parent.left
227- right: parent.right
228- top: label.bottom
229- }
230- fontSize: "small"
231- elide: Text.ElideRight
232- text: highlightTerms(url, suggestions.model.terms)
233- }
234- }
235-
236- onClicked: suggestions.selected(url)
237-
238- function escapeTerm(term) {
239- // Build a regular expression suitable for highlighting a term
240- // in a case-insensitive manner and globally, by escaping
241- // special characters (a simpler version of preg_quote).
242- var escaped = term.replace(/[().?]/g, '\\$&')
243- return new RegExp(escaped, 'ig')
244- }
245-
246- function highlightTerms(text, terms) {
247- // Highlight the matching terms (bold and Ubuntu orange)
248- if (text === undefined) {
249- return ''
250- }
251- var highlighted = text.toString()
252- var count = terms.length
253- var highlight = '<b><font color="%1">$&</font></b>'.arg(UbuntuColors.orange)
254- for (var i = 0; i < count; ++i) {
255- var term = terms[i]
256- highlighted = highlighted.replace(escapeTerm(term), highlight)
257- }
258- highlighted = highlighted.replace(new RegExp('&', 'g'), '&amp;')
259- highlighted = '<html>' + highlighted + '</html>'
260- return highlighted
261+ id: suggestionsList
262+ anchors.fill: parent
263+
264+ model: suggestions.models
265+ delegate: Column {
266+ width: suggestionsList.width
267+ height: childrenRect.height
268+
269+ Repeater {
270+ id: suggestionsSource
271+ model: modelData
272+ property int firstItemIndex: models.slice(0, index).reduce(countItems, 0)
273+
274+ delegate: Suggestion {
275+ width: suggestionsList.width
276+ showDivider: suggestionsSource.firstItemIndex + index <
277+ suggestions.count - 1
278+
279+ title: highlightTerms(model.title)
280+ subtitle: highlightTerms(model.url)
281+ url: model.url
282+ icon: suggestionsSource.model.icon
283+
284+ onSelected: suggestions.selected(url)
285+ }
286 }
287 }
288 }
289
290 Scrollbar {
291- flickableItem: listview
292+ flickableItem: suggestionsList
293 align: Qt.AlignTrailing
294 }
295+
296+ function escapeTerm(term) {
297+ // Build a regular expression suitable for highlighting a term
298+ // in a case-insensitive manner and globally, by escaping
299+ // special characters (a simpler version of preg_quote).
300+ var escaped = term.replace(/[().?]/g, '\\$&')
301+ return new RegExp(escaped, 'ig')
302+ }
303+
304+ function highlightTerms(text) {
305+ // Highlight the matching terms (bold and Ubuntu orange)
306+ if (text === undefined) {
307+ return ''
308+ }
309+ var highlighted = text.toString()
310+ var count = searchTerms.length
311+ var highlight = '<b><font color="%1">$&</font></b>'.arg(UbuntuColors.orange)
312+ for (var i = 0; i < count; ++i) {
313+ var term = searchTerms[i]
314+ highlighted = highlighted.replace(escapeTerm(term), highlight)
315+ }
316+ highlighted = highlighted.replace(new RegExp('&', 'g'), '&amp;')
317+ highlighted = '<html>' + highlighted + '</html>'
318+ return highlighted
319+ }
320+
321+ function countItems(total, model) { return total + model.count }
322 }
323
324=== removed file 'src/app/webbrowser/history-matches-model.cpp'
325--- src/app/webbrowser/history-matches-model.cpp 2014-03-12 10:59:54 +0000
326+++ src/app/webbrowser/history-matches-model.cpp 1970-01-01 00:00:00 +0000
327@@ -1,92 +0,0 @@
328-/*
329- * Copyright 2013 Canonical Ltd.
330- *
331- * This file is part of webbrowser-app.
332- *
333- * webbrowser-app is free software; you can redistribute it and/or modify
334- * it under the terms of the GNU General Public License as published by
335- * the Free Software Foundation; version 3.
336- *
337- * webbrowser-app is distributed in the hope that it will be useful,
338- * but WITHOUT ANY WARRANTY; without even the implied warranty of
339- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
340- * GNU General Public License for more details.
341- *
342- * You should have received a copy of the GNU General Public License
343- * along with this program. If not, see <http://www.gnu.org/licenses/>.
344- */
345-
346-#include "history-matches-model.h"
347-#include "history-model.h"
348-
349-// Qt
350-#include <QtCore/QRegExp>
351-
352-/*!
353- \class HistoryMatchesModel
354- \brief Proxy model that filters the contents of the history model
355- based on a query string
356-
357- HistoryMatchesModel is a proxy model that filters the contents of a
358- HistoryModel based on a query string.
359-
360- The query string may contain several terms (or words).
361-
362- An entry in the history model matches if all the terms are contained in
363- either its URL or its title (inclusive OR).
364-*/
365-HistoryMatchesModel::HistoryMatchesModel(QObject* parent)
366- : QSortFilterProxyModel(parent)
367-{
368-}
369-
370-HistoryModel* HistoryMatchesModel::sourceModel() const
371-{
372- return qobject_cast<HistoryModel*>(QSortFilterProxyModel::sourceModel());
373-}
374-
375-void HistoryMatchesModel::setSourceModel(HistoryModel* sourceModel)
376-{
377- if (sourceModel != this->sourceModel()) {
378- QSortFilterProxyModel::setSourceModel(sourceModel);
379- Q_EMIT sourceModelChanged();
380- }
381-}
382-
383-const QString& HistoryMatchesModel::query() const
384-{
385- return m_query;
386-}
387-
388-void HistoryMatchesModel::setQuery(const QString& query)
389-{
390- if (query != m_query) {
391- m_query = query;
392- m_terms = query.split(QRegExp("\\s+"), QString::SkipEmptyParts);
393- invalidateFilter();
394- Q_EMIT queryChanged();
395- Q_EMIT termsChanged();
396- }
397-}
398-
399-const QStringList& HistoryMatchesModel::terms() const
400-{
401- return m_terms;
402-}
403-
404-bool HistoryMatchesModel::filterAcceptsRow(int source_row, const QModelIndex& source_parent) const
405-{
406- if (m_terms.isEmpty()) {
407- return false;
408- }
409- QModelIndex index = sourceModel()->index(source_row, 0, source_parent);
410- QString url = sourceModel()->data(index, HistoryModel::Url).toUrl().toString();
411- QString title = sourceModel()->data(index, HistoryModel::Title).toString();
412- Q_FOREACH (const QString& term, m_terms) {
413- if (!url.contains(term, Qt::CaseInsensitive) &&
414- !title.contains(term, Qt::CaseInsensitive)) {
415- return false;
416- }
417- }
418- return true;
419-}
420
421=== removed file 'src/app/webbrowser/history-matches-model.h'
422--- src/app/webbrowser/history-matches-model.h 2014-03-12 10:59:54 +0000
423+++ src/app/webbrowser/history-matches-model.h 1970-01-01 00:00:00 +0000
424@@ -1,62 +0,0 @@
425-/*
426- * Copyright 2013 Canonical Ltd.
427- *
428- * This file is part of webbrowser-app.
429- *
430- * webbrowser-app is free software; you can redistribute it and/or modify
431- * it under the terms of the GNU General Public License as published by
432- * the Free Software Foundation; version 3.
433- *
434- * webbrowser-app is distributed in the hope that it will be useful,
435- * but WITHOUT ANY WARRANTY; without even the implied warranty of
436- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
437- * GNU General Public License for more details.
438- *
439- * You should have received a copy of the GNU General Public License
440- * along with this program. If not, see <http://www.gnu.org/licenses/>.
441- */
442-
443-#ifndef __HISTORY_MATCHES_MODEL_H__
444-#define __HISTORY_MATCHES_MODEL_H__
445-
446-// Qt
447-#include <QtCore/QSortFilterProxyModel>
448-#include <QtCore/QString>
449-#include <QtCore/QStringList>
450-
451-class HistoryModel;
452-
453-class HistoryMatchesModel : public QSortFilterProxyModel
454-{
455- Q_OBJECT
456-
457- Q_PROPERTY(HistoryModel* sourceModel READ sourceModel WRITE setSourceModel NOTIFY sourceModelChanged)
458- Q_PROPERTY(QString query READ query WRITE setQuery NOTIFY queryChanged)
459- Q_PROPERTY(QStringList terms READ terms NOTIFY termsChanged)
460-
461-public:
462- HistoryMatchesModel(QObject* parent=0);
463-
464- HistoryModel* sourceModel() const;
465- void setSourceModel(HistoryModel* sourceModel);
466-
467- const QString& query() const;
468- void setQuery(const QString& query);
469-
470- const QStringList& terms() const;
471-
472-Q_SIGNALS:
473- void sourceModelChanged() const;
474- void queryChanged() const;
475- void termsChanged() const;
476-
477-protected:
478- // reimplemented from QSortFilterProxyModel
479- bool filterAcceptsRow(int source_row, const QModelIndex& source_parent) const;
480-
481-private:
482- QString m_query;
483- QStringList m_terms;
484-};
485-
486-#endif // __HISTORY_MATCHES_MODEL_H__
487
488=== added file 'src/app/webbrowser/suggestions-filter-model.cpp'
489--- src/app/webbrowser/suggestions-filter-model.cpp 1970-01-01 00:00:00 +0000
490+++ src/app/webbrowser/suggestions-filter-model.cpp 2015-04-22 10:06:25 +0000
491@@ -0,0 +1,135 @@
492+/*
493+ * Copyright 2015 Canonical Ltd.
494+ *
495+ * This file is part of webbrowser-app.
496+ *
497+ * webbrowser-app is free software; you can redistribute it and/or modify
498+ * it under the terms of the GNU General Public License as published by
499+ * the Free Software Foundation; version 3.
500+ *
501+ * webbrowser-app is distributed in the hope that it will be useful,
502+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
503+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
504+ * GNU General Public License for more details.
505+ *
506+ * You should have received a copy of the GNU General Public License
507+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
508+ */
509+
510+#include "suggestions-filter-model.h"
511+
512+#include <QtCore/QDebug>
513+#include <QtCore/QSet>
514+
515+/*!
516+ \class SuggestionsFilterModel
517+ \brief Proxy model that filters the contents of a model based on a list of
518+ keywords applied to multiple fields.
519+
520+ SuggestionsFilterModel is a proxy model that filters the contents of a
521+ model based on a list of terms string that is applied to multiple user
522+ defined fields (matching role names in the source model).
523+
524+ An item in the source model is returned by this model if all the search
525+ terms are contained across all of the item's fields (i.e. given two search
526+ terms, if one is found in a field and the other in a different field, then
527+ the item will be returned)
528+*/
529+SuggestionsFilterModel::SuggestionsFilterModel(QObject* parent)
530+ : QSortFilterProxyModel(parent)
531+{
532+}
533+
534+QVariant SuggestionsFilterModel::sourceModel() const
535+{
536+ QAbstractItemModel* source = QSortFilterProxyModel::sourceModel();
537+ return (source) ? QVariant::fromValue(source) : QVariant();
538+}
539+
540+void SuggestionsFilterModel::setSourceModel(QVariant sourceModel)
541+{
542+ QAbstractItemModel* currentSource = QSortFilterProxyModel::sourceModel();
543+ QAbstractItemModel* newSource = qvariant_cast<QAbstractItemModel*>(sourceModel);
544+ if (newSource != currentSource) {
545+ updateSearchRoles(newSource);
546+ QSortFilterProxyModel::setSourceModel(newSource);
547+ Q_EMIT sourceModelChanged();
548+ Q_EMIT countChanged();
549+ }
550+}
551+
552+void SuggestionsFilterModel::setTerms(const QStringList& terms)
553+{
554+ if (terms != m_terms) {
555+ m_terms = terms;
556+ invalidateFilter();
557+ Q_EMIT termsChanged();
558+ Q_EMIT countChanged();
559+ }
560+}
561+
562+const QStringList& SuggestionsFilterModel::terms() const
563+{
564+ return m_terms;
565+}
566+
567+void SuggestionsFilterModel::setSearchFields(const QStringList& searchFields)
568+{
569+ if (searchFields != m_searchFields) {
570+ m_searchFields = searchFields;
571+ updateSearchRoles(QSortFilterProxyModel::sourceModel());
572+ invalidateFilter();
573+ Q_EMIT searchFieldsChanged();
574+ Q_EMIT countChanged();
575+ }
576+}
577+
578+const QStringList& SuggestionsFilterModel::searchFields() const
579+{
580+ return m_searchFields;
581+}
582+
583+void SuggestionsFilterModel::updateSearchRoles(const QAbstractItemModel* model) {
584+ m_searchRoles.clear();
585+ if (model) {
586+ Q_FOREACH(const QString& field, m_searchFields) {
587+ int role = model->roleNames().key(field.toUtf8(), -1);
588+ if (role != -1) {
589+ m_searchRoles.append(role);
590+ } else {
591+ qWarning() << "Source model does not have role matching field:" << field;
592+ }
593+ }
594+ }
595+}
596+
597+bool SuggestionsFilterModel::filterAcceptsRow(int source_row, const QModelIndex& source_parent) const
598+{
599+ if (m_terms.isEmpty() || m_searchFields.isEmpty()) {
600+ return false;
601+ }
602+
603+ QAbstractItemModel* source = QSortFilterProxyModel::sourceModel();
604+ QModelIndex index = source->index(source_row, 0, source_parent);
605+
606+ QSet<QString> foundTerms;
607+ int searchTermsCount = QSet<QString>::fromList(m_terms).count();
608+
609+ Q_FOREACH(int role, m_searchRoles) {
610+ QString value = source->data(index, role).toString();
611+ Q_FOREACH (const QString& term, m_terms) {
612+ if (value.contains(term, Qt::CaseInsensitive)) {
613+ foundTerms.insert(term);
614+ }
615+ if (foundTerms.count() == searchTermsCount) {
616+ return true;
617+ }
618+ }
619+ }
620+ return false;
621+}
622+
623+int SuggestionsFilterModel::count() const
624+{
625+ return rowCount();
626+}
627
628=== added file 'src/app/webbrowser/suggestions-filter-model.h'
629--- src/app/webbrowser/suggestions-filter-model.h 1970-01-01 00:00:00 +0000
630+++ src/app/webbrowser/suggestions-filter-model.h 2015-04-22 10:06:25 +0000
631@@ -0,0 +1,72 @@
632+/*
633+ * Copyright 2015 Canonical Ltd.
634+ *
635+ * This file is part of webbrowser-app.
636+ *
637+ * webbrowser-app is free software; you can redistribute it and/or modify
638+ * it under the terms of the GNU General Public License as published by
639+ * the Free Software Foundation; version 3.
640+ *
641+ * webbrowser-app is distributed in the hope that it will be useful,
642+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
643+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
644+ * GNU General Public License for more details.
645+ *
646+ * You should have received a copy of the GNU General Public License
647+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
648+ */
649+
650+#ifndef SUGGESTIONSFILTERMODEL_H
651+#define SUGGESTIONSFILTERMODEL_H
652+
653+// Qt
654+#include <QtCore/QAbstractItemModel>
655+#include <QtCore/QList>
656+#include <QtCore/QSortFilterProxyModel>
657+#include <QtCore/QString>
658+#include <QtCore/QStringList>
659+#include <QtCore/QVariant>
660+
661+class SuggestionsFilterModel : public QSortFilterProxyModel
662+{
663+ Q_OBJECT
664+
665+ Q_PROPERTY(QVariant sourceModel READ sourceModel WRITE setSourceModel NOTIFY sourceModelChanged)
666+ Q_PROPERTY(QStringList terms READ terms WRITE setTerms NOTIFY termsChanged)
667+ Q_PROPERTY(QStringList searchFields READ searchFields WRITE setSearchFields NOTIFY searchFieldsChanged)
668+ Q_PROPERTY(int count READ count NOTIFY countChanged)
669+
670+public:
671+ SuggestionsFilterModel(QObject* parent=0);
672+
673+ QVariant sourceModel() const;
674+ void setSourceModel(QVariant sourceModel);
675+
676+ int count() const;
677+
678+ const QStringList& terms() const;
679+ void setTerms(const QStringList&);
680+
681+ const QStringList& searchFields() const;
682+ void setSearchFields(const QStringList&);
683+
684+Q_SIGNALS:
685+ void sourceModelChanged() const;
686+ void termsChanged() const;
687+ void searchFieldsChanged() const;
688+ void countChanged() const;
689+
690+protected:
691+ // reimplemented from QSortFilterProxyModel
692+ bool filterAcceptsRow(int source_row, const QModelIndex& source_parent) const;
693+
694+private:
695+ void updateSearchRoles(const QAbstractItemModel* model);
696+
697+ QStringList m_terms;
698+ QStringList m_searchFields;
699+ QList<int> m_searchRoles;
700+};
701+
702+
703+#endif // SUGGESTIONSFILTERMODEL_H
704
705=== modified file 'src/app/webbrowser/webbrowser-app.cpp'
706--- src/app/webbrowser/webbrowser-app.cpp 2015-04-09 14:05:35 +0000
707+++ src/app/webbrowser/webbrowser-app.cpp 2015-04-22 10:06:25 +0000
708@@ -22,11 +22,11 @@
709 #include "file-operations.h"
710 #include "history-domainlist-chronological-model.h"
711 #include "history-domainlist-model.h"
712-#include "history-matches-model.h"
713 #include "history-model.h"
714 #include "history-timeframe-model.h"
715 #include "limit-proxy-model.h"
716 #include "searchengine.h"
717+#include "suggestions-filter-model.h"
718 #include "tabs-model.h"
719 #include "top-sites-model.h"
720 #include "webbrowser-app.h"
721@@ -90,7 +90,6 @@
722
723 const char* uri = "webbrowserapp.private";
724 qmlRegisterType<HistoryModel>(uri, 0, 1, "HistoryModel");
725- qmlRegisterType<HistoryMatchesModel>(uri, 0, 1, "HistoryMatchesModel");
726 qmlRegisterType<HistoryTimeframeModel>(uri, 0, 1, "HistoryTimeframeModel");
727 qmlRegisterType<TopSitesModel>(uri, 0 , 1, "TopSitesModel");
728 qmlRegisterType<HistoryDomainListModel>(uri, 0, 1, "HistoryDomainListModel");
729@@ -101,6 +100,7 @@
730 qmlRegisterSingletonType<FileOperations>(uri, 0, 1, "FileOperations", FileOperations_singleton_factory);
731 qmlRegisterType<SearchEngine>(uri, 0, 1, "SearchEngine");
732 qmlRegisterSingletonType<CacheDeleter>(uri, 0, 1, "CacheDeleter", CacheDeleter_singleton_factory);
733+ qmlRegisterType<SuggestionsFilterModel>(uri, 0, 1, "SuggestionsFilterModel");
734
735 if (BrowserApplication::initialize("webbrowser/webbrowser-app.qml")) {
736 m_window->setProperty("newSession", m_arguments.contains("--new-session"));
737
738=== modified file 'tests/autopilot/webbrowser_app/emulators/browser.py'
739--- tests/autopilot/webbrowser_app/emulators/browser.py 2015-04-07 13:05:03 +0000
740+++ tests/autopilot/webbrowser_app/emulators/browser.py 2015-04-22 10:06:25 +0000
741@@ -197,11 +197,9 @@
742
743 class Suggestions(uitk.UbuntuUIToolkitCustomProxyObjectBase):
744
745- def get_list(self):
746- return self.select_single("QQuickListView")
747-
748- def get_entries(self):
749- return self.get_list().select_many("Base")
750+ def get_ordered_entries(self):
751+ return sorted(self.select_many("Suggestion"),
752+ key=lambda item: item.globalRect.y)
753
754
755 class GeolocationPermissionRequest(uitk.UbuntuUIToolkitCustomProxyObjectBase):
756
757=== renamed file 'tests/autopilot/webbrowser_app/tests/test_history.py' => 'tests/autopilot/webbrowser_app/tests/test_suggestions.py'
758--- tests/autopilot/webbrowser_app/tests/test_history.py 2015-03-23 07:49:19 +0000
759+++ tests/autopilot/webbrowser_app/tests/test_suggestions.py 2015-04-22 10:06:25 +0000
760@@ -25,12 +25,17 @@
761 from webbrowser_app.tests import StartOpenRemotePageTestCaseBase
762
763
764-class PrepopulatedHistoryDatabaseTestCaseBase(StartOpenRemotePageTestCaseBase):
765+class PrepopulatedDatabaseTestCaseBase(StartOpenRemotePageTestCaseBase):
766
767- """Helper test class that pre-populates the history database."""
768+ """Helper test class that pre-populates history and bookmarks databases."""
769
770 def setUp(self):
771 self.clear_datadir()
772+ self.populate_history()
773+ self.populate_bookmarks()
774+ super(PrepopulatedDatabaseTestCaseBase, self).setUp()
775+
776+ def populate_history(self):
777 db_path = os.path.join(os.path.expanduser("~"), ".local", "share",
778 "webbrowser-app", "history.sqlite")
779 connection = sqlite3.connect(db_path)
780@@ -54,7 +59,9 @@
781 "example - Google Search"),
782 ("http://example.iana.org/", "iana.org", "Example Domain"),
783 ("http://www.iana.org/domains/special", "iana.org",
784- "IANA — Special Use Domains")
785+ "IANA — Special Use Domains"),
786+ ("http://doc.qt.io/qt-5/qtqml-index.html", "qt.io",
787+ "Qt QML 5.4 - Qt Documentation")
788 ]
789 for i, row in enumerate(rows):
790 visits = random.randint(1, 5)
791@@ -65,12 +72,47 @@
792 connection.execute(query)
793 connection.commit()
794 connection.close()
795- super(PrepopulatedHistoryDatabaseTestCaseBase, self).setUp()
796-
797-
798-class TestHistorySuggestions(PrepopulatedHistoryDatabaseTestCaseBase):
799-
800- """Test the address bar suggestions based on navigation history."""
801+
802+ def populate_bookmarks(self):
803+ db_path = os.path.join(os.path.expanduser("~"), ".local", "share",
804+ "webbrowser-app", "bookmarks.sqlite")
805+ connection = sqlite3.connect(db_path)
806+ connection.execute("""CREATE TABLE IF NOT EXISTS bookmarks
807+ (url VARCHAR, title VARCHAR, icon VARCHAR,
808+ created INTEGER);""")
809+ rows = [
810+ ("http://www.rsc.org/periodic-table/element/24/chromium",
811+ "Chromium - Element Information"),
812+ ("http://www.rsc.org/periodic-table/element/77/iridium",
813+ "Iridium - Element Information"),
814+ ("http://www.rsc.org/periodic-table/element/31/gallium",
815+ "Gallium - Element Information"),
816+ ("http://www.rsc.org/periodic-table/element/116/livermorium",
817+ "Livermorium - Element Information"),
818+ ("http://www.rsc.org/periodic-table/element/62/samarium",
819+ "Samarium - Element Information"),
820+ ("http://en.wikipedia.org/wiki/Linux",
821+ "Linux - Wikipedia, the free encyclopedia"),
822+ ("https://www.linux.com/",
823+ "Linux.com | The source for Linux information"),
824+ ("http://doc.qt.io/qt-5/qtqml-index.html",
825+ "Qt QML 5.4 - Qt Documentation")
826+ ]
827+
828+ for i, row in enumerate(rows):
829+ timestamp = int(time.time()) - i * 10
830+ query = "INSERT INTO bookmarks \
831+ VALUES ('{}', '{}', '', {});"
832+ query = query.format(row[0], row[1], timestamp)
833+ connection.execute(query)
834+
835+ connection.commit()
836+ connection.close()
837+
838+
839+class TestSuggestions(PrepopulatedDatabaseTestCaseBase):
840+
841+ """Test the address bar suggestions (based on history and bookmarks)."""
842
843 def setUp(self):
844 super().setUp()
845@@ -85,24 +127,49 @@
846 self.assertThat(suggestions.opacity, Eventually(Equals(0)))
847
848 def test_show_list_of_suggestions(self):
849- listview = self.main_window.get_suggestions().get_list()
850+ suggestions = self.main_window.get_suggestions()
851 self.assert_suggestions_eventually_hidden()
852 self.assert_suggestions_eventually_hidden()
853 self.address_bar.focus()
854 self.assert_suggestions_eventually_shown()
855- self.assertThat(listview.count, Eventually(Equals(1)))
856+ self.assertThat(suggestions.count, Eventually(Equals(1)))
857 self.address_bar.clear()
858 self.assert_suggestions_eventually_hidden()
859- self.address_bar.write('u')
860- self.assert_suggestions_eventually_shown()
861- self.assertThat(listview.count, Eventually(Equals(6)))
862- self.address_bar.write('b', clear=False)
863- self.assertThat(listview.count, Eventually(Equals(5)))
864- self.address_bar.write('leh', clear=False)
865- self.assertThat(listview.count, Eventually(Equals(0)))
866- self.address_bar.clear()
867+
868+ def test_list_of_suggestions_case_insensitive(self):
869+ suggestions = self.main_window.get_suggestions()
870 self.address_bar.write('xaMPL')
871- self.assertThat(listview.count, Eventually(Equals(2)))
872+ self.assertThat(suggestions.count, Eventually(Equals(2)))
873+
874+ def test_list_of_suggestions_history_limits(self):
875+ suggestions = self.main_window.get_suggestions()
876+ self.address_bar.write('ubuntu')
877+ self.assert_suggestions_eventually_shown()
878+ self.assertThat(suggestions.count, Eventually(Equals(4)))
879+ self.address_bar.write('bleh', clear=False)
880+ self.assertThat(suggestions.count, Eventually(Equals(0)))
881+ self.address_bar.write('iana')
882+ self.assertThat(suggestions.count, Eventually(Equals(2)))
883+
884+ def test_list_of_suggestions_bookmark_limits(self):
885+ suggestions = self.main_window.get_suggestions()
886+ self.address_bar.write('element')
887+ self.assert_suggestions_eventually_shown()
888+ self.assertThat(suggestions.count, Eventually(Equals(4)))
889+ self.address_bar.write('bleh', clear=False)
890+ self.assertThat(suggestions.count, Eventually(Equals(0)))
891+ self.address_bar.write('linux')
892+ self.assertThat(suggestions.count, Eventually(Equals(2)))
893+
894+ def test_list_of_suggestions_order(self):
895+ suggestions = self.main_window.get_suggestions()
896+ self.address_bar.write('QML')
897+ self.assert_suggestions_eventually_shown()
898+ self.assertThat(suggestions.count, Eventually(Equals(2)))
899+ entries = suggestions.get_ordered_entries()
900+ self.assertThat(len(entries), Equals(2))
901+ self.assertThat(entries[0].icon, Equals("history"))
902+ self.assertThat(entries[1].icon, Equals("non-starred"))
903
904 def test_clear_address_bar_dismisses_suggestions(self):
905 self.address_bar.focus()
906@@ -140,18 +207,17 @@
907
908 def test_select_suggestion(self):
909 suggestions = self.main_window.get_suggestions()
910- listview = suggestions.get_list()
911 self.address_bar.focus()
912 self.assert_suggestions_eventually_shown()
913 self.address_bar.clear()
914 self.address_bar.write('ubuntu')
915 self.assert_suggestions_eventually_shown()
916- self.assertThat(listview.count, Eventually(Equals(5)))
917- entries = suggestions.get_entries()
918+ self.assertThat(suggestions.count, Eventually(Equals(4)))
919+ entries = suggestions.get_ordered_entries()
920 highlight = '<b><font color="#dd4814">Ubuntu</font></b>'
921 url = "http://en.wikipedia.org/wiki/{}_(operating_system)"
922 url = url.format(highlight)
923- entries = [entry for entry in entries if url in entry.subText]
924+ entries = [entry for entry in entries if url in entry.subtitle]
925 entry = entries[0] if len(entries) == 1 else None
926 self.assertIsNotNone(entry)
927 self.pointing_device.click_object(entry)
928@@ -165,9 +231,8 @@
929 self.address_bar.write('(phil')
930 self.assert_suggestions_eventually_shown()
931 suggestions = self.main_window.get_suggestions()
932- listview = suggestions.get_list()
933- self.assertThat(listview.count, Eventually(Equals(1)))
934- entry = suggestions.get_entries()[0]
935+ self.assertThat(suggestions.count, Eventually(Equals(1)))
936+ entry = suggestions.get_ordered_entries()[0]
937 highlight = '<b><font color="#dd4814">(phil</font></b>'
938 url = "http://en.wikipedia.org/wiki/Ubuntu_{}osophy)".format(highlight)
939- self.assertThat(entry.subText, Contains(url))
940+ self.assertThat(entry.subtitle, Contains(url))
941
942=== modified file 'tests/unittests/CMakeLists.txt'
943--- tests/unittests/CMakeLists.txt 2015-04-08 22:36:49 +0000
944+++ tests/unittests/CMakeLists.txt 2015-04-22 10:06:25 +0000
945@@ -2,13 +2,13 @@
946 add_subdirectory(qml)
947 add_subdirectory(domain-utils)
948 add_subdirectory(history-model)
949-add_subdirectory(history-matches-model)
950 add_subdirectory(history-timeframe-model)
951 add_subdirectory(history-domain-model)
952 add_subdirectory(history-domainlist-model)
953 add_subdirectory(history-domainlist-chronological-model)
954 add_subdirectory(top-sites-model)
955 add_subdirectory(session-utils)
956+add_subdirectory(suggestions-filter-model)
957 add_subdirectory(tabs-model)
958 add_subdirectory(bookmarks-model)
959 add_subdirectory(limit-proxy-model)
960
961=== renamed directory 'tests/unittests/history-matches-model' => 'tests/unittests/suggestions-filter-model'
962=== modified file 'tests/unittests/suggestions-filter-model/CMakeLists.txt'
963--- tests/unittests/history-matches-model/CMakeLists.txt 2014-03-17 12:44:23 +0000
964+++ tests/unittests/suggestions-filter-model/CMakeLists.txt 2015-04-22 10:06:25 +0000
965@@ -1,5 +1,5 @@
966-set(TEST tst_HistoryMatchesModelTests)
967-add_executable(${TEST} tst_HistoryMatchesModelTests.cpp)
968+set(TEST tst_SuggestionsFilterModelTests)
969+add_executable(${TEST} tst_SuggestionsFilterModelTests.cpp)
970 include_directories(${webbrowser-app_SOURCE_DIR})
971 qt5_use_modules(${TEST} Test)
972 target_link_libraries(${TEST} webbrowser-app-models)
973
974=== renamed file 'tests/unittests/history-matches-model/tst_HistoryMatchesModelTests.cpp' => 'tests/unittests/suggestions-filter-model/tst_SuggestionsFilterModelTests.cpp'
975--- tests/unittests/history-matches-model/tst_HistoryMatchesModelTests.cpp 2013-08-02 13:52:58 +0000
976+++ tests/unittests/suggestions-filter-model/tst_SuggestionsFilterModelTests.cpp 2015-04-22 10:06:25 +0000
977@@ -22,24 +22,23 @@
978
979 // local
980 #include "history-model.h"
981-#include "history-matches-model.h"
982-
983-
984-class HistoryMatchesModelTests : public QObject
985+#include "suggestions-filter-model.h"
986+
987+class SuggestionsFilterModelTests : public QObject
988 {
989 Q_OBJECT
990
991 private:
992 HistoryModel* model;
993- HistoryMatchesModel* matches;
994+ SuggestionsFilterModel* matches;
995
996 private Q_SLOTS:
997 void init()
998 {
999 model = new HistoryModel;
1000 model->setDatabasePath(":memory:");
1001- matches = new HistoryMatchesModel;
1002- matches->setSourceModel(model);
1003+ matches = new SuggestionsFilterModel;
1004+ matches->setSourceModel(QVariant::fromValue(model));
1005 }
1006
1007 void cleanup()
1008@@ -56,45 +55,58 @@
1009 void shouldNotifyWhenChangingSourceModel()
1010 {
1011 QSignalSpy spy(matches, SIGNAL(sourceModelChanged()));
1012- matches->setSourceModel(model);
1013+ matches->setSourceModel(QVariant::fromValue(model));
1014 QVERIFY(spy.isEmpty());
1015 HistoryModel* model2 = new HistoryModel;
1016- matches->setSourceModel(model2);
1017+ matches->setSourceModel(QVariant::fromValue(model2));
1018 QCOMPARE(spy.count(), 1);
1019- QCOMPARE(matches->sourceModel(), model2);
1020- matches->setSourceModel(0);
1021+ QCOMPARE(matches->sourceModel(), QVariant::fromValue(model2));
1022+ matches->setSourceModel(QVariant());
1023 QCOMPARE(spy.count(), 2);
1024- QCOMPARE(matches->sourceModel(), (HistoryModel*) 0);
1025+ QVERIFY(matches->sourceModel().isNull());
1026 delete model2;
1027 }
1028
1029- void shouldBeEmptyWhenQueryIsEmpty()
1030+ void shouldBeEmptyWhileTermsAndFieldsEmpty()
1031 {
1032 model->add(QUrl("http://example.org"), "Example Domain", QUrl());
1033 model->add(QUrl("http://example.com"), "Example Domain", QUrl());
1034 QCOMPARE(matches->rowCount(), 0);
1035 }
1036
1037- void shouldRecordQuery()
1038- {
1039- QVERIFY(matches->query().isEmpty());
1040- matches->setQuery("example");
1041- QCOMPARE(matches->query(), QString("example"));
1042- }
1043-
1044- void shouldMatchUrl()
1045+ void shouldRecordTerms()
1046+ {
1047+ QVERIFY(matches->terms().isEmpty());
1048+ QStringList terms;
1049+ terms.append("example");
1050+ matches->setTerms(terms);
1051+ QCOMPARE(matches->terms(), terms);
1052+ }
1053+
1054+ void shouldRecordSearchFields()
1055+ {
1056+ QVERIFY(matches->searchFields().isEmpty());
1057+ QStringList fields;
1058+ fields.append("url");
1059+ matches->setSearchFields(fields);
1060+ QCOMPARE(matches->searchFields(), fields);
1061+ }
1062+
1063+ void shouldMatch()
1064 {
1065 model->add(QUrl("http://example.org"), "Example Domain", QUrl());
1066 model->add(QUrl("http://example.com"), "Example Domain", QUrl());
1067- matches->setQuery("example");
1068+ matches->setTerms(QStringList({"example"}));
1069+ matches->setSearchFields(QStringList({"url", "title"}));
1070 QCOMPARE(matches->rowCount(), 2);
1071 }
1072
1073- void shouldMatchTitle()
1074+ void shouldMatchSecondField()
1075 {
1076 model->add(QUrl("http://example.org"), "Example Domain", QUrl());
1077 model->add(QUrl("http://example.com"), "Example Domain", QUrl());
1078- matches->setQuery("domain");
1079+ matches->setTerms(QStringList({"domain"}));
1080+ matches->setSearchFields(QStringList({"url", "title"}));
1081 QCOMPARE(matches->rowCount(), 2);
1082 }
1083
1084@@ -104,58 +116,73 @@
1085 model->add(QUrl("http://ubuntu.com"), "Home | Ubuntu", QUrl());
1086 model->add(QUrl("http://example.com"), "Example Domain", QUrl());
1087 model->add(QUrl("http://wikipedia.org"), "Wikipedia", QUrl());
1088- matches->setQuery("example");
1089+ matches->setTerms(QStringList({"domain"}));
1090+ matches->setSearchFields(QStringList({"url", "title"}));
1091 QCOMPARE(matches->rowCount(), 2);
1092 }
1093
1094- void shouldUpdateResultsWhenQueryChanges()
1095+ void shouldUpdateResultsWhenTermsChange()
1096 {
1097 model->add(QUrl("http://example.org"), "Example Domain", QUrl());
1098 model->add(QUrl("http://ubuntu.com"), "Home | Ubuntu", QUrl());
1099 model->add(QUrl("http://wikipedia.org"), "Wikipedia", QUrl());
1100 model->add(QUrl("http://ubuntu.com/download"), "Download Ubuntu | Ubuntu", QUrl());
1101- matches->setQuery("ubuntu");
1102+ matches->setSearchFields(QStringList({"url", "title"}));
1103+ matches->setTerms(QStringList({"ubuntu"}));
1104 QCOMPARE(matches->rowCount(), 2);
1105- matches->setQuery("wiki");
1106+ matches->setTerms(QStringList({"wiki"}));
1107 QCOMPARE(matches->rowCount(), 1);
1108- matches->setQuery("");
1109+ matches->setTerms(QStringList());
1110 QCOMPARE(matches->rowCount(), 0);
1111 }
1112
1113- void shouldUpdateResultsWhenHistoryChanges()
1114+ void shouldUpdateResultsWhenSourceModelUpdates()
1115 {
1116 model->add(QUrl("http://example.org"), "Example Domain", QUrl());
1117 model->add(QUrl("http://wikipedia.org"), "Wikipedia", QUrl());
1118- matches->setQuery("ubuntu");
1119+ matches->setTerms(QStringList({"ubuntu"}));
1120+ matches->setSearchFields(QStringList({"url"}));
1121 QCOMPARE(matches->rowCount(), 0);
1122 model->add(QUrl("http://ubuntu.com"), "Home | Ubuntu", QUrl());
1123 QCOMPARE(matches->rowCount(), 1);
1124 }
1125
1126- void shouldExtractTermsFromQuery()
1127- {
1128- matches->setQuery("ubuntu");
1129- QCOMPARE(matches->terms(), QStringList() << "ubuntu");
1130- matches->setQuery("download ubuntu");
1131- QCOMPARE(matches->terms(), QStringList() << "download" << "ubuntu");
1132- matches->setQuery(" ubuntu touch ");
1133- QCOMPARE(matches->terms(), QStringList() << "ubuntu" << "touch");
1134- matches->setQuery("ubuntu+touch");
1135- QCOMPARE(matches->terms(), QStringList() << "ubuntu+touch");
1136- }
1137-
1138 void shouldMatchAllTerms()
1139 {
1140 model->add(QUrl("http://example.org"), "Example Domain", QUrl());
1141- model->add(QUrl("http://ubuntu.com"), "Home | Ubuntu", QUrl());
1142- model->add(QUrl("http://wikipedia.org"), "Wikipedia", QUrl());
1143- model->add(QUrl("http://ubuntu.com/download"), "Download Ubuntu | Ubuntu", QUrl());
1144- matches->setQuery("ubuntu home");
1145- QCOMPARE(matches->rowCount(), 1);
1146- matches->setQuery("ubuntu wiki");
1147- QCOMPARE(matches->rowCount(), 0);
1148+ model->add(QUrl("http://example.com"), "Example Domain", QUrl());
1149+ matches->setTerms(QStringList({"example", "org"}));
1150+ matches->setSearchFields(QStringList({"url", "title"}));
1151+ QCOMPARE(matches->rowCount(), 1);
1152+ }
1153+
1154+ void shouldMatchTermsInDifferentFields()
1155+ {
1156+ model->add(QUrl("http://example.org"), "Example Domain", QUrl());
1157+ model->add(QUrl("http://example.com"), "Example Domain", QUrl());
1158+ matches->setTerms(QStringList({"org", "domain"}));
1159+ matches->setSearchFields(QStringList({"url", "title"}));
1160+ QCOMPARE(matches->rowCount(), 1);
1161+ }
1162+
1163+ void shouldMatchDuplicateTerms()
1164+ {
1165+ model->add(QUrl("http://example.org"), "Example Domain", QUrl());
1166+ model->add(QUrl("http://example.com"), "Example Domain", QUrl());
1167+ matches->setTerms(QStringList({"org", "org", "org", "org"}));
1168+ matches->setSearchFields(QStringList({"url", "title"}));
1169+ QCOMPARE(matches->rowCount(), 1);
1170+ }
1171+
1172+ void shouldWarnOnInvalidFields()
1173+ {
1174+ QTest::ignoreMessage(QtWarningMsg, "Source model does not have role matching field: \"foo\"");
1175+ model->add(QUrl("http://example.org"), "Example Domain", QUrl());
1176+ matches->setTerms(QStringList({"org"}));
1177+ matches->setSearchFields(QStringList({"url", "foo"}));
1178+ QCOMPARE(matches->count(), 1);
1179 }
1180 };
1181
1182-QTEST_MAIN(HistoryMatchesModelTests)
1183-#include "tst_HistoryMatchesModelTests.moc"
1184+QTEST_MAIN(SuggestionsFilterModelTests)
1185+#include "tst_SuggestionsFilterModelTests.moc"

Subscribers

People subscribed via source and target branches

to status/vote changes: