webbrowser-app

CMakeLists.txt (+2/-0)
src/app/config.h.in (+5/-0)
src/app/webbrowser/Browser.qml (+26/-5)
src/app/webbrowser/SearchSuggestions.qml (+78/-0)
src/app/webbrowser/Suggestion.qml (+5/-2)
src/app/webbrowser/Suggestions.qml (+22/-11)
src/app/webbrowser/searchengine.cpp (+13/-1)
src/app/webbrowser/searchengine.h (+4/-0)
tests/autopilot/webbrowser_app/tests/__init__.py (+6/-6)
tests/autopilot/webbrowser_app/tests/http_server.py (+14/-1)
tests/autopilot/webbrowser_app/tests/test_suggestions.py (+97/-28)

To merge this branch:

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

High

Fix Released

Link a bug report

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

This proposal supersedes a proposal from 2015-04-22.

Commit Message

Add suggestions from search engines in the suggestions list.

Description of the Change

Add suggestions from search engines in the suggestions list.

Ugo Riboni (uriboni) wrote on 2015-04-22: Posted in a previous version of this proposal

Added inline comments

PS Jenkins bot (ps-jenkins) wrote on 2015-04-22: Posted in a previous version of this proposal

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: Posted in a previous version of this proposal

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: Posted in a previous version of this proposal

From a very quick functional test, this seems to work nicely (I’ll do more in-depth testing and a code review later).

One thing that I noticed is that the terms are not highlighted in search suggestions, as they are for history entries and bookmarks. According to the visual spec they should be.

review: Needs Fixing

Olivier Tilloy (osomon) wrote on 2015-04-22: Posted in a previous version of this proposal

« This API is not official, but it is what Google products and everyone else uses to implement search suggestions. The "client=firefox" is there to request JSON output. »

Can you add this as a comment in config.h.in ?

PS Jenkins bot (ps-jenkins) wrote on 2015-04-22: Posted in a previous version of this proposal

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

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-04-23: Posted in a previous version of this proposal

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)

PS Jenkins bot (ps-jenkins) wrote on 2015-04-23: Posted in a previous version of this proposal

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: Posted in a previous version of this proposal

I’m seeing one funny behaviour: I’m adding a custom search engine definition (https://duckduckgo.com/opensearch.xml) and making it the default search engine, but the suggestions URL doesn’t seem to be used, instead the default suggestions from Google are used.

There are two things to fix here:
- that XML description file seems to have a valid suggestions URL, for some reason it’s not being parsed/used, it should be
- when loading a valid XML description file that doesn’t have a suggestions URL, we shouldn’t fall back to Google, instead there shouldn’t be any search suggestions

review: Needs Fixing

Olivier Tilloy (osomon) wrote on 2015-04-24: Posted in a previous version of this proposal

When launching the app, the address bar is not focused, so the suggestions list is not shown, yet I’m seeing a query issued to the suggestions URL with the current URL. There shouldn’t be any queries issued until the address bar has focus and its contents change.

review: Needs Fixing

PS Jenkins bot (ps-jenkins) wrote on 2015-04-27: Posted in a previous version of this proposal

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: Posted in a previous version of this proposal

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

review: Needs Fixing (continuous-integration)

Olivier Tilloy (osomon) on 2015-04-28: Posted in a previous version of this proposal

review: Approve

PS Jenkins bot (ps-jenkins) wrote on 2015-04-28: Posted in a previous version of this proposal

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: Posted in a previous version of this proposal

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

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-05-06: Posted in a previous version of this proposal

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: Posted in a previous version of this proposal

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: Posted in a previous version of this proposal

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: Posted in a previous version of this proposal

A few minor additional remarks:

In SearchSuggestions.qml:
- QQuickItem already has an 'enabled' property, I don’t think it’s necessary to override its definition
- in resetSearch(), the request (if any) should be aborted even if !enabled

A suggestion (pun not intended): in Browser.qml, in the two instances of LimitProxyModel that populate the list of suggestions for history entries and bookmarks, properties 'icon' and 'displayUrl' can be marked 'readonly'.

Similarly, in Suggestions.qml, the new properties added can be marked readonly.

In test_suggestions.py:
  - in setup_suggestions_source(), you should be able to replace "http://localhost:{}/" by "http://test/", as there is automatic mapping of test:80 requests to localhost:port
  - In highlight_term(), this function won’t work if there’s more than one occurrence of term in text. This is arguably unimportant for the scope of the tests, but the function could be simplified (and made to work in all cases) by simply returning "<html>{}</html>".format(re.sub(term, lambda m: self.highlight(m.group(0)), text))
  - in test_search_suggestions(), is the sleep really necessary?

Olivier Tilloy (osomon) wrote on 2015-05-12: Posted in a previous version of this proposal

There is now a (trivial) conflict when merging this branch into the latest trunk.

test_suggestions.py will require a bit of refactoring to make use of the newly-introduced self.data_location (revision 994) to write test search engine descriptions.

Ugo Riboni (uriboni) wrote on 2015-05-12: Posted in a previous version of this proposal

All suggestions implemented except for the following:

> In SearchSuggestions.qml:
> - QQuickItem already has an 'enabled' property, I don’t think it’s necessary
> to override its definition

Item.enabled has a completely different meaning tied to keyboard focus, and re-using it to do something else is wrong (and actually does not ever work since the property is toggled on and off various times by QML itself).
It was my mistake to redefine it though, as the original can't be accessed any more, so I defined a new "active" property that does the same thing, while leaving Item.enabled alone and with its original meaning.

> In test_suggestions.py:
> - in setup_suggestions_source(), you should be able to replace
> "http://localhost:{}/" by "http://test/", as there is automatic mapping of
> test:80 requests to localhost:port

Is this a DNS mapping or a redirect of some sort ? Because it does not seem to work when using XmlHttpRequest from QML to get the list of suggestions. The responseText comes out empty, which is the way XmlHttpRequest signals errors, but I can't tell more details on why.
I spent way too much time on this already, so I will leave it as it was for now.

> - In highlight_term(), this function won’t work if there’s more than one
> occurrence of term in text. This is arguably unimportant for the scope of the
> tests, but the function could be simplified (and made to work in all cases) by
> simply returning "<html>{}</html>".format(re.sub(term, lambda m:
> self.highlight(m.group(0)), text))

This does not work. In some cases I get a complain from re.py of "unbalanced parenthesis". Unless you have a quick fix I would leave it as it is.

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

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

> It was my mistake to redefine it though, as the original can't be accessed
> any more, so I defined a new "active" property that does the same thing,
> while leaving Item.enabled alone and with its original meaning.

Yeah, that’s better and safer indeed.

> Is this a DNS mapping or a redirect of some sort ? Because it does not seem
> to work when using XmlHttpRequest from QML to get the list of suggestions.

This is oxide-specific (see Oxide.WebContext.hostMappingRules), so indeed it won’t work with XmlHttpRequest. Sorry for putting you on a false trail, I hadn’t realized the requests were not going through the WebView.

> This does not work. In some cases I get a complain from re.py of
> "unbalanced parenthesis". Unless you have a quick fix I would leave
> it as it is.

Yeah, that’s fine.

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

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

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

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

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

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

review: Needs Fixing (continuous-integration)

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

review: Approve

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 'CMakeLists.txt'
 --- CMakeLists.txt	2015-02-04 10:56:38 +0000
 +++ CMakeLists.txt	2015-05-12 13:40:34 +0000
@@ -1,6 +1,8 @@
  cmake_minimum_required(VERSION 2.8.9)
  set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
++project("webbrowser")
++
  find_program(INTLTOOL_MERGE intltool-merge)
  if(NOT INTLTOOL_MERGE)
      message(FATAL_ERROR "Could not find intltool-merge, please install the intltool package")
 === 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 13:40:34 +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 13:40:34 +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 13:40:34 +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 13:40:34 +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 13:40:34 +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 13:40:34 +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 13:40:34 +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-05-12 13:40:34 +0000
 +++ tests/autopilot/webbrowser_app/tests/__init__.py	2015-05-12 13:40:34 +0000
@@ -166,8 +166,8 @@
          self.assertThat(lambda: len(self.main_window.get_webviews()),
                          Eventually(Equals(count)))
--    def ping_server(self):
--        url = "http://localhost:{}/ping".format(self.server.port)
++    def ping_server(self, server):
++        url = "http://localhost:{}/ping".format(server.port)
          ping = urllib.request.urlopen(url)
          self.assertThat(ping.read(), Equals(b"pong"))
@@ -185,12 +185,12 @@
      """
      def setUp(self):
--        self.server = http_server.HTTPServerInAThread()
--        self.ping_server()
--        self.addCleanup(self.server.cleanup)
++        self.http_server = http_server.HTTPServerInAThread()
++        self.ping_server(self.http_server)
++        self.addCleanup(self.http_server.cleanup)
          self.useFixture(fixtures.EnvironmentVariable(
              'UBUNTU_WEBVIEW_HOST_MAPPING_RULES',
--            "MAP test:80 localhost:{}".format(self.server.port)))
++            "MAP test:80 localhost:{}".format(self.http_server.port)))
          self.base_url = "http://test"
          self.url = self.base_url + "/test1"
          self.ARGS = self.ARGS + [self.url]
 === 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 13:40:34 +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 13:40:34 +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.suggest_http_server = http_server.HTTPServerInAThread()
++        self.ping_server(self.suggest_http_server)
++        self.addCleanup(self.suggest_http_server.cleanup)
++
++        self.create_temporary_profile()
++        self.setup_suggestions_source(self.suggest_http_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(''))