Merge lp:~osomon/webbrowser-app/autofocus-addressbar-wide into lp:webbrowser-app

[Olivier Tilloy]

Automatically focus the address bar when opening a new tab in wide mode (instead of inferring a "desktop" form factor).
This is not exactly the correct fix (which would require the QInputInfo API), but is better than the current situation anyway.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1329
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2569/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/6112
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1322
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1322
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1322/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1322
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-touch/4719
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6123
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6123/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26879

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

review: Needs Fixing (continuous-integration)

=== modified file 'src/app/webbrowser/Browser.qml'
--- src/app/webbrowser/Browser.qml	2016-01-12 14:37:11 +0000
+++ src/app/webbrowser/Browser.qml	2016-01-21 10:33:49 +0000
@@ -66,7 +66,7 @@
         onCurrentIndexChanged: {
             // Remove focus from the address bar when the current tab
             // changes to ensure that its contents are updated.
-            tabContainer.forceActiveFocus()
+            contentsContainer.forceActiveFocus()
 
             // In narrow mode, the tabslist is a stack:
             // the current tab is always at the top.
@@ -170,6 +170,7 @@
     }
 
     FocusScope {
+        id: contentsContainer
         anchors.fill: parent
         visible: !settingsViewLoader.active && !historyViewLoader.active && !bookmarksViewLoader.active && !downloadsViewLoader.active
 
@@ -181,9 +182,15 @@
                 top: parent.top
             }
             height: parent.height - osk.height
+
+            focus: !errorSheetLoader.focus &&
+                   !invalidCertificateErrorSheetLoader.focus &&
+                   !newTabViewLoader.focus &&
+                   !sadTabLoader.focus
         }
 
         Loader {
+            id: errorSheetLoader
             anchors {
                 fill: tabContainer
                 topMargin: (chrome.state == "shown") ? chrome.height : 0
@@ -193,10 +200,12 @@
                 url: currentWebview ? currentWebview.url : ""
                 onRefreshClicked: currentWebview.reload()
             }
+            focus: item.visible
             asynchronous: true
         }
 
         Loader {
+            id: invalidCertificateErrorSheetLoader
             anchors {
                 fill: tabContainer
                 topMargin: (chrome.state == "shown") ? chrome.height : 0
@@ -214,6 +223,7 @@
                     currentWebview.resetCertificateError()
                 }
             }
+            focus: item.visible
             asynchronous: true
         }
 
@@ -235,6 +245,7 @@
                 }
             }
             active: false
+            focus: active
             asynchronous: true
 
             Connections {
@@ -301,12 +312,14 @@
         }
 
         Loader {
+            id: sadTabLoader
             anchors {
                 fill: tabContainer
                 topMargin: (chrome.state == "shown") ? chrome.height : 0
             }
 
             active: webProcessMonitor.crashed || (webProcessMonitor.killed && !currentWebview.loading)
+            focus: active
 
             sourceComponent: SadTab {
                 webview: currentWebview
@@ -421,6 +434,7 @@
             state = ""
             recentToolbar.state = "hidden"
             tabslist.reset()
+            internal.resetFocus()
         }
     }
 
@@ -1489,9 +1503,8 @@
                 if (recentView.visible) {
                     recentView.focus = true
                 } else if (tab) {
-                    if (!tab.url.toString() && !tab.initialUrl.toString() &&
-                        (formFactor == "desktop")) {
-                        focusAddressBar()
+                    if (!tab.url.toString() && !tab.initialUrl.toString()) {
+                        maybeFocusAddressBar()
                     } else {
                         tabContainer.forceActiveFocus()
                     }
@@ -1507,14 +1520,31 @@
 
         function resetFocus() {
             if (browser.currentWebview) {
-                if (!browser.currentWebview.url.toString() && (formFactor == "desktop")) {
-                    internal.focusAddressBar()
+                if (!browser.currentWebview.url.toString()) {
+                    internal.maybeFocusAddressBar()
                 } else {
-                    tabContainer.forceActiveFocus()
+                    contentsContainer.forceActiveFocus()
                 }
             }
         }
 
+        function maybeFocusAddressBar() {
+            // XXX: this is not the right condition, but it is better than
+            // inferring a "desktop" form factor from various heuristics.
+            // The real fix is to detect whether there is a physical keyboard
+            // connected, for which there is currently no API yet (there will
+            // be a QInputInfo API in a future version of Qt).
+            // Wide mode might be in use on a device without a physical
+            // keyboard (e.g. a 10" tablet), and conversely the browser window
+            // might be shrinked to a narrow layout on a desktop setup with a
+            // physical keyboard and no OSK.
+            if (browser.wide) {
+                focusAddressBar()
+            } else {
+                contentsContainer.forceActiveFocus()
+            }
+        }
+
         // Invalid certificates the user has explicitly allowed for this session
         property var allowedCertificateErrors: []
 
@@ -1580,8 +1610,8 @@
         if (load) {
             tab.load()
         }
-        if (!url.toString() && (formFactor == "desktop")) {
-            internal.focusAddressBar()
+        if (!url.toString()) {
+            internal.maybeFocusAddressBar()
         }
     }
 
@@ -1741,8 +1771,8 @@
                 browser.openUrlInNewTab(settings.homepage, true, false)
             }
             tabsModel.currentTab.load()
-            if (!tabsModel.currentTab.url.toString() && !tabsModel.currentTab.restoreState && (formFactor == "desktop")) {
-                internal.focusAddressBar()
+            if (!tabsModel.currentTab.url.toString() && !tabsModel.currentTab.restoreState) {
+                internal.maybeFocusAddressBar()
             }
 
             BookmarksModel.databasePath = dataLocation + "/bookmarks.sqlite"
@@ -1784,7 +1814,7 @@
                 if (browser.incognito) {
                     browser.incognito = false
                     internal.resetFocus()
-                } else if ((formFactor == "desktop") || browser.wide) {
+                } else if (browser.wide) {
                     Qt.quit()
                 }
             }

=== modified file 'tests/autopilot/webbrowser_app/tests/__init__.py'
--- tests/autopilot/webbrowser_app/tests/__init__.py	2015-10-26 11:09:25 +0000
+++ tests/autopilot/webbrowser_app/tests/__init__.py	2016-01-21 10:33:49 +0000
@@ -1,6 +1,6 @@
 # -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
 #
-# Copyright 2013-2015 Canonical
+# Copyright 2013-2016 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
@@ -174,10 +174,9 @@
             self.assert_number_webviews_eventually(new_count)
             new_tab_view = self.main_window.get_new_tab_view()
 
-        if model() == 'Desktop':
-            self.assertThat(
-                self.main_window.address_bar.activeFocus,
-                Eventually(Equals(True)))
+        if self.main_window.wide:
+            self.assertThat(self.main_window.address_bar.activeFocus,
+                            Eventually(Equals(True)))
 
         if not self.main_window.wide and expand_view:
             more_button = new_tab_view.get_bookmarks_more_button()

=== modified file 'tests/autopilot/webbrowser_app/tests/test_keyboard.py'
--- tests/autopilot/webbrowser_app/tests/test_keyboard.py	2015-12-22 18:15:29 +0000
+++ tests/autopilot/webbrowser_app/tests/test_keyboard.py	2016-01-21 10:33:49 +0000
@@ -1,6 +1,6 @@
 # -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
 #
-# Copyright 2015 Canonical
+# Copyright 2015-2016 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
@@ -147,26 +147,28 @@
         self.check_tab_number(0)
 
     def test_close_tabs_ctrl_f4(self):
+        wide = self.main_window.wide
         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')
-        if model() == 'Desktop':
-            # On desktop, closing the last open tab exits the application
+        if wide:
+            # closing the last open tab exits the application
             self.app.process.wait()
         else:
             webview = self.main_window.get_current_webview()
             self.assertThat(webview.url, Equals(""))
 
     def test_close_tabs_ctrl_w(self):
+        wide = self.main_window.wide
         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')
-        if model() == 'Desktop':
-            # On desktop, closing the last open tab exits the application
+        if wide:
+            # closing the last open tab exits the application
             self.app.process.wait()
         else:
             webview = self.main_window.get_current_webview()
@@ -181,12 +183,8 @@
         self.main_window.press_key('Ctrl+w')
         self.check_tab_number(0)
         self.main_window.press_key('Ctrl+F4')
-        if model() == 'Desktop':
-            # On desktop, closing the last open tab exits the application
-            self.app.process.wait()
-        else:
-            webview = self.main_window.get_current_webview()
-            self.assertThat(webview.url, Equals(""))
+        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')

=== modified file 'tests/autopilot/webbrowser_app/tests/test_private.py'
--- tests/autopilot/webbrowser_app/tests/test_private.py	2015-11-23 17:12:15 +0000
+++ tests/autopilot/webbrowser_app/tests/test_private.py	2016-01-21 10:33:49 +0000
@@ -1,6 +1,6 @@
 # -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
 #
-# Copyright 2015 Canonical
+# Copyright 2015-2016 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
@@ -16,7 +16,6 @@
 
 from testtools.matchers import Equals, NotEquals
 from autopilot.matchers import Eventually
-from autopilot.platform import model
 
 from webbrowser_app.tests import StartOpenRemotePageTestCaseBase
 
@@ -31,7 +30,7 @@
         self.assert_number_incognito_webviews_eventually(1)
         self.assertTrue(self.main_window.is_new_private_tab_view_visible())
         self.assertThat(address_bar.activeFocus,
-                        Eventually(Equals(model() == 'Desktop')))
+                        Eventually(Equals(self.main_window.wide)))
         self.assertThat(address_bar.text, Eventually(Equals("")))
 
         self.main_window.leave_private_mode()

=== modified file 'tests/autopilot/webbrowser_app/tests/test_sad_tab.py'
--- tests/autopilot/webbrowser_app/tests/test_sad_tab.py	2015-09-09 12:14:06 +0000
+++ tests/autopilot/webbrowser_app/tests/test_sad_tab.py	2016-01-21 10:33:49 +0000
@@ -1,6 +1,6 @@
 # -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
 #
-# Copyright 2015 Canonical
+# Copyright 2015-2016 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
@@ -17,8 +17,6 @@
 import signal
 import time
 
-from autopilot.platform import model
-
 from webbrowser_app.tests import StartOpenRemotePageTestCaseBase
 
 
@@ -43,10 +41,11 @@
         self.assert_home_page_eventually_loaded()
 
     def test_close_tab_web_process_killed(self):
+        wide = self.main_window.wide
         sad_tab = self._kill_web_process()
         sad_tab.click_close_tab_button()
-        if model() == 'Desktop':
-            # On desktop, closing the last open tab exits the application
+        if wide:
+            # closing the last open tab exits the application
             self.app.process.wait()
             return
         sad_tab.wait_until_destroyed()
@@ -64,9 +63,10 @@
         self.assert_home_page_eventually_loaded()
 
     def test_close_tab_web_process_crashed(self):
+        wide = self.main_window.wide
         sad_tab = self._crash_web_process()
         sad_tab.click_close_tab_button()
-        if model() == 'Desktop':
+        if wide:
             # On desktop, closing the last open tab exits the application
             self.app.process.wait()
             return

=== modified file 'tests/autopilot/webbrowser_app/tests/test_tabs.py'
--- tests/autopilot/webbrowser_app/tests/test_tabs.py	2015-12-03 10:47:22 +0000
+++ tests/autopilot/webbrowser_app/tests/test_tabs.py	2016-01-21 10:33:49 +0000
@@ -1,6 +1,6 @@
 # -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
 #
-# Copyright 2013-2015 Canonical
+# Copyright 2013-2016 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
@@ -59,16 +59,9 @@
     def test_close_last_open_tab(self):
         tabs_view = self.main_window.get_tabs_view()
         tabs_view.get_previews()[0].close()
-        if model() == 'Desktop':
-            # On desktop, closing the last open tab exits the application
-            self.app.process.wait()
-            return
         tabs_view.visible.wait_for(False)
         self.assert_number_webviews_eventually(1)
         self.main_window.get_new_tab_view()
-        if model() == 'Desktop':
-            address_bar = self.main_window.address_bar
-            self.assertThat(address_bar.activeFocus, Eventually(Equals(True)))
         webview = self.main_window.get_current_webview()
         self.assertThat(webview.url, Equals(""))
 
@@ -140,6 +133,9 @@
         """Test that switching between tabs correctly resets focus to the
            webview if a page is loaded, and to the address bar if we are in
            the new page view"""
+        if not self.main_window.wide:
+            self.skipTest("only on wide form factors")
+
         address_bar = self.main_window.address_bar
 
         self.main_window.press_key('Ctrl+t')
@@ -174,18 +170,20 @@
         self.main_window.press_key('Ctrl+t')
 
         self.main_window.press_key('Ctrl+w')
-        self.assertThat(address_bar.activeFocus, Eventually(Equals(True)))
-
-        self.main_window.press_key('Ctrl+w')
-        webview = self.main_window.get_current_webview()
-        self.assertThat(webview.activeFocus, Eventually(Equals(True)))
-
-        self.main_window.press_key('Ctrl+w')
-        webview = self.main_window.get_current_webview()
-        self.assertThat(webview.activeFocus, Eventually(Equals(True)))
-
-        self.main_window.press_key('Ctrl+w')
-        self.assertThat(address_bar.activeFocus, Eventually(Equals(True)))
+        if self.main_window.wide:
+            self.assertThat(address_bar.activeFocus, Eventually(Equals(True)))
+
+        self.main_window.press_key('Ctrl+w')
+        webview = self.main_window.get_current_webview()
+        self.assertThat(webview.activeFocus, Eventually(Equals(True)))
+
+        self.main_window.press_key('Ctrl+w')
+        webview = self.main_window.get_current_webview()
+        self.assertThat(webview.activeFocus, Eventually(Equals(True)))
+
+        self.main_window.press_key('Ctrl+w')
+        if self.main_window.wide:
+            self.assertThat(address_bar.activeFocus, Eventually(Equals(True)))
 
 
 class TestTabsManagement(StartOpenRemotePageTestCaseBase, TestTabsMixin):
@@ -271,9 +269,10 @@
             toolbar.click_button("doneButton")
             tabs_view.visible.wait_for(False)
         webview = self.main_window.get_current_webview()
+        wide = self.main_window.wide
         self.pointing_device.click_object(webview)
-        if model() == 'Desktop':
-            # On desktop, closing the last open tab exits the application
+        if wide:
+            # closing the last open tab exits the application
             self.app.process.wait()
             return
         webview.wait_until_destroyed()