[Merge] lp:~uriboni/webbrowser-app/keyboard-navigation into lp:webbrowser-app

Riccardo Padovani riccardo at rpadovani.com
Thu Jun 11 23:05:20 UTC 2015


Thanks for working on this, now the desktop experience is definitely better!

I leave some impression, don't know if you want to fix in this branch or in a latter one

Some comments: 
- I expect to navigate the history with arrows after I open it with CTRL+H
- If I navigate to a page I can scroll with arrows (good!), then I open history with CTRL+H, then I press ESC, arrows don't do anything anymore. I expect they still scroll the page

Some more keybinding I would like:
- CTRL+SHIFT+T to reopen the last closed tab
- ALT+F (as on Chromium) to open the menu and then arrows to navigate it
- F11 to go fullscreen (but this I think requires some other works)

I took a fast look to the code and, apart of a little typo I left a comment inline, looks good to me, except Python I didn't review due my lack of knowledge of Python.

Cool work!

Diff comments:

> === modified file 'src/app/webbrowser/AddressBar.qml'
> --- src/app/webbrowser/AddressBar.qml	2015-05-26 21:42:54 +0000
> +++ src/app/webbrowser/AddressBar.qml	2015-06-10 11:24:31 +0000
> @@ -37,6 +37,7 @@
>      signal requestReload()
>      signal requestStop()
>      property string searchUrl
> +    property bool preventSimplifyText: false
>  
>      property var securityStatus: null
>  
> @@ -48,6 +49,8 @@
>  
>      height: textField.height
>  
> +    function selectAll() { textField.selectAll() }
> +
>      TextField {
>          id: textField
>          objectName: "addressBarTextField"
> @@ -84,7 +87,7 @@
>                      height: parent.height
>                      width: height
>  
> -                    visible: addressbar.activeFocus || addressbar.loading || !addressbar.text
> +                    visible: addressbar.activeFocus || addressbar.loading || !addressbar.text || preventSimplifyText
>  
>                      enabled: addressbar.text
>                      opacity: enabled ? 1.0 : 0.3
> @@ -229,7 +232,7 @@
>      QtObject {
>          id: internal
>  
> -        readonly property bool idle: !addressbar.loading && !addressbar.activeFocus
> +        readonly property bool idle: !addressbar.loading && !addressbar.activeFocus && !addressbar.preventSimplifyText
>  
>          readonly property int securityLevel: addressbar.securityStatus ? addressbar.securityStatus.securityLevel : Oxide.SecurityStatus.SecurityLevelNone
>          readonly property bool secureConnection: addressbar.securityStatus ? (securityLevel == Oxide.SecurityStatus.SecurityLevelSecure || securityLevel == Oxide.SecurityStatus.SecurityLevelSecureEV || securityLevel == Oxide.SecurityStatus.SecurityLevelWarning) : false
> @@ -291,7 +294,7 @@
>          }
>      }
>  
> -    onActiveFocusChanged: {
> +    function updateUrlFromFocus() {
>          if (activeFocus) {
>              text = actualUrl
>          } else if (!loading && actualUrl.toString()) {
> @@ -299,6 +302,9 @@
>          }
>      }
>  
> +    onActiveFocusChanged: if (!preventSimplifyText) updateUrlFromFocus()
> +    onPreventSimplifyTextChanged: if (!preventSimplifyText) updateUrlFromFocus()
> +
>      onActualUrlChanged: {
>          if (!activeFocus || !actualUrl.toString()) {
>              text = internal.simplifyUrl(actualUrl)
> 
> === modified file 'src/app/webbrowser/Browser.qml'
> --- src/app/webbrowser/Browser.qml	2015-06-02 14:26:38 +0000
> +++ src/app/webbrowser/Browser.qml	2015-06-10 11:24:31 +0000
> @@ -303,7 +303,10 @@
>                      text: i18n.tr("History")
>                      iconName: "history"
>                      enabled: browser.historyModel
> -                    onTriggered: historyViewComponent.createObject(historyViewContainer)
> +                    onTriggered: {
> +                        historyViewComponent.createObject(historyViewContainer)
> +                        historyViewContainer.focus = true
> +                    }
>                  },
>                  Action {
>                      objectName: "tabs"
> @@ -313,6 +316,7 @@
>                      onTriggered: {
>                          recentView.state = "shown"
>                          recentToolbar.state = "shown"
> +                        recentView.focus = true
>                      }
>                  },
>                  Action {
> @@ -326,7 +330,10 @@
>                      objectName: "settings"
>                      text: i18n.tr("Settings")
>                      iconName: "settings"
> -                    onTriggered: settingsComponent.createObject(settingsContainer)
> +                    onTriggered: {
> +                        settingsComponent.createObject(settingsContainer)
> +                        settingsContainer.focus = true
> +                    }
>                  },
>                  Action {
>                      objectName: "privatemode"
> @@ -346,6 +353,10 @@
>                      }
>                  }
>              ]
> +
> +            addressBarPreventSimplifyText: activeFocus || suggestionsList.activeFocus
> +            Keys.onDownPressed: if (suggestionsList.count) suggestionsList.focus = true
> +            Keys.onEscapePressed: internal.resetFocus()
>          }
>  
>          ChromeController {
> @@ -358,7 +369,7 @@
>  
>          Suggestions {
>              id: suggestionsList
> -            opacity: ((chrome.state == "shown") && chrome.activeFocus && (count > 0) && !chrome.drawerOpen) ? 1.0 : 0.0
> +            opacity: ((chrome.state == "shown") && (activeFocus || chrome.activeFocus) && count > 0 && !chrome.drawerOpen) ? 1.0 : 0.0
>              Behavior on opacity {
>                  UbuntuNumberAnimation {}
>              }
> @@ -372,6 +383,9 @@
>  
>              searchTerms: chrome.text.split(/\s+/g).filter(function(term) { return term.length > 0 })
>  
> +            Keys.onUpPressed: chrome.focus = true
> +            Keys.onEscapePressed: internal.resetFocus()
> +
>              models: [historySuggestions,
>                       bookmarksSuggestions,
>                       searchSuggestions.limit(4)]
> @@ -404,7 +418,7 @@
>                  id: searchSuggestions
>                  terms: suggestionsList.searchTerms
>                  searchEngine: currentSearchEngine
> -                active: chrome.activeFocus &&
> +                active: (chrome.activeFocus || suggestionsList.activeFocus) &&
>                           !browser.incognito &&
>                           !UrlManagement.looksLikeAUrl(chrome.text.replace(/ /g, "+"))
>  
> @@ -416,7 +430,7 @@
>                  }
>              }
>  
> -            onSelected: {
> +            onActivated: {
>                  browser.currentWebview.url = url
>                  browser.currentWebview.forceActiveFocus()
>                  chrome.requestedUrl = url
> @@ -426,6 +440,7 @@
>  
>      FocusScope {
>          id: recentView
> +        objectName: "recentView"
>  
>          anchors.fill: parent
>          visible: bottomEdgeHandle.dragging || tabslist.animating || (state == "shown")
> @@ -434,6 +449,13 @@
>              name: "shown"
>          }
>  
> +        function closeAndSwitchToTab(index) {
> +            recentView.reset()
> +            internal.switchToTab(index)
> +        }
> +
> +        Keys.onEscapePressed: closeAndSwitchToTab(0)
> +
>          TabsList {
>              id: tabslist
>              anchors.fill: parent
> @@ -453,15 +475,7 @@
>                  }
>              }
>              chromeOffset: chrome.height - invisibleTabChrome.height
> -            onTabSelected: {
> -                var tab = tabsModel.get(index)
> -                if (tab) {
> -                    tab.load()
> -                    tab.forceActiveFocus()
> -                    tabslist.model.setCurrent(index)
> -                }
> -                recentView.reset()
> -            }
> +            onTabSelected: recentView.closeAndSwitchToTab(index)
>              onTabClosed: {
>                  var tab = tabsModel.remove(index)
>                  if (tab) {
> @@ -497,10 +511,7 @@
>  
>                  text: i18n.tr("Done")
>  
> -                onClicked: {
> -                    recentView.reset()
> -                    tabsModel.currentTab.load()
> -                }
> +                onClicked: recentView.closeAndSwitchToTab(0)
>              }
>  
>              ToolbarAction {
> @@ -600,8 +611,9 @@
>          }
>      }
>  
> -    Item {
> +    FocusScope {
>          id: historyViewContainer
> +        objectName: "historyView"
>  
>          visible: children.length > 0
>          anchors.fill: parent
> @@ -612,6 +624,9 @@
>              HistoryView {
>                  anchors.fill: parent
>                  visible: historyViewContainer.children.length == 1
> +                focus: true
> +
> +                Keys.onEscapePressed: destroy()
>  
>                  Timer {
>                      // Set the model asynchronously to ensure
> @@ -650,7 +665,7 @@
>          }
>      }
>  
> -    Item {
> +    FocusScope {
>          id: settingsContainer
>  
>          visible: children.length > 0
> @@ -661,9 +676,11 @@
>  
>              SettingsPage {
>                  anchors.fill: parent
> +                focus: true
>                  historyModel: browser.historyModel
>                  settingsObject: settings
>                  onDone: destroy()
> +                Keys.onEscapePressed: destroy()
>              }
>          }
>      }
> @@ -920,9 +937,20 @@
>              }
>          }
>  
> -        function focusAddressBar() {
> +        function switchToTab(index) {
> +            var tab = tabsModel.get(index)
> +            if (tab) {
> +                tab.load()
> +                tabslist.model.setCurrent(index)
> +                if (tab.initialUrl == "" && formFactor == "desktop") focusAddressBar()
> +                else tab.forceActiveFocus()
> +            }
> +        }
> +
> +        function focusAddressBar(selectContent) {
>              chrome.forceActiveFocus()
>              Qt.inputMethod.show() // work around http://pad.lv/1316057
> +            if (selectContent) chrome.addressBarSelectAll()
>          }
>  
>          function resetFocus() {
> @@ -1128,4 +1156,132 @@
>              }
>          }
>      }
> +
> +    Keys.onPressed: {
> +        if (!chrome.visible && !recentView.visible) return;
> +
> +        if (event.modifiers & Qt.ControlModifier) {
> +            switch (event.key) {
> +            case Qt.Key_Tab:
> +                // Ctrl + Tab: pull the tab from the bottom of the stack to the
> +                // top (i.e. make it current)
> +                internal.switchToTab(tabsModel.count - 1)
> +                if (chrome.visible) recentView.reset()
> +                else if (recentView.visible) recentView.focus = true;
> +                event.accepted = true;
> +                break;
> +
> +            case Qt.Key_W:
> +            case Qt.Key_F4:
> +                // Ctrl + w or Ctrl+F4: Close the current tab
> +                if (tabsModel.count >= 0) {
> +                   var tab = tabsModel.remove(0);
> +                   if (tab) {
> +                       tab.close()
> +                   }
> +
> +                   if (tabsModel.count === 0) {
> +                       browser.openUrlInNewTab("", true)
> +                   } else {
> +                       internal.switchToTab(0)
> +                   }
> +                   event.accepted = true;
> +                }
> +                break;
> +
> +            case Qt.Key_T:
> +                // Ctrl + t: Open a new Tab
> +                openUrlInNewTab("", true);
> +                if (recentView.visible) recentView.focus = true
> +                event.accepted = true;
> +                break;
> +           }
> +        }
> +
> +        if (!chrome.visible) return;
> +
> +        if (event.modifiers & Qt.ControlModifier) {
> +            switch(event.key) {
> +            case Qt.Key_L:
> +                // Ctrl + l: Select the content in the address bar
> +                internal.focusAddressBar(true);
> +                event.accepted = true;
> +                break;
> +
> +            case Qt.Key_R:
> +                // Ctrl + R: Reload current Tab
> +                if (currentWebview) {
> +                    currentWebview.reload()
> +                    event.accepted = true;
> +                }
> +                break;
> +
> +            case Qt.Key_D:
> +                // Ctrl + D: Toggle bookmarked state on current Tab
> +                if (currentWebview) {
> +                    if (bookmarksModel.contains(currentWebview.url)) {
> +                         bookmarksModel.remove(currentWebview.url)
> +                    } else {
> +                        bookmarksModel.add(currentWebview.url, currentWebview.title, currentWebview.title)

It should be 

bookmarksModel.add(currentWebview.url, currentWebview.title, currentWebview.icon)

> +                    }
> +                    event.accepted = true;
> +                }
> +                break;
> +
> +            case Qt.Key_H:
> +                // Ctrl + H: Show History
> +                if (historyViewContainer.children.length === 0) {
> +                    historyViewComponent.createObject(historyViewContainer);
> +                    historyViewContainer.focus = true;
> +                    event.accepted = true;
> +                }
> +                break;
> +            }
> +        } else if (event.modifiers & Qt.AltModifier) {
> +            switch(event.key) {
> +            case Qt.Key_Left:
> +                // Alt + Left Arrow: Goes to the previous page in history
> +                if (currentWebview && currentWebview.canGoBack) {
> +                    internal.resetFocus();
> +                    currentWebview.goBack();
> +                    event.accepted = true;
> +                }
> +                break;
> +
> +            case Qt.Key_Right:
> +                // Alt + Right Arrow: Goes to the previous page in history
> +                if (currentWebview && currentWebview.canGoForward) {
> +                    internal.resetFocus();
> +                    currentWebview.goForward();
> +                    event.accepted = true;
> +                }
> +                break;
> +
> +            case Qt.Key_D:
> +                // Alt + d: Select the content in the address bar
> +                internal.focusAddressBar(true);
> +                event.accepted = true;
> +                break;
> +            }
> +
> +        } else if (event.modifiers & Qt.ShiftModifier) {
> +            switch(event.key) {
> +            case Qt.Key_Backspace:
> +                // Shift + Backspace: Goes to the next page in history
> +                if (currentWebview && currentWebview.canGoForward) {
> +                    internal.resetFocus();
> +                    currentWebview.goForward();
> +                    event.accepted = true;
> +                }
> +                break;
> +            }
> +        } else if (event.key === Qt.Key_Backspace) {
> +            // Backspace: Goes to the previous page in history
> +            if (currentWebview && currentWebview.canGoBack) {
> +                internal.resetFocus();
> +                currentWebview.goBack();
> +                event.accepted = true;
> +            }
> +        }
> +    }
>  }
> 
> === modified file 'src/app/webbrowser/Chrome.qml'
> --- src/app/webbrowser/Chrome.qml	2015-05-26 21:42:54 +0000
> +++ src/app/webbrowser/Chrome.qml	2015-06-10 11:24:31 +0000
> @@ -29,10 +29,13 @@
>      property list<Action> drawerActions
>      readonly property bool drawerOpen: internal.openDrawer
>      property alias requestedUrl: addressbar.requestedUrl
> +    property alias addressBarPreventSimplifyText: addressbar.preventSimplifyText
>      property alias incognito: addressbar.incognito
>  
>      backgroundColor: incognito ? UbuntuColors.darkGrey : Theme.palette.normal.background
>  
> +    function addressBarSelectAll() { addressbar.selectAll() }
> +
>      FocusScope {
>          anchors {
>              fill: parent
> 
> === modified file 'src/app/webbrowser/Suggestion.qml'
> --- src/app/webbrowser/Suggestion.qml	2015-04-22 11:00:02 +0000
> +++ src/app/webbrowser/Suggestion.qml	2015-06-10 11:24:31 +0000
> @@ -28,7 +28,7 @@
>      property alias icon: icon.name
>      property url url
>  
> -    signal selected(url url)
> +    signal activated(url url)
>  
>      __height: Math.max(middleVisuals.height, units.gu(6))
>      // disable focus handling
> @@ -77,8 +77,9 @@
>              fontSize: "small"
>              elide: Text.ElideRight
>              visible: text !== ""
> +            color: selected ? "#DB4923" : "black"
>          }
>      }
>  
> -    onClicked: selected(url)
> +    onClicked: activated(url)
>  }
> 
> === modified file 'src/app/webbrowser/Suggestions.qml'
> --- src/app/webbrowser/Suggestions.qml	2015-05-12 09:56:40 +0000
> +++ src/app/webbrowser/Suggestions.qml	2015-06-10 11:24:31 +0000
> @@ -19,20 +19,24 @@
>  import QtQuick 2.0
>  import Ubuntu.Components 1.1
>  
> -Rectangle {
> +FocusScope {
>      id: suggestions
>  
>      property var searchTerms
>      property var models
> +
>      readonly property int count: models.reduce(countItems, 0)
>      readonly property alias contentHeight: suggestionsList.contentHeight
>  
> -    signal selected(url url)
> +    signal activated(url url)
>  
> -    radius: units.gu(0.5)
> -    border {
> -        color: "#dedede"
> -        width: 1
> +    Rectangle {
> +        anchors.fill: parent
> +        radius: units.gu(0.5)
> +        border {
> +            color: "#dedede"
> +            width: 1
> +        }
>      }
>  
>      clip: true
> @@ -40,39 +44,37 @@
>      ListView {
>          id: suggestionsList
>          anchors.fill: parent
> -
> -        model: suggestions.models
> -        delegate: Column {
> -            id: suggestionsSection
> +        focus: true
> +
> +        model: models.reduce(function(list, model) {
> +            var modelItems = [];
> +
> +            // Models inheriting from QAbstractItemModel and JS arrays expose their
> +            // data differently, so we need to collect their items differently
> +            if (model.forEach) {
> +                model.forEach(function(item) { modelItems.push(item) })
> +            } else {
> +                for (var i = 0; i < model.count; i++) modelItems.push(model.get(i))
> +            }
> +
> +            modelItems.forEach(function(item) {
> +                item["icon"] = model.icon
> +                item["displayUrl"] = model.displayUrl
> +                list.push(item);
> +            })
> +            return list;
> +        }, [])
> +
> +        delegate: Suggestion {
>              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
> -
> -                delegate: Suggestion {
> -                    id: suggestion
> -                    width: suggestionsList.width
> -                    showDivider: suggestionsSection.firstItemIndex + index <
> -                                 suggestions.count - 1
> -
> -                    // 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)
> -                }
> -            }
> +            showDivider: index < model.length - 1
> +
> +            title: highlightTerms(modelData.title)
> +            subtitle: modelData.displayUrl ? highlightTerms(modelData.url) : ""
> +            icon: modelData.icon
> +            selected: suggestionsList.activeFocus && ListView.isCurrentItem
> +
> +            onActivated: suggestions.activated(modelData.url)
>          }
>      }
>  
> 
> === modified file 'src/app/webbrowser/limit-proxy-model.cpp'
> --- src/app/webbrowser/limit-proxy-model.cpp	2014-07-04 12:30:09 +0000
> +++ src/app/webbrowser/limit-proxy-model.cpp	2015-06-10 11:24:31 +0000
> @@ -266,3 +266,19 @@
>          m_dataChangedEnd = -1;
>      }
>  }
> +
> +QVariantMap LimitProxyModel::get(int i) const
> +{
> +    QVariantMap item;
> +    QHash<int, QByteArray> roles = roleNames();
> +
> +    QModelIndex modelIndex = index(i, 0);
> +    if (modelIndex.isValid()) {
> +        Q_FOREACH(int role, roles.keys()) {
> +            QString roleName = QString::fromUtf8(roles.value(role));
> +            item.insert(roleName, data(modelIndex, role));
> +        }
> +    }
> +    return item;
> +}
> +
> 
> === modified file 'src/app/webbrowser/limit-proxy-model.h'
> --- src/app/webbrowser/limit-proxy-model.h	2014-06-27 20:29:52 +0000
> +++ src/app/webbrowser/limit-proxy-model.h	2015-06-10 11:24:31 +0000
> @@ -45,6 +45,8 @@
>      int rowCount(const QModelIndex &parent = QModelIndex()) const;
>      int unlimitedRowCount(const QModelIndex &parent = QModelIndex()) const;
>  
> +    Q_INVOKABLE QVariantMap get(int index) const;
> +
>  Q_SIGNALS:
>      void sourceModelChanged() const;
>      void limitChanged() const;
> 
> === modified file 'tests/autopilot/webbrowser_app/emulators/browser.py'
> --- tests/autopilot/webbrowser_app/emulators/browser.py	2015-05-28 13:56:52 +0000
> +++ tests/autopilot/webbrowser_app/emulators/browser.py	2015-06-10 11:24:31 +0000
> @@ -19,7 +19,7 @@
>  import autopilot.logging
>  import ubuntuuitoolkit as uitk
>  from autopilot import exceptions
> -
> +from autopilot import input
>  
>  logger = logging.getLogger(__name__)
>  
> @@ -30,6 +30,7 @@
>          super().__init__(*args)
>          self.chrome = self._get_chrome()
>          self.address_bar = self.chrome.address_bar
> +        self.keyboard = input.Keyboard.create()
>  
>      def _get_chrome(self):
>          return self.select_single(Chrome)
> @@ -150,6 +151,17 @@
>      def get_bottom_edge_hint(self):
>          return self.select_single("QQuickImage", objectName="bottomEdgeHint")
>  
> +    # The history view is dynamically created, so it might or might not be
> +    # available
> +    def get_history_view(self):
> +        try:
> +            return self.select_single("HistoryView")
> +        except exceptions.StateNotFoundError:
> +            return None
> +
> +    def press_key(self, key):
> +        self.keyboard.press_and_release(key)
> +
>  
>  class Chrome(uitk.UbuntuUIToolkitCustomProxyObjectBase):
>  
> @@ -224,11 +236,14 @@
>      @autopilot.logging.log_action(logger.info)
>      def go_to_url(self, url):
>          self.write(url)
> -        self.text_field.keyboard.press_and_release('Enter')
> +        self.press_key('Enter')
>  
>      def write(self, text, clear=True):
>          self.text_field.write(text, clear)
>  
> +    def press_key(self, key):
> +        self.text_field.keyboard.press_and_release(key)
> +
>      @autopilot.logging.log_action(logger.info)
>      def click_action_button(self):
>          button = self.select_single("QQuickMouseArea",
> 
> === modified file 'tests/autopilot/webbrowser_app/tests/__init__.py'
> --- tests/autopilot/webbrowser_app/tests/__init__.py	2015-05-25 19:17:25 +0000
> +++ tests/autopilot/webbrowser_app/tests/__init__.py	2015-06-10 11:24:31 +0000
> @@ -169,6 +169,14 @@
>          self.pointing_device.click_object(settings_action)
>          return self.main_window.get_settings_page()
>  
> +    def open_history(self):
> +        chrome = self.main_window.chrome
> +        drawer_button = chrome.get_drawer_button()
> +        self.pointing_device.click_object(drawer_button)
> +        chrome.get_drawer()
> +        settings_action = chrome.get_drawer_action("history")
> +        self.pointing_device.click_object(settings_action)
> +
>      def assert_number_webviews_eventually(self, count):
>          self.assertThat(lambda: len(self.main_window.get_webviews()),
>                          Eventually(Equals(count)))
> @@ -195,7 +203,7 @@
>      are executed, thus making them more robust.
>      """
>  
> -    def setUp(self):
> +    def setUp(self, url="/test1"):
>          self.http_server = http_server.HTTPServerInAThread()
>          self.ping_server(self.http_server)
>          self.addCleanup(self.http_server.cleanup)
> @@ -203,7 +211,7 @@
>              'UBUNTU_WEBVIEW_HOST_MAPPING_RULES',
>              "MAP test:80 localhost:{}".format(self.http_server.port)))
>          self.base_url = "http://test"
> -        self.url = self.base_url + "/test1"
> +        self.url = self.base_url + url
>          self.ARGS = self.ARGS + [self.url]
>          super(StartOpenRemotePageTestCaseBase, self).setUp()
>          self.assert_home_page_eventually_loaded()
> 
> === modified file 'tests/autopilot/webbrowser_app/tests/http_server.py'
> --- tests/autopilot/webbrowser_app/tests/http_server.py	2015-04-23 15:34:16 +0000
> +++ tests/autopilot/webbrowser_app/tests/http_server.py	2015-06-10 11:24:31 +0000
> @@ -140,6 +140,10 @@
>              if query in self.suggestions_data:
>                  suggestions = self.suggestions_data[query]
>                  self.wfile.write(json.dumps(suggestions).encode())
> +        elif self.path.startswith("/tab/"):
> +            self.send_response(200)
> +            name = self.path[len("/tab/"):]
> +            self.send_html('<html><body>' + name + '</body></html>')
>          else:
>              self.send_error(404)
>  
> 
> === added file 'tests/autopilot/webbrowser_app/tests/test_keyboard.py'
> --- tests/autopilot/webbrowser_app/tests/test_keyboard.py	1970-01-01 00:00:00 +0000
> +++ tests/autopilot/webbrowser_app/tests/test_keyboard.py	2015-06-10 11:24:31 +0000
> @@ -0,0 +1,234 @@
> +# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
> +#
> +# Copyright 2015 Canonical
> +#
> +# This program is free software: you can redistribute it and/or modify it
> +# under the terms of the GNU General Public License version 3, as published
> +# by the Free Software Foundation.
> +#
> +# This program 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 os
> +import sqlite3
> +import time
> +import unittest
> +
> +from testtools.matchers import Equals, NotEquals, GreaterThan
> +from autopilot.matchers import Eventually
> +from autopilot.platform import model
> +
> +from webbrowser_app.tests import StartOpenRemotePageTestCaseBase
> +
> +
> +class PrepopulatedDatabaseTestCaseBase(StartOpenRemotePageTestCaseBase):
> +
> +    """Helper test class that pre-populates history and bookmarks databases."""
> +
> +    def setUp(self):
> +        self.create_temporary_profile()
> +        self.populate_bookmarks()
> +        super(PrepopulatedDatabaseTestCaseBase, self).setUp("/tab/0")
> +
> +    def populate_bookmarks(self):
> +        db_path = os.path.join(self.data_location, "bookmarks.sqlite")
> +        connection = sqlite3.connect(db_path)
> +        connection.execute("""CREATE TABLE IF NOT EXISTS bookmarks
> +                              (url VARCHAR, title VARCHAR, icon VARCHAR,
> +                              created INTEGER);""")
> +        rows = [
> +            ("http://www.rsc.org/periodic-table/element/77/iridium",
> +             "Iridium - Element Information")
> +        ]
> +
> +        for i, row in enumerate(rows):
> +            timestamp = int(time.time()) - i * 10
> +            query = "INSERT INTO bookmarks \
> +                     VALUES ('{}', '{}', '', {});"
> +            query = query.format(row[0], row[1], timestamp)
> +            connection.execute(query)
> +
> +        connection.commit()
> +        connection.close()
> +
> +
> +# Use PrepopulatedDatabaseTestCaseBase to ensure that at least one suggestion
> +# will appear in the suggestions menu by creating a bookmark we can search for
> + at unittest.skipIf(model() != "Desktop", "on desktop only")
> +class TestKeyboard(PrepopulatedDatabaseTestCaseBase):
> +
> +    """Test keyboard interaction"""
> +
> +    def setUp(self):
> +        super(TestKeyboard, self).setUp()
> +        self.address_bar = self.main_window.address_bar
> +
> +    def open_tab(self, url):
> +        self.main_window.press_key('Ctrl+T')
> +        new_tab_view = self.main_window.get_new_tab_view()
> +        self.address_bar.go_to_url(url)
> +        new_tab_view.wait_until_destroyed()
> +        self.assertThat(lambda: self.main_window.get_current_webview().url,
> +                        Eventually(Equals(url)))
> +
> +    # Name tabs starting from 1 by default because tab 0 has been opened
> +    # already via StartOpenRemotePageTestCaseBase
> +    def open_tabs(self, count, base=1):
> +        for i in range(0, count):
> +            self.open_tab(self.base_url + "/tab/" + str(i + base))
> +
> +    def check_tab_number(self, number):
> +        url = self.base_url + "/tab/" + str(number)
> +        self.assertThat(lambda: self.main_window.get_current_webview().url,
> +                        Eventually(Equals(url)))
> +
> +    def test_new_tab(self):
> +        self.main_window.press_key('Ctrl+T')
> +
> +        webview = self.main_window.get_current_webview()
> +        self.assertThat(webview.url, Equals(""))
> +        new_tab_view = self.main_window.get_new_tab_view()
> +        self.assertThat(new_tab_view.visible, Eventually(Equals(True)))
> +
> +    def test_switch_tabs(self):
> +        self.open_tabs(2)
> +        self.check_tab_number(2)
> +        self.main_window.press_key('Ctrl+Tab')
> +        self.check_tab_number(0)
> +        self.main_window.press_key('Ctrl+Tab')
> +        self.check_tab_number(1)
> +        self.main_window.press_key('Ctrl+Tab')
> +        self.check_tab_number(2)
> +
> +    def test_can_switch_tabs_after_suggestions_escape(self):
> +        self.open_tabs(1)
> +        self.check_tab_number(1)
> +
> +        suggestions = self.main_window.get_suggestions()
> +        self.address_bar.write('el')
> +        self.assertThat(suggestions.opacity, Eventually(Equals(1)))
> +        self.main_window.press_key('Down')
> +        self.assertThat(suggestions.activeFocus, Eventually(Equals(True)))
> +
> +        self.main_window.press_key('Escape')
> +        self.assertThat(suggestions.opacity, Eventually(Equals(0)))
> +
> +        self.main_window.press_key('Ctrl+Tab')
> +        self.check_tab_number(0)
> +
> +    def test_close_tabs_ctrl_f4(self):
> +        self.open_tabs(1)
> +        self.check_tab_number(1)
> +        self.main_window.press_key('Ctrl+F4')
> +        self.check_tab_number(0)
> +        self.main_window.press_key('Ctrl+F4')
> +        webview = self.main_window.get_current_webview()
> +        self.assertThat(webview.url, Equals(""))
> +
> +    def test_close_tabs_ctrl_w(self):
> +        self.open_tabs(1)
> +        self.check_tab_number(1)
> +        self.main_window.press_key('Ctrl+w')
> +        self.check_tab_number(0)
> +        self.main_window.press_key('Ctrl+w')
> +        webview = self.main_window.get_current_webview()
> +        self.assertThat(webview.url, Equals(""))
> +
> +    def test_select_address_bar_ctrl_l(self):
> +        self.main_window.press_key('Ctrl+L')
> +        self.assertThat(self.address_bar.text_field.selectedText,
> +                        Eventually(Equals(self.address_bar.text_field.text)))
> +
> +    def test_select_address_bar_alt_d(self):
> +        self.main_window.press_key('Alt+D')
> +        self.assertThat(self.address_bar.text_field.selectedText,
> +                        Eventually(Equals(self.address_bar.text_field.text)))
> +
> +    def test_escape_from_address_bar(self):
> +        self.main_window.press_key('Alt+D')
> +        self.assertThat(self.address_bar.text_field.selectedText,
> +                        Eventually(Equals(self.address_bar.text_field.text)))
> +        self.main_window.press_key('Escape')
> +        self.assertThat(self.address_bar.text_field.selectedText,
> +                        Eventually(Equals("")))
> +        self.assertThat(self.address_bar.activeFocus,
> +                        Eventually(Equals(False)))
> +
> +    def test_reload(self):
> +        webview = self.main_window.get_current_webview()
> +        self.assertThat(webview.loading, Eventually(Equals(False)))
> +
> +        watcher = webview.watch_signal('loadingStateChanged()')
> +        previous = watcher.num_emissions
> +
> +        self.main_window.press_key('Ctrl+R')
> +        self.assertThat(
> +            lambda: watcher.num_emissions,
> +            Eventually(GreaterThan(previous)))
> +
> +        self.assertThat(webview.loading, Eventually(Equals(False)))
> +
> +    def test_bookmark(self):
> +        chrome = self.main_window.chrome
> +        self.assertThat(chrome.bookmarked, Equals(False))
> +        self.main_window.press_key('Ctrl+D')
> +        self.assertThat(chrome.bookmarked, Eventually(Equals(True)))
> +        self.main_window.press_key('Ctrl+D')
> +        self.assertThat(chrome.bookmarked, Eventually(Equals(False)))
> +
> +    def test_history_navigation_with_alt_arrows(self):
> +        previous = self.main_window.get_current_webview().url
> +        url = self.base_url + "/test2"
> +        self.main_window.go_to_url(url)
> +        self.main_window.wait_until_page_loaded(url)
> +
> +        self.main_window.press_key('Alt+Left')
> +        self.assertThat(lambda: self.main_window.get_current_webview().url,
> +                        Eventually(Equals(previous)))
> +
> +        self.main_window.press_key('Alt+Right')
> +        self.assertThat(lambda: self.main_window.get_current_webview().url,
> +                        Eventually(Equals(url)))
> +
> +    def test_history_navigation_with_backspace(self):
> +        previous = self.main_window.get_current_webview().url
> +        url = self.base_url + "/test2"
> +        self.main_window.go_to_url(url)
> +        self.main_window.wait_until_page_loaded(url)
> +
> +        self.main_window.press_key('Backspace')
> +        self.assertThat(lambda: self.main_window.get_current_webview().url,
> +                        Eventually(Equals(previous)))
> +
> +        self.main_window.press_key('Shift+Backspace')
> +        self.assertThat(lambda: self.main_window.get_current_webview().url,
> +                        Eventually(Equals(url)))
> +
> +    def test_toggle_history(self):
> +        self.assertThat(self.main_window.get_history_view(), Equals(None))
> +        self.main_window.press_key('Ctrl+H')
> +        self.assertThat(lambda: self.main_window.get_history_view(),
> +                        Eventually(NotEquals(None)))
> +        history_view = self.main_window.get_history_view()
> +
> +        self.main_window.press_key('Escape')
> +        history_view.wait_until_destroyed()
> +
> +    def test_toggle_history_from_menu(self):
> +        self.assertThat(self.main_window.get_history_view(), Equals(None))
> +        self.open_history()
> +        history_view = self.main_window.get_history_view()
> +        self.assertThat(history_view.activeFocus, Eventually(Equals(True)))
> +
> +        self.main_window.press_key('Escape')
> +        history_view.wait_until_destroyed()
> +
> +    def test_escape_settings(self):
> +        settings = self.open_settings()
> +        self.main_window.press_key('Escape')
> +        settings.wait_until_destroyed()
> 
> === modified file 'tests/autopilot/webbrowser_app/tests/test_suggestions.py'
> --- tests/autopilot/webbrowser_app/tests/test_suggestions.py	2015-05-20 05:45:29 +0000
> +++ tests/autopilot/webbrowser_app/tests/test_suggestions.py	2015-06-10 11:24:31 +0000
> @@ -18,9 +18,11 @@
>  import random
>  import sqlite3
>  import time
> +import unittest
>  
> -from testtools.matchers import Contains, Equals
> +from testtools.matchers import Contains, Equals, NotEquals
>  from autopilot.matchers import Eventually
> +from autopilot.platform import model
>  
>  from webbrowser_app.tests import StartOpenRemotePageTestCaseBase
>  from . import http_server
> @@ -170,10 +172,9 @@
>      def test_show_list_of_suggestions(self):
>          suggestions = self.main_window.get_suggestions()
>          self.assert_suggestions_eventually_hidden()
> -        self.assert_suggestions_eventually_hidden()
>          self.address_bar.focus()
>          self.assert_suggestions_eventually_shown()
> -        self.assertThat(suggestions.count, Eventually(Equals(1)))
> +        self.assertThat(suggestions.count, Eventually(NotEquals(0)))
>          self.address_bar.clear()
>          self.assert_suggestions_eventually_hidden()
>  
> @@ -303,3 +304,67 @@
>          highlighted = self.highlight_term("highlight", "high")
>          self.assertThat(entries[0].title, Equals(highlighted))
>          self.assertThat(entries[0].subtitle, Equals(''))
> +
> +    @unittest.skipIf(model() != "Desktop", "on desktop only")
> +    def test_keyboard_movement(self):
> +        suggestions = self.main_window.get_suggestions()
> +        address_bar = self.address_bar
> +        address_bar.write('element')
> +        self.assert_suggestions_eventually_shown()
> +        self.assertThat(suggestions.count, Eventually(Equals(2)))
> +        entries = suggestions.get_ordered_entries()
> +        self.assertThat(entries[0].selected, Equals(False))
> +        self.assertThat(entries[1].selected, Equals(False))
> +
> +        address_bar.press_key('Down')
> +        self.assertThat(address_bar.activeFocus, Eventually(Equals(False)))
> +        self.assertThat(suggestions.activeFocus, Eventually(Equals(True)))
> +        self.assertThat(entries[0].selected, Equals(True))
> +
> +        self.main_window.press_key('Down')
> +        self.assertThat(entries[0].selected, Equals(False))
> +        self.assertThat(entries[1].selected, Equals(True))
> +
> +        # verify that selection does not wrap around
> +        self.main_window.press_key('Down')
> +        self.assertThat(entries[0].selected, Equals(False))
> +        self.assertThat(entries[1].selected, Equals(True))
> +
> +        self.main_window.press_key('Up')
> +        self.assertThat(entries[0].selected, Equals(True))
> +        self.assertThat(entries[1].selected, Equals(False))
> +
> +        self.main_window.press_key('Up')
> +        self.assertThat(address_bar.activeFocus, Eventually(Equals(True)))
> +        self.assertThat(suggestions.activeFocus, Eventually(Equals(False)))
> +        self.assertThat(entries[0].selected, Equals(False))
> +        self.assertThat(entries[1].selected, Equals(False))
> +
> +    @unittest.skipIf(model() != "Desktop", "on desktop only")
> +    def test_suggestions_escape(self):
> +        suggestions = self.main_window.get_suggestions()
> +        previous_text = self.address_bar.text
> +        self.address_bar.write('element')
> +        self.assert_suggestions_eventually_shown()
> +        self.main_window.press_key('Down')
> +        self.assertThat(suggestions.activeFocus, Eventually(Equals(True)))
> +        self.assertThat(self.address_bar.text, Equals("element"))
> +
> +        self.main_window.press_key('Escape')
> +        self.assert_suggestions_eventually_hidden()
> +        self.assertThat(self.address_bar.text, Equals(previous_text))
> +
> +    @unittest.skipIf(model() != "Desktop", "on desktop only")
> +    def test_suggestions_escape_on_addressbar(self):
> +        suggestions = self.main_window.get_suggestions()
> +        previous_text = self.address_bar.text
> +        self.address_bar.write('element')
> +        self.assert_suggestions_eventually_shown()
> +        self.main_window.press_key('Down')
> +        self.assertThat(suggestions.activeFocus, Eventually(Equals(True)))
> +        self.main_window.press_key('Up')
> +        self.assertThat(suggestions.activeFocus, Eventually(Equals(False)))
> +
> +        self.main_window.press_key('Escape')
> +        self.assert_suggestions_eventually_hidden()
> +        self.assertThat(self.address_bar.text, Equals(previous_text))
> 
> === modified file 'tests/unittests/limit-proxy-model/tst_LimitProxyModelTests.cpp'
> --- tests/unittests/limit-proxy-model/tst_LimitProxyModelTests.cpp	2015-05-19 10:58:04 +0000
> +++ tests/unittests/limit-proxy-model/tst_LimitProxyModelTests.cpp	2015-06-10 11:24:31 +0000
> @@ -157,6 +157,28 @@
>          QCOMPARE(model->unlimitedRowCount(), 3);
>          QCOMPARE(model->rowCount(), 2);
>      }
> +
> +    void shouldGetItemWithCorrectValues()
> +    {
> +        history->add(QUrl("http://example1.org/"), "Example 1 Domain", QUrl());
> +
> +        QVariantMap item = model->get(0);
> +        QHash<int, QByteArray> roles = model->roleNames();
> +
> +        QCOMPARE(roles.count(), item.count());
> +
> +        Q_FOREACH(int role, roles.keys()) {
> +            QString roleName = QString::fromUtf8(roles.value(role));
> +            QCOMPARE(model->data(model->index(0, 0), role), item.value(roleName));
> +        }
> +    }
> +
> +    void shouldReturnEmptyItemIfGetOutOfBounds()
> +    {
> +        QVariantMap item = model->get(1);
> +        QCOMPARE(item.count(), 0);
> +    }
> +
>  };
>  
>  QTEST_MAIN(LimitProxyModelTests)
> 


-- 
https://code.launchpad.net/~uriboni/webbrowser-app/keyboard-navigation/+merge/260183
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app.



More information about the Ubuntu-reviews mailing list