webbrowser-app

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

search-suggestions
Merge into trunk

Proposed by Ugo Riboni on 2015-04-22

Status:

Superseded

Proposed branch:

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

Merge into:

lp:webbrowser-app

Prerequisite:

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

Diff against target:

633 lines (+272/-48)

10 files modified

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 (+8/-0)
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

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Needs Fixing on 2015-05-07
Olivier Tilloy		2015-04-22	Approve on 2015-04-28
Review via email: mp+257062@code.launchpad.net

This proposal has been superseded by a proposal from 2015-05-12.

Description of the Change

Add suggestions from search engines in the suggestions list.

Ugo Riboni (uriboni) wrote on 2015-04-22:

Added inline comments

PS Jenkins bot (ps-jenkins) wrote on 2015-04-22:

FAILED: Continuous integration, rev:972
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1675/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2386/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/432/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/432/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/432/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2384/console

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

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-04-22:

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

review: Needs Fixing (continuous-integration)

Olivier Tilloy (osomon) wrote on 2015-04-22:

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

lp:~uriboni/webbrowser-app/search-suggestions updated on 2015-04-22

976. By Ugo Riboni on 2015-04-22: Suggestion title should be highlighted, was removed by mistake

Olivier Tilloy (osomon) wrote on 2015-04-22:

« 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 on 2015-04-22:

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

review: Needs Fixing (continuous-integration)

lp:~uriboni/webbrowser-app/search-suggestions updated on 2015-04-23

977. By Ugo Riboni on 2015-04-23: Add comment to clarify Google suggestion URL
978. By Ugo Riboni on 2015-04-23: Add search suggestion tests and fix the ones that were failing before

PS Jenkins bot (ps-jenkins) wrote on 2015-04-23:

FAILED: Continuous integration, rev:978
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1679/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2414/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/436/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/436/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/436/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2412/console

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

review: Needs Fixing (continuous-integration)

lp:~uriboni/webbrowser-app/search-suggestions updated on 2015-04-23

979. By Ugo Riboni on 2015-04-23: Fix flake8 issues, refactor and simplify some tests, fix some broken tests

PS Jenkins bot (ps-jenkins) wrote on 2015-04-23:

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

review: Needs Fixing (continuous-integration)

Olivier Tilloy (osomon) wrote on 2015-04-24:

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

Olivier Tilloy (osomon) wrote on 2015-04-24:

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

lp:~uriboni/webbrowser-app/search-suggestions updated on 2015-04-27

980. By Ugo Riboni on 2015-04-27: Ask for search suggestions only when the user is typing something

PS Jenkins bot (ps-jenkins) wrote on 2015-04-27:

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

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-04-28:

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

review: Needs Fixing (continuous-integration)

lp:~uriboni/webbrowser-app/search-suggestions updated on 2015-04-28

981. By Ugo Riboni on 2015-04-28: Remove unnecessary property alias

Olivier Tilloy (osomon) on 2015-04-28:

review: Approve

lp:~uriboni/webbrowser-app/search-suggestions updated on 2015-04-28

982. By Ugo Riboni on 2015-04-28: Merge changes from trunk

PS Jenkins bot (ps-jenkins) wrote on 2015-04-28:

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

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-04-28:

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

review: Needs Fixing (continuous-integration)

lp:~uriboni/webbrowser-app/search-suggestions updated on 2015-05-06

983. By Ugo Riboni on 2015-05-06: Merge changes from trunk
984. By Ugo Riboni on 2015-05-06: Adjust order of suggestions according to new input from design. Fix tests accordingly.

PS Jenkins bot (ps-jenkins) wrote on 2015-05-06:

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

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-05-07:

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

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-05-07:

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

review: Needs Fixing (continuous-integration)

Olivier Tilloy (osomon) wrote on 2015-05-11:

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?

Olivier Tilloy (osomon) wrote on 2015-05-12:

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.

lp:~uriboni/webbrowser-app/search-suggestions updated on 2015-05-12

985. By Ugo Riboni on 2015-05-12: Merge changes from trunk. Refactor some AP tests.
986. By Ugo Riboni on 2015-05-12: Merge new prerequisite branch to create separate temp dir for ~/.config
987. By Ugo Riboni on 2015-05-12: More fixes due to temp locations support
988. By Ugo Riboni on 2015-05-12: Create a new "active" property instead of overriding the Item.enabled property which might have side effects. Always abort the request when resetting.
989. By Ugo Riboni on 2015-05-12: Implement suggestions from code review
990. By Ugo Riboni on 2015-05-12: Remove unnecessary sleep

Ugo Riboni (uriboni) wrote on 2015-05-12:

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.

lp:~uriboni/webbrowser-app/search-suggestions updated on 2015-05-12

991. By Ugo Riboni on 2015-05-12: Add project name in main cmake file, for convenience in qtcreator
992. By Ugo Riboni on 2015-05-12: Keep the server in a temporary variable, instead of in the test instance
993. By Ugo Riboni on 2015-05-12: More safely store the server in an instance variable
994. By Ugo Riboni on 2015-05-12: Do the same in the package main
995. By Ugo Riboni on 2015-05-12: Better names

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Ubuntu Phablet Team

Ugo Riboni

to status/vote changes:

Rachel Liu

 === modified file 'src/app/config.h.in'
 --- src/app/config.h.in	2015-03-23 22:35:12 +0000
 +++ src/app/config.h.in	2015-05-12 10:18:25 +0000
@@ -30,6 +30,11 @@
  #define DEFAULT_SEARCH_DESC "Use google.com to search the Web"
  #define DEFAULT_SEARCH_TEMPLATE "https://google.com/search?client=ubuntu&q={searchTerms}&ie=utf-8&oe=utf-8"
++// This API is not official, but it is what Google products and everyone else
++// use to implement search suggestions. The "client=firefox" is there to request
++// JSON output.
++#define DEFAULT_SEARCH_SUGGESTIONS_TEMPLATE "http://suggestqueries.google.com/complete/search?client=firefox&q={searchTerms}"
++
  inline bool isRunningInstalled()
+ {
      static bool installed = (QCoreApplication::applicationDirPath() == QDir("@CMAKE_INSTALL_FULL_BINDIR@").canonicalPath());
 === modified file 'src/app/webbrowser/Browser.qml'
 --- src/app/webbrowser/Browser.qml	2015-05-06 21:51:16 +0000
 +++ src/app/webbrowser/Browser.qml	2015-05-12 10:18:25 +0000
@@ -26,6 +26,8 @@
  import "../actions" as Actions
  import ".."
  import "../UrlUtils.js" as UrlUtils
++import "urlManagement.js" as UrlManagement
++
  BrowserView {
      id: browser
@@ -272,12 +274,15 @@
              searchTerms: chrome.text.split(/\s+/g).filter(function(term) { return term.length > 0 })
--            models: [historySuggestions, bookmarksSuggestions]
++            models: [historySuggestions,
++                     bookmarksSuggestions,
++                     searchSuggestions.limit(4)]
              LimitProxyModel {
                  id: historySuggestions
--                limit: 4
--                property string icon: "history"
++                limit: 2
++                readonly property string icon: "history"
++                readonly property bool displayUrl: true
                  sourceModel: SuggestionsFilterModel {
                      sourceModel: browser.historyModel
                      terms: suggestionsList.searchTerms
@@ -287,8 +292,9 @@
              LimitProxyModel {
                  id: bookmarksSuggestions
--                limit: 4
--                property string icon: "non-starred"
++                limit: 2
++                readonly property string icon: "non-starred"
++                readonly property bool displayUrl: true
                  sourceModel: SuggestionsFilterModel {
                      sourceModel: browser.bookmarksModel
                      terms: suggestionsList.searchTerms
@@ -296,6 +302,21 @@
+                 }
+             }
++            SearchSuggestions {
++                id: searchSuggestions
++                terms: suggestionsList.searchTerms
++                searchEngine: currentSearchEngine
++                active: chrome.activeFocus &&
++                         !UrlManagement.looksLikeAUrl(chrome.text.replace(/ /g, "+"))
++
++                function limit(number) {
++                    var slice = results.slice(0, number)
++                    slice.icon = 'search'
++                    slice.displayUrl = false
++                    return slice
++                }
++            }
++
              onSelected: {
                  browser.currentWebview.url = url
                  browser.currentWebview.forceActiveFocus()
 === added file 'src/app/webbrowser/SearchSuggestions.qml'
 --- src/app/webbrowser/SearchSuggestions.qml	1970-01-01 00:00:00 +0000
 +++ src/app/webbrowser/SearchSuggestions.qml	2015-05-12 10:18:25 +0000
@@ -0,0 +1,78 @@
++/*
++ * Copyright 2015 Canonical Ltd.
++ *
++ * This file is part of webbrowser-app.
++ *
++ * webbrowser-app is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; version 3.
++ *
++ * webbrowser-app is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ */
++
++import QtQuick 2.0
++import webbrowserapp.private 0.1
++
++Item {
++    property var terms
++    property SearchEngine searchEngine
++    property var results: []
++    property bool active: false
++
++    property var _request: new XMLHttpRequest()
++    onSearchEngineChanged: resetSearch()
++    onTermsChanged: resetSearch()
++    onActiveChanged: resetSearch()
++
++    Component.onCompleted: {
++        _request.onreadystatechange = function() {
++            if (_request.readyState === XMLHttpRequest.DONE) {
++                results = parseResponse(_request.responseText)
++            }
++        }
++    }
++
++    Timer {
++        id: limiter
++        interval: 250
++        onTriggered:  {
++            if (_request && terms.length > 0 && searchEngine) {
++                var url = searchEngine.suggestionsUrlTemplate
++                url = url.replace("{searchTerms}", encodeURIComponent(terms.join(" ")))
++
++                _request.open("GET", url);
++                _request.send();
++            }
++        }
++    }
++
++    function parseResponse(response) {
++        try {
++            var data = JSON.parse(response)
++        } catch (error) {
++            return []
++        }
++
++        if (data.length > 1) {
++            return data[1].map(function(result) {
++                return {
++                    title: result,
++                    url: searchEngine.urlTemplate.replace("{searchTerms}",
++                                                          encodeURIComponent(result))
++                }
++            })
++        } else return []
++    }
++
++    function resetSearch() {
++        results = []
++        if (_request) _request.abort()
++        if (active) limiter.restart()
++    }
++}
 === modified file 'src/app/webbrowser/Suggestion.qml'
 --- src/app/webbrowser/Suggestion.qml	2015-04-16 16:42:04 +0000
 +++ src/app/webbrowser/Suggestion.qml	2015-05-12 10:18:25 +0000
@@ -41,7 +41,7 @@
              right: parent.right
              verticalCenter: parent.verticalCenter
+         }
--        height: label.height + subLabel.height
++        height: subLabel.visible ? label.height + subLabel.height : icon.height
          Icon {
              id: icon
@@ -56,11 +56,13 @@
          Label {
              id: label
              anchors {
--                top: parent.top
++                top: subLabel.visible ? parent.top : undefined
++                verticalCenter: subLabel.visible ? undefined : parent.verticalCenter
                  left: icon.right
                  leftMargin: units.gu(2)
                  right: parent.right
+             }
++
              elide: Text.ElideRight
+         }
@@ -74,6 +76,7 @@
+             }
              fontSize: "small"
              elide: Text.ElideRight
++            visible: text !== ""
+         }
+     }
 === modified file 'src/app/webbrowser/Suggestions.qml'
 --- src/app/webbrowser/Suggestions.qml	2015-04-16 16:41:31 +0000
 +++ src/app/webbrowser/Suggestions.qml	2015-05-12 10:18:25 +0000
@@ -24,8 +24,8 @@
      property var searchTerms
      property var models
--    property int count: models.reduce(countItems, 0)
--    property alias contentHeight: suggestionsList.contentHeight
++    readonly property int count: models.reduce(countItems, 0)
++    readonly property alias contentHeight: suggestionsList.contentHeight
      signal selected(url url)
@@ -43,25 +43,34 @@
          model: suggestions.models
          delegate: Column {
++            id: suggestionsSection
              width: suggestionsList.width
              height: childrenRect.height
++            property string icon: models[index].icon
++            property bool displayUrl: models[index].displayUrl
++            property int firstItemIndex: models.slice(0, index).reduce(countItems, 0)
++
              Repeater {
                  id: suggestionsSource
                  model: modelData
--                property int firstItemIndex: models.slice(0, index).reduce(countItems, 0)
                  delegate: Suggestion {
++                    id: suggestion
                      width: suggestionsList.width
--                    showDivider: suggestionsSource.firstItemIndex + index <
++                    showDivider: suggestionsSection.firstItemIndex + index <
                                   suggestions.count - 1
--                    title: highlightTerms(model.title)
--                    subtitle: highlightTerms(model.url)
--                    url: model.url
--                    icon: suggestionsSource.model.icon
--
--                    onSelected: suggestions.selected(url)
++                    // Necessary to support both using objects inheriting from
++                    // QAbstractItemModel and JS arrays as models, since they
++                    // expose their data differently
++                    property var item: (model.modelData) ? model.modelData : model
++
++                    title: highlightTerms(item.title)
++                    subtitle: suggestionsSection.displayUrl ? highlightTerms(item.url) : ""
++                    icon: suggestionsSection.icon
++
++                    onSelected: suggestions.selected(item.url)
+                 }
+             }
+         }
@@ -97,5 +106,7 @@
          return highlighted
+     }
--    function countItems(total, model) { return total + model.count }
++    function countItems(total, model) {
++        return total + (model.hasOwnProperty("length") ? model.length : model.count);
++    }
+ }
 === modified file 'src/app/webbrowser/searchengine.cpp'
 --- src/app/webbrowser/searchengine.cpp	2015-03-19 11:58:33 +0000
 +++ src/app/webbrowser/searchengine.cpp	2015-05-12 10:18:25 +0000
@@ -30,6 +30,7 @@
      , m_name(DEFAULT_SEARCH_NAME)
      , m_description(DEFAULT_SEARCH_DESC)
      , m_template(DEFAULT_SEARCH_TEMPLATE)
++    , m_suggestionsTemplate(DEFAULT_SEARCH_SUGGESTIONS_TEMPLATE)
+ {
+ }
@@ -47,6 +48,7 @@
          m_name = DEFAULT_SEARCH_NAME;
          m_description = DEFAULT_SEARCH_DESC;
          m_template = DEFAULT_SEARCH_TEMPLATE;
++        m_suggestionsTemplate = DEFAULT_SEARCH_SUGGESTIONS_TEMPLATE;
          if (!filename.isEmpty()) {
              QString filepath = QStandardPaths::locate(QStandardPaths::DataLocation,
@@ -66,9 +68,13 @@
                              } else if (name == "Description") {
                                  m_description = parser.readElementText();
                              } else if (name == "Url") {
--                                if (parser.attributes().value("type") == "text/html") {
++                                QStringRef type = parser.attributes().value("type");
++                                if (type == "text/html") {
                                      m_template = parser.attributes().value("template").toString();
++                                } else if (type == "application/x-suggestions+json") {
++                                    m_suggestionsTemplate = parser.attributes().value("template").toString();
+                                 }
++
+                             }
+                         }
+                     }
@@ -79,6 +85,7 @@
          Q_EMIT nameChanged();
          Q_EMIT descriptionChanged();
          Q_EMIT urlTemplateChanged();
++        Q_EMIT suggestionsUrlTemplateChanged();
+     }
+ }
@@ -96,3 +103,8 @@
+ {
      return m_template;
+ }
++
++const QString& SearchEngine::suggestionsUrlTemplate() const
++{
++    return m_suggestionsTemplate;
++}
 === modified file 'src/app/webbrowser/searchengine.h'
 --- src/app/webbrowser/searchengine.h	2015-03-19 11:58:33 +0000
 +++ src/app/webbrowser/searchengine.h	2015-05-12 10:18:25 +0000
@@ -31,6 +31,7 @@
      Q_PROPERTY(QString name READ name NOTIFY nameChanged)
      Q_PROPERTY(QString description READ description NOTIFY descriptionChanged)
      Q_PROPERTY(QString urlTemplate READ urlTemplate NOTIFY urlTemplateChanged)
++    Q_PROPERTY(QString suggestionsUrlTemplate READ suggestionsUrlTemplate NOTIFY suggestionsUrlTemplateChanged)
  public:
      SearchEngine(QObject* parent=0);
@@ -41,18 +42,21 @@
      const QString& name() const;
      const QString& description() const;
      const QString& urlTemplate() const;
++    const QString& suggestionsUrlTemplate() const;
  Q_SIGNALS:
      void filenameChanged() const;
      void nameChanged() const;
      void descriptionChanged() const;
      void urlTemplateChanged() const;
++    void suggestionsUrlTemplateChanged() const;
  private:
      QString m_filename;
      QString m_name;
      QString m_description;
      QString m_template;
++    QString m_suggestionsTemplate;
  };
  #endif // __SEARCH_ENGINE_H__
 === modified file 'tests/autopilot/webbrowser_app/tests/__init__.py'
 --- tests/autopilot/webbrowser_app/tests/__init__.py	2015-04-27 03:23:00 +0000
 +++ tests/autopilot/webbrowser_app/tests/__init__.py	2015-05-12 10:18:25 +0000
@@ -65,6 +65,14 @@
          if not os.path.exists(self.data_location):
              os.makedirs(self.data_location)
++        xdg_config = os.path.join(self._temp_xdg_dir, 'config')
++        self.useFixture(fixtures.EnvironmentVariable(
++            'XDG_CONFIG_HOME',
++            xdg_config))
++        self.config_location = os.path.join(xdg_config, appname)
++        if not os.path.exists(self.config_location):
++            os.makedirs(self.config_location)
++
          xdg_cache = os.path.join(self._temp_xdg_dir, 'cache')
          self.useFixture(fixtures.EnvironmentVariable(
              'XDG_CACHE_HOME',
 === modified file 'tests/autopilot/webbrowser_app/tests/http_server.py'
 --- tests/autopilot/webbrowser_app/tests/http_server.py	2015-03-20 15:08:55 +0000
 +++ tests/autopilot/webbrowser_app/tests/http_server.py	2015-05-12 10:18:25 +0000
@@ -7,6 +7,7 @@
  # by the Free Software Foundation.
  import http.server as http
++import json
  import logging
  import threading
  import time
@@ -19,6 +20,7 @@
      """
      A custom HTTP request handler that serves GET resources.
      """
++    suggestions_data = {}
      def make_html(self, title, body):
          html = "<html><title>{}</title><body>{}</body></html>"
@@ -130,6 +132,14 @@
              html += '<div style="height: 100%"></div>'
              html += '</a></body></html>'
              self.send_html(html)
++        elif self.path.startswith("/suggest"):
++            self.send_response(200)
++            self.send_header("Content-Type", "text/x-suggestions+json")
++            self.end_headers()
++            query = self.path[len("/suggest?q="):]
++            if query in self.suggestions_data:
++                suggestions = self.suggestions_data[query]
++                self.wfile.write(json.dumps(suggestions).encode())
          else:
              self.send_error(404)
@@ -145,10 +155,13 @@
      """
      A simple custom HTTP server run in a separate thread.
      """
++    def set_suggestions_data(self, data):
++        self.handler.suggestions_data = data
      def __init__(self):
          # port == 0 will assign a random free port
--        self.server = http.HTTPServer(("", 0), HTTPRequestHandler)
++        self.handler = HTTPRequestHandler
++        self.server = http.HTTPServer(("", 0), self.handler)
          self.server.allow_reuse_address = True
          self.server_thread = threading.Thread(target=self.server.serve_forever)
          self.server_thread.start()
 === modified file 'tests/autopilot/webbrowser_app/tests/test_suggestions.py'
 --- tests/autopilot/webbrowser_app/tests/test_suggestions.py	2015-04-27 03:23:00 +0000
 +++ tests/autopilot/webbrowser_app/tests/test_suggestions.py	2015-05-12 10:18:25 +0000
@@ -23,6 +23,7 @@
  from autopilot.matchers import Eventually
  from webbrowser_app.tests import StartOpenRemotePageTestCaseBase
++from . import http_server
  class PrepopulatedDatabaseTestCaseBase(StartOpenRemotePageTestCaseBase):
@@ -31,6 +32,7 @@
      def setUp(self):
          self.create_temporary_profile()
++
          self.populate_history()
          self.populate_bookmarks()
          super(PrepopulatedDatabaseTestCaseBase, self).setUp()
@@ -56,7 +58,6 @@
               "Ubuntu (philosophy) - Wikipedia, the free encyclopedia"),
              (search_uri.format("example"), "google.com",
               "example - Google Search"),
--            ("http://example.iana.org/", "iana.org", "Example Domain"),
              ("http://www.iana.org/domains/special", "iana.org",
               "IANA — Special Use Domains"),
              ("http://doc.qt.io/qt-5/qtqml-index.html", "qt.io",
@@ -91,8 +92,6 @@
               "Samarium - Element Information"),
              ("http://en.wikipedia.org/wiki/Linux",
               "Linux - Wikipedia, the free encyclopedia"),
--            ("https://www.linux.com/",
--             "Linux.com | The source for Linux information"),
              ("http://doc.qt.io/qt-5/qtqml-index.html",
               "Qt QML 5.4 - Qt Documentation")
+         ]
@@ -112,10 +111,54 @@
      """Test the address bar suggestions (based on history and bookmarks)."""
++    def setup_suggestions_source(self, server):
++        search_engines_path = os.path.join(self.data_location, "searchengines")
++        os.makedirs(search_engines_path, exist_ok=True)
++        with open(os.path.join(search_engines_path, "test.xml"), "w") as f:
++            f.write("""
++            <OpenSearchDescription>
++             <Url type="application/x-suggestions+json"
++                  template="http://localhost:{}/suggest?q={searchTerms}"/>
++              <Url type="text/html"
++                   template="http://aserver.somewhere/search?q={searchTerms}"/>
++            </OpenSearchDescription>
++            """.replace("{}", str(server.port)))
++
++        with open(os.path.join(self.config_location, "webbrowser-app.conf"),
++                  "w") as f:
++            f.write("""
++            [General]
++            searchEngine=test
++            """)
++        server.set_suggestions_data({
++            "high": ["high", ["highlight"]],
++            "foo": ["foo", ["food", "foot", "fool", "foobar", "foo five"]],
++            "QML": ["QML", ["qt qml", "qml documentation", "qml rocks"]]
++        })
++
      def setUp(self):
++        self.server = http_server.HTTPServerInAThread()
++        self.ping_server()
++        self.addCleanup(self.server.cleanup)
++
++        self.create_temporary_profile()
++        self.setup_suggestions_source(self.server)
++
          super(TestSuggestions, self).setUp()
++
          self.address_bar = self.main_window.address_bar
++    def highlight_term(self, text, term):
++        parts = text.split(term)
++        if len(parts) < 2:
++            return text
++        else:
++            pattern = '<html>{}{}{}</html>'
++            return pattern.format(parts[0], self.highlight(term), parts[1])
++
++    def highlight(self, text):
++        return '<b><font color="#dd4814">{}</font></b>'.format(text)
++
      def assert_suggestions_eventually_shown(self):
          suggestions = self.main_window.get_suggestions()
          self.assertThat(suggestions.opacity, Eventually(Equals(1)))
@@ -136,38 +179,47 @@
      def test_list_of_suggestions_case_insensitive(self):
          suggestions = self.main_window.get_suggestions()
--        self.address_bar.write('xaMPL')
--        self.assertThat(suggestions.count, Eventually(Equals(2)))
++        self.address_bar.write('SpEciAl')
++        self.assertThat(suggestions.count, Eventually(Equals(1)))
      def test_list_of_suggestions_history_limits(self):
          suggestions = self.main_window.get_suggestions()
          self.address_bar.write('ubuntu')
          self.assert_suggestions_eventually_shown()
--        self.assertThat(suggestions.count, Eventually(Equals(4)))
++        self.assertThat(suggestions.count, Eventually(Equals(2)))
          self.address_bar.write('bleh', clear=False)
          self.assertThat(suggestions.count, Eventually(Equals(0)))
          self.address_bar.write('iana')
--        self.assertThat(suggestions.count, Eventually(Equals(2)))
++        self.assertThat(suggestions.count, Eventually(Equals(1)))
      def test_list_of_suggestions_bookmark_limits(self):
          suggestions = self.main_window.get_suggestions()
          self.address_bar.write('element')
          self.assert_suggestions_eventually_shown()
++        self.assertThat(suggestions.count, Eventually(Equals(2)))
++        self.address_bar.write('bleh', clear=False)
++        self.assertThat(suggestions.count, Eventually(Equals(0)))
++        self.address_bar.write('linux')
++        self.assertThat(suggestions.count, Eventually(Equals(1)))
++
++    def test_list_of_suggestions_search_limits(self):
++        suggestions = self.main_window.get_suggestions()
++        self.address_bar.write('foo')
++        self.assert_suggestions_eventually_shown()
          self.assertThat(suggestions.count, Eventually(Equals(4)))
          self.address_bar.write('bleh', clear=False)
          self.assertThat(suggestions.count, Eventually(Equals(0)))
--        self.address_bar.write('linux')
--        self.assertThat(suggestions.count, Eventually(Equals(2)))
      def test_list_of_suggestions_order(self):
          suggestions = self.main_window.get_suggestions()
          self.address_bar.write('QML')
          self.assert_suggestions_eventually_shown()
--        self.assertThat(suggestions.count, Eventually(Equals(2)))
++        self.assertThat(suggestions.count, Eventually(Equals(5)))
          entries = suggestions.get_ordered_entries()
--        self.assertThat(len(entries), Equals(2))
++        self.assertThat(len(entries), Equals(5))
          self.assertThat(entries[0].icon, Equals("history"))
          self.assertThat(entries[1].icon, Equals("non-starred"))
++        self.assertThat(entries[2].icon, Equals("search"))
      def test_clear_address_bar_dismisses_suggestions(self):
          self.address_bar.focus()
@@ -208,21 +260,29 @@
          self.address_bar.focus()
          self.assert_suggestions_eventually_shown()
          self.address_bar.clear()
--        self.address_bar.write('ubuntu')
--        self.assert_suggestions_eventually_shown()
--        self.assertThat(suggestions.count, Eventually(Equals(4)))
--        entries = suggestions.get_ordered_entries()
--        highlight = '<b><font color="#dd4814">Ubuntu</font></b>'
--        url = "http://en.wikipedia.org/wiki/{}_(operating_system)"
--        url = url.format(highlight)
--        entries = [entry for entry in entries if url in entry.subtitle]
--        entry = entries[0] if len(entries) == 1 else None
--        self.assertIsNotNone(entry)
--        self.pointing_device.click_object(entry)
--        webview = self.main_window.get_current_webview()
--        url = "wikipedia.org/wiki/Ubuntu_(operating_system)"
++        self.address_bar.write('linux')
++        self.assert_suggestions_eventually_shown()
++        self.assertThat(suggestions.count, Eventually(Equals(1)))
++        entries = suggestions.get_ordered_entries()
++        url = "http://en.wikipedia.org/wiki/Linux"
++        self.pointing_device.click_object(entries[0])
++        webview = self.main_window.get_current_webview()
++        self.assertThat(webview.url, Eventually(Equals(url)))
++        self.assert_suggestions_eventually_hidden()
++
++    def test_select_search_suggestion(self):
++        suggestions = self.main_window.get_suggestions()
++        self.address_bar.focus()
++        self.assert_suggestions_eventually_shown()
++        self.address_bar.clear()
++        self.address_bar.write('high')
++        self.assert_suggestions_eventually_shown()
++        self.assertThat(suggestions.count, Eventually(Equals(1)))
++        entries = suggestions.get_ordered_entries()
++        self.pointing_device.click_object(entries[0])
++        webview = self.main_window.get_current_webview()
++        url = "aserver.somewhere/search?q=highlight"
          self.assertThat(webview.url, Eventually(Contains(url)))
--        self.assert_suggestions_eventually_hidden()
      def test_special_characters(self):
          self.address_bar.clear()
@@ -231,6 +291,15 @@
          suggestions = self.main_window.get_suggestions()
          self.assertThat(suggestions.count, Eventually(Equals(1)))
          entry = suggestions.get_ordered_entries()[0]
--        highlight = '<b><font color="#dd4814">(phil</font></b>'
--        url = "http://en.wikipedia.org/wiki/Ubuntu_{}osophy)".format(highlight)
--        self.assertThat(entry.subtitle, Contains(url))
++        url = "http://en.wikipedia.org/wiki/Ubuntu_(philosophy)"
++        highlighted = self.highlight_term(url, "(phil")
++        self.assertThat(entry.subtitle, Equals(highlighted))
++
++    def test_search_suggestions(self):
++        self.address_bar.write('high')
++        suggestions = self.main_window.get_suggestions()
++        self.assertThat(suggestions.count, Eventually(Equals(1)))
++        entries = suggestions.get_ordered_entries()
++        highlighted = self.highlight_term("highlight", "high")
++        self.assertThat(entries[0].title, Equals(highlighted))
++        self.assertThat(entries[0].subtitle, Equals(''))

webbrowser-app

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

Commit Message

Description of the Change

Unmerged revisions

Preview Diff

Subscribers