Merge lp:~mhall119/webbrowser-app/alt-num-shortcut into lp:webbrowser-app

[Michael Hall]

Adds Alt+[0-9] KeyboartShortcut to switch between the first 10 tabs directly, similar to Chrome and Firefox.

Currently the tests fail, I'm not sure why, but the code works when I run it. The key press events seems to be falling through to the page in the tab though, which causes some pages like Facebook.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1203
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2286/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4366/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1040/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1040/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1040/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4363/console

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

review: Needs Fixing (continuous-integration)

[Riccardo Padovani (rpadovani)]

Thanks, works quite well.

Just a note: it doesn't work when the address bar is focused.

E.g.:
0. Start the browser
1. Click CTRL+T
2. Click ALT+1

I expect to return to first tab, but instead it inserts 1 in the address bar.

BTW, I don't find any documentation about KeyboardShortcut, nor in developer.ubuntu.com (actually, a grep in ubuntu-ui-toolkit return nothing) nor in developer.qt.io (but I didn't check the source code here) - where did you find it?

review: Needs Fixing

[Olivier Tilloy (osomon)]

@Riccardo: KeyboardShortcut is a custom item in webbrowser-app’s source tree: http://bazaar.launchpad.net/~phablet-team/webbrowser-app/trunk/view/head:/src/app/webbrowser/KeyboardShortcut.qml.

See also the KeyboardShortcuts item that is meant to contain all keyboard shortcuts: http://bazaar.launchpad.net/~phablet-team/webbrowser-app/trunk/view/head:/src/app/webbrowser/KeyboardShortcuts.qml.

[Olivier Tilloy (osomon)]

QmlTests::TabsBar::test_goto_tab_keyboardshortcut consistently fails. This is because keyboard shortcuts are not handled at the TabsBar level, but in the top-level Browser component. So either that unit test needs to be removed, or the definition of these shortcuts need to be encapsulated at the TabsBar level (probably a better approach).

On a related note, given that the definition of this kind of shortcuts results in very repetitive (and thus error-prone) code, it would be good to consider instantiating them dynamically in a Component.onCompleted handler.

review: Needs Fixing

Unmerged revisions

1203. By Michael Hall on 2015-09-25

Add alt+# keyboard shortcuts to go directly to a tab

=== modified file 'src/app/webbrowser/Browser.qml'
--- src/app/webbrowser/Browser.qml	2015-09-22 01:27:19 +0000
+++ src/app/webbrowser/Browser.qml	2015-09-25 17:45:14 +0000
@@ -1728,5 +1728,67 @@
                 chrome.focus = true
             }
         }
+
+        // Alt + [0-9]: Go to tab
+        KeyboardShortcut {
+            modifiers: Qt.AltModifier
+            key: Qt.Key_1
+            enabled: tabsModel.count >= 1
+            onTriggered: internal.switchToTab(0)
+        }
+        KeyboardShortcut {
+            modifiers: Qt.AltModifier
+            key: Qt.Key_2
+            enabled: tabsModel.count >= 2
+            onTriggered: internal.switchToTab(1)
+        }
+        KeyboardShortcut {
+            modifiers: Qt.AltModifier
+            key: Qt.Key_3
+            enabled: tabsModel.count >= 3
+            onTriggered: internal.switchToTab(2)
+        }
+        KeyboardShortcut {
+            modifiers: Qt.AltModifier
+            key: Qt.Key_4
+            enabled: tabsModel.count >= 4
+            onTriggered: internal.switchToTab(3)
+        }
+        KeyboardShortcut {
+            modifiers: Qt.AltModifier
+            key: Qt.Key_5
+            enabled: tabsModel.count >= 5
+            onTriggered: internal.switchToTab(4)
+        }
+        KeyboardShortcut {
+            modifiers: Qt.AltModifier
+            key: Qt.Key_6
+            enabled: tabsModel.count >= 6
+            onTriggered: internal.switchToTab(5)
+        }
+        KeyboardShortcut {
+            modifiers: Qt.AltModifier
+            key: Qt.Key_7
+            enabled: tabsModel.count >= 7
+            onTriggered: internal.switchToTab(6)
+        }
+        KeyboardShortcut {
+            modifiers: Qt.AltModifier
+            key: Qt.Key_8
+            enabled: tabsModel.count >= 8
+            onTriggered: internal.switchToTab(7)
+        }
+        KeyboardShortcut {
+            modifiers: Qt.AltModifier
+            key: Qt.Key_9
+            enabled: tabsModel.count >= 9
+            onTriggered: internal.switchToTab(8)
+        }
+        KeyboardShortcut {
+            modifiers: Qt.AltModifier
+            key: Qt.Key_0
+            enabled: tabsModel.count >= 10
+            onTriggered: internal.switchToTab(9)
+        }
     }
 }

=== modified file 'tests/autopilot/webbrowser_app/tests/test_keyboard.py'
--- tests/autopilot/webbrowser_app/tests/test_keyboard.py	2015-08-14 09:25:38 +0000
+++ tests/autopilot/webbrowser_app/tests/test_keyboard.py	2015-09-25 17:45:14 +0000
@@ -103,6 +103,16 @@
         self.main_window.press_key('Ctrl+Tab')
         self.check_tab_number(2)
 
+    def test_goto_tab(self):
+        self.open_tabs(2)
+        self.check_tab_number(2)
+        self.main_window.press_key('Alt+1')
+        self.check_tab_number(0)
+        self.main_window.press_key('Alt+2')
+        self.check_tab_number(1)
+        self.main_window.press_key('Alt+3')
+        self.check_tab_number(2)
+
     def test_can_switch_tabs_after_suggestions_escape(self):
         self.open_tabs(1)
         self.check_tab_number(1)

=== modified file 'tests/unittests/qml/tst_TabsBar.qml'
--- tests/unittests/qml/tst_TabsBar.qml	2015-08-10 15:22:00 +0000
+++ tests/unittests/qml/tst_TabsBar.qml	2015-09-25 17:45:14 +0000
@@ -103,6 +103,16 @@
             compare(newTabRequestSpy.count, 3)
         }
 
+        function test_goto_tab_keyboardshortcut() {
+            // Alt+[0-9] goes to the numbered tab
+            populateTabs()
+            var keys = [Qt.Key_1, Qt.Key_2, Qt.Key_3]
+            for (var i = 2; i >= 0; --i) {
+                keyClick(keys[i], Qt.AltModifier)
+                compare(tabsModel.currentIndex, i)
+            }
+        }
+
         function test_mouse_left_click() {
             // Left click makes the tab current
             populateTabs()