Merge lp:~mandel/ubuntu-sso-client/tc-not-loading into lp:ubuntu-sso-client

[Manuel de la Peña]

- Connected the WebKit browser correctly so that the tc page gets loaded (LP: #933081).
- Added code to check if the browser with the t&c was already loaded. If it is just show the t&c page, otherwise perform the request (LP: #956185).

- Connected the Webkit correctly so that the tc page gets loaded (LP: #933081).
- Added code to check if the browser with the t&c was already loaded. If it is just show the t&c page, otherwise perform the request (LP: #956185).

In order to do an IRL you ca do something like ./bin/ubuntu-sso-login-gtk --app_name test --tc_url https://one.ubuntu.com/terms/

PS: remember to set the python path.

[Eric Casteleijn (thisfred)]

Looks good, T&C loads

review: Approve

[Natalia Bidart (nataliabidart)]

Looks great!

review: Approve

=== modified file 'ubuntu_sso/gtk/gui.py'
--- ubuntu_sso/gtk/gui.py	2012-02-17 18:43:17 +0000
+++ ubuntu_sso/gtk/gui.py	2012-03-15 16:57:19 +0000
@@ -90,23 +90,6 @@
     return c
 # pylint: enable=C0103
 
-
-# To be removed when Python bindings provide these constants
-# as per http://code.google.com/p/pywebkitgtk/issues/detail?id=44
-# WebKitLoadStatus
-WEBKIT_LOAD_PROVISIONAL = 0
-WEBKIT_LOAD_COMMITTED = 1
-WEBKIT_LOAD_FINISHED = 2
-WEBKIT_LOAD_FIRST_VISUALLY_NON_EMPTY_LAYOUT = 3
-WEBKIT_LOAD_FAILED = 4
-# WebKitWebNavigationReason
-WEBKIT_WEB_NAVIGATION_REASON_LINK_CLICKED = 0
-WEBKIT_WEB_NAVIGATION_REASON_FORM_SUBMITTED = 1
-WEBKIT_WEB_NAVIGATION_REASON_BACK_FORWARD = 2
-WEBKIT_WEB_NAVIGATION_REASON_RELOAD = 3
-WEBKIT_WEB_NAVIGATION_REASON_FORM_RESUBMITTED = 4
-WEBKIT_WEB_NAVIGATION_REASON_OTHER = 5
-
 DEFAULT_WIDTH = 30
 # To be replaced by values from the theme (LP: #616526)
 HELP_TEXT_COLOR = parse_color("#bfbfbf")
@@ -947,23 +930,14 @@
 
         self._set_current_page(self.processing_vbox)
 
-    def on_tc_button_clicked(self, *args, **kwargs):
-        """The T&C button was clicked, create the browser and load terms."""
+    def _add_webkit_browser(self):
+        """Add the webkit browser for the t&c."""
         # delay the import of webkit to be able to build without it
         from gi.repository import WebKit  # pylint: disable=E0611
+
         browser = WebKit.WebView()
 
-        # The signal WebKitWebView::load-finished is deprecated and should not
-        # be used in newly-written code. Use the "load-status" property
-        # instead. Connect to "notify::load-status" to monitor loading.
-
-        # nataliabidart (2010-10-04): connecting this signal makes the loading
-        # of the Ubuntu One terms URL to fail. So we're using the deprecated
-        # 'load-finished' for now.
-
-        #browser.connect('notify::load-status',
-        #                self.on_tc_browser_notify_load_status)
-        browser.connect('load-finished',
+        browser.connect('notify::load-status',
                         self.on_tc_browser_notify_load_status)
         browser.connect('navigation-policy-decision-requested',
                         self.on_tc_browser_navigation_requested)
@@ -978,7 +952,14 @@
         browser.load_uri(self.tc_url)
         browser.show()
         self.tc_browser_window.add(browser)
-        self._set_current_page(self.processing_vbox)
+
+    def on_tc_button_clicked(self, *args, **kwargs):
+        """The T&C button was clicked, create the browser and load terms."""
+        if self.tc_browser_window.get_child() is None:
+            self._add_webkit_browser()
+            self._set_current_page(self.processing_vbox)
+        else:
+            self._set_current_page(self.tc_browser_vbox)
 
     def on_tc_back_button_clicked(self, *args, **kwargs):
         """T & C 'back' button was clicked, return to the previous page."""
@@ -986,14 +967,18 @@
 
     def on_tc_browser_notify_load_status(self, browser, *args, **kwargs):
         """The T&C page is being loaded."""
-        if browser.get_load_status() == WEBKIT_LOAD_FINISHED:
+        from gi.repository import WebKit  # pylint: disable=E0611
+
+        if browser.get_load_status().real == WebKit.LoadStatus.FINISHED:
             self._set_current_page(self.tc_browser_vbox)
 
     def on_tc_browser_navigation_requested(self, browser, frame, request,
                                            action, decision, *args, **kwargs):
         """The user wants to navigate within the T&C browser."""
+        from gi.repository import WebKit  # pylint: disable=E0611
+
         if action is not None and \
-           action.get_reason() == WEBKIT_WEB_NAVIGATION_REASON_LINK_CLICKED:
+           action.get_reason() == WebKit.WebNavigationReason.LINK_CLICKED:
             if decision is not None:
                 decision.ignore()
             url = action.get_original_uri()

=== modified file 'ubuntu_sso/gtk/tests/test_gui.py'
--- ubuntu_sso/gtk/tests/test_gui.py	2012-02-17 18:43:17 +0000
+++ ubuntu_sso/gtk/tests/test_gui.py	2012-03-15 16:57:19 +0000
@@ -134,7 +134,7 @@
 
     def get_load_status(self):
         """Return the current load status."""
-        return gui.WEBKIT_LOAD_FINISHED
+        return WebKit.LoadStatus.FINISHED
 
     def show(self):
         """Show this instance."""
@@ -981,7 +981,7 @@
     def test_notify_load_finished_connected(self):
         """The 'load-finished' signal is connected."""
         expected = [self.ui.on_tc_browser_notify_load_status]
-        self.assertEqual(self.browser._signals['load-finished'],
+        self.assertEqual(self.browser._signals['notify::load-status'],
                          expected)
 
     def test_tc_loaded_morphs_into_tc_browser_vbox(self):
@@ -998,7 +998,7 @@
     def test_navigation_requested_succeeds_for_no_clicking(self):
         """The navigation request succeeds when user hasn't clicked a link."""
         action = WebKit.WebNavigationAction()
-        action.set_reason(gui.WEBKIT_WEB_NAVIGATION_REASON_OTHER)
+        action.set_reason(WebKit.WebNavigationReason.OTHER)
 
         decision = WebKit.WebPolicyDecision()
         decision.use = self._set_called
@@ -1011,7 +1011,7 @@
     def test_navigation_requested_ignores_clicked_links(self):
         """The navigation request is ignored if a link was clicked."""
         action = WebKit.WebNavigationAction()
-        action.set_reason(gui.WEBKIT_WEB_NAVIGATION_REASON_LINK_CLICKED)
+        action.set_reason(WebKit.WebNavigationReason.LINK_CLICKED)
 
         decision = WebKit.WebPolicyDecision()
         decision.ignore = self._set_called
@@ -1037,7 +1037,7 @@
         """
         url = 'http://something.com/yadda'
         action = WebKit.WebNavigationAction()
-        action.set_reason(gui.WEBKIT_WEB_NAVIGATION_REASON_LINK_CLICKED)
+        action.set_reason(WebKit.WebNavigationReason.LINK_CLICKED)
         action.set_original_uri(url)
 
         decision = WebKit.WebPolicyDecision()
@@ -1050,6 +1050,35 @@
         self.ui.on_tc_browser_navigation_requested(**kwargs)
         self.assertEqual(self._called, ((url,), {}))
 
+    def test_on_tc_button_clicked_no_child(self):
+        """Test the tc loading with no child."""
+        called = []
+
+        def fake_add_browser():
+            """Fake add browser."""
+            called.append('fake_add_browser')
+
+        self.patch(self.ui, '_add_webkit_browser', fake_add_browser)
+        self.patch(self.ui.tc_browser_window, 'get_child', lambda: None)
+
+        self.ui.on_tc_button_clicked()
+        self.assertIn('fake_add_browser', called)
+
+    def test_on_tc_button_clicked_child(self):
+        """Test the tc loading with child."""
+        called = []
+
+        def fake_add_browser(i_self):
+            """Fake add browser."""
+            called.append('fake_add_browser')
+
+        self.patch(self.ui, '_add_webkit_browser', fake_add_browser)
+
+        browser = WebKit.WebView()
+        self.ui.tc_browser_window.add(browser)
+        self.ui.on_tc_button_clicked()
+        self.assertNotIn('fake_add_browser', called)
+
 
 class RegistrationErrorTestCase(UbuntuSSOClientTestCase):
     """Test suite for the user registration error handling."""