Merge lp:~daker/webbrowser-app/fix.1212980 into lp:webbrowser-app

[Adnane Belmadiaf]

Added support for HTTP/Proxy auth

[Olivier Tilloy (osomon)]

The new QML files are missing copyright and license headers.

review: Needs Fixing

[Olivier Tilloy (osomon)]

Internationalized strings should be only in the app, not in the browser component.
You’re going to have to move the following two lines to src/app/Browser.qml (as well as the new components under src/app/):

    64 + experimental.authenticationDialog: AuthenticationDialog {}
    65 + experimental.proxyAuthenticationDialog: ProxyAuthenticationDialog {}

review: Needs Fixing

[Olivier Tilloy (osomon)]

14 + text: i18n.tr("The website %1 requires authentication.").arg(model.hostname)
54 + text: i18n.tr("The website %1:%2 requires authentication.").arg(model.hostname).arg(model.port)

Internationalized strings containing %-placeholders should be preceded by a comment for translators explaining what the value of the placeholder is expected to be. See for example, in src/app/ErrorSheet.qml:

    // TRANSLATORS: %1 refers to the URL of the current page
    text: i18n.tr("It appears you are having trouble viewing: %1.").arg(url)

review: Needs Fixing

[Olivier Tilloy (osomon)]

A suggestion:

20 + onAccepted: model.accept(usernameInput.text, passwordInput.text)
27 + onAccepted: model.accept(usernameInput.text, passwordInput.text)
33 + onClicked: model.accept(usernameInput.text, passwordInput.text)

You might want to factor out this code into an accept() function.

[Adnane Belmadiaf (daker)]

i'll fix it, one other thing i still don't know how to do is the test, any idea how it should be done ?

[Olivier Tilloy (osomon)]

> i'll fix it, one other thing i still don't know how to do is the test, any
> idea how it should be done ?

If you want to add an autopilot test then you’re going to need to patch the test HTTP server to add a special URL that requires HTTP authentication. For an (incomplete) implementation, see the test script I attached to bug #1212980.
Then you’d need to write a new test that navigates to this special URL, checks that the authentication dialog is being displayed, and you would need various test cases to test cancelling, entering invalid credentials, and entering correct credentials.

However it’s fine by me if you do it in a separate MR, or if you choose to not do it for now.

[Olivier Tilloy (osomon)]

Looks great!

A couple of additional remarks before we can merge it:

 - The component ids ('authDialog' and 'proxyAuthDialog') are unused here, you can safely remove them.

 - Can you regenerate the translation templates now that there are new strings available? To do that, you will need to explicitly re-run cmake, then "make webbrowser-app.pot". Then you can commit the changes to po/webbrowser-app.pot.

[Olivier Tilloy (osomon)]

Looks great now!

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/webbrowser-app-autolanding/228/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/3771
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/1299
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-amd64-autolanding/172
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-autolanding/172
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-autolanding/172/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-i386-autolanding/172
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-saucy/115
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3783
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3783/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/1301
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/1301/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/1088
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/1102

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/webbrowser-app-autolanding/234/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/3783
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/1311
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-amd64-autolanding/178
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-autolanding/178
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-autolanding/178/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-i386-autolanding/178
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-saucy/129
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3795
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3795/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/1313
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/1313/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/1102
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/1114

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/webbrowser-app-autolanding/241/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/3814
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/1342
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-amd64-autolanding/185
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-autolanding/185
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-autolanding/185/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-i386-autolanding/185
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-saucy/156
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3826
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3826/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/1344
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/1344/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/1130
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/1141

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/webbrowser-app-autolanding/250/
Executed test runs:
    None: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/3834/console
    None: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/1362/console
    None: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3846/console

review: Needs Fixing (continuous-integration)

[lp:~daker/webbrowser-app/fix.1212980]

317. By Adnane Belmadiaf on 2013-09-23

Merged trunk
Fixed merge conflicts

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'po/webbrowser-app.pot'
--- po/webbrowser-app.pot	2013-09-13 11:19:15 +0000
+++ po/webbrowser-app.pot	2013-09-23 14:50:13 +0000
@@ -8,7 +8,7 @@
 msgstr ""
 "Project-Id-Version: webbrowser-app\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2013-09-13 13:16+0200\n"
+"POT-Creation-Date: 2013-09-20 17:41+0200\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <[e-mail]>\n"
@@ -17,7 +17,7 @@
 "Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
 
-#: src/app/ActivityView.qml:36 src/app/Chrome.qml:120
+#: src/app/ActivityView.qml:36 src/app/Chrome.qml:127
 msgid "Activity"
 msgstr ""
 
@@ -25,89 +25,115 @@
 msgid "Bookmarks"
 msgstr ""
 
-#: src/app/Browser.qml:53
+#: src/app/AuthenticationDialog.qml:24
+msgid "Authentication required."
+msgstr ""
+
+#. TRANSLATORS: %1 refers to the URL of the current website
+#: src/app/AuthenticationDialog.qml:26
+#, qt-format
+msgid "The website %1 requires authentication."
+msgstr ""
+
+#: src/app/AuthenticationDialog.qml:34
+msgid "Username"
+msgstr ""
+
+#: src/app/AuthenticationDialog.qml:41
+msgid "Password"
+msgstr ""
+
+#: src/app/AuthenticationDialog.qml:47
+msgid "OK"
+msgstr ""
+
+#: src/app/AuthenticationDialog.qml:53
+msgid "Cancel"
+msgstr ""
+
+#: src/app/Browser.qml:62
 msgid "Goto"
 msgstr ""
 
 #. TRANSLATORS: This is a free-form list of keywords associated to the 'Goto' action.
 #. Keywords may actually be sentences, and must be separated by semi-colons.
-#: src/app/Browser.qml:56
+#: src/app/Browser.qml:65
 msgid "Address;URL;www"
 msgstr ""
 
-#: src/app/Browser.qml:62 src/app/Chrome.qml:67
+#: src/app/Browser.qml:71 src/app/Chrome.qml:72
 msgid "Back"
 msgstr ""
 
 #. TRANSLATORS: This is a free-form list of keywords associated to the 'Back' action.
 #. Keywords may actually be sentences, and must be separated by semi-colons.
-#: src/app/Browser.qml:65
+#: src/app/Browser.qml:74
 msgid "Older Page"
 msgstr ""
 
-#: src/app/Browser.qml:70 src/app/Chrome.qml:85
+#: src/app/Browser.qml:79 src/app/Chrome.qml:91
 msgid "Forward"
 msgstr ""
 
 #. TRANSLATORS: This is a free-form list of keywords associated to the 'Forward' action.
 #. Keywords may actually be sentences, and must be separated by semi-colons.
-#: src/app/Browser.qml:73
+#: src/app/Browser.qml:82
 msgid "Newer Page"
 msgstr ""
 
-#: src/app/Browser.qml:78
+#: src/app/Browser.qml:87
 msgid "Reload"
 msgstr ""
 
 #. TRANSLATORS: This is a free-form list of keywords associated to the 'Reload' action.
 #. Keywords may actually be sentences, and must be separated by semi-colons.
-#: src/app/Browser.qml:81
+#: src/app/Browser.qml:90
 msgid "Leave Page"
 msgstr ""
 
-#: src/app/Browser.qml:86
+#: src/app/Browser.qml:95
 msgid "Bookmark"
 msgstr ""
 
 #. TRANSLATORS: This is a free-form list of keywords associated to the 'Bookmark' action.
 #. Keywords may actually be sentences, and must be separated by semi-colons.
-#: src/app/Browser.qml:89
+#: src/app/Browser.qml:98
 msgid "Add This Page to Bookmarks"
 msgstr ""
 
-#: src/app/Browser.qml:94
+#: src/app/Browser.qml:103
 msgid "New Tab"
 msgstr ""
 
 #. TRANSLATORS: This is a free-form list of keywords associated to the 'New Tab' action.
 #. Keywords may actually be sentences, and must be separated by semi-colons.
-#: src/app/Browser.qml:97
+#: src/app/Browser.qml:106
 msgid "Open a New Tab"
 msgstr ""
 
-#: src/app/Browser.qml:102
+#: src/app/Browser.qml:111
 msgid "Clear History"
 msgstr ""
 
 #. TRANSLATORS: This is a free-form list of keywords associated to the 'Clear History' action.
 #. Keywords may actually be sentences, and must be separated by semi-colons.
-#: src/app/Browser.qml:105
+#: src/app/Browser.qml:114
 msgid "Clear Navigation History"
 msgstr ""
 
-#: src/app/Browser.qml:338
+#: src/app/Browser.qml:354
 msgid "Share"
 msgstr ""
 
-#: src/app/Browser.qml:342
+#: src/app/Browser.qml:358
 msgid "Save"
 msgstr ""
 
-#: src/app/Browser.qml:346
+#: src/app/Browser.qml:362
 msgid "Copy"
 msgstr ""
 
-#: src/app/Browser.qml:353
+#: src/app/Browser.qml:369
 msgid "This page wants to know your device’s location."
 msgstr ""
 
@@ -142,6 +168,16 @@
 msgid "Allow"
 msgstr ""
 
+#: src/app/ProxyAuthenticationDialog.qml:22
+msgid "Proxy authentication required."
+msgstr ""
+
+#. TRANSLATORS: %1 refers to the proxy address, %2 refers to the proxy port
+#: src/app/ProxyAuthenticationDialog.qml:24
+#, qt-format
+msgid "The website %1:%2 requires authentication."
+msgstr ""
+
 #. TRANSLATORS: %1 refers to the number of open tabs
 #: src/app/TabsList.qml:34
 #, qt-format
@@ -197,11 +233,11 @@
 msgstr ""
 
 #. TRANSLATORS: %1 refers to the current page’s title
-#: src/app/webbrowser-app.qml:37
+#: src/app/webbrowser-app.qml:49
 #, qt-format
 msgid "%1 - Ubuntu Web Browser"
 msgstr ""
 
-#: src/app/webbrowser-app.qml:38
+#: src/app/webbrowser-app.qml:51
 msgid "Ubuntu Web Browser"
 msgstr ""

=== added file 'src/app/AuthenticationDialog.qml'
--- src/app/AuthenticationDialog.qml	1970-01-01 00:00:00 +0000
+++ src/app/AuthenticationDialog.qml	2013-09-23 14:50:13 +0000
@@ -0,0 +1,59 @@
+/*
+ * Copyright 2013 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 Ubuntu.Components 0.1
+import Ubuntu.Components.Popups 0.1 as Popups
+
+Popups.Dialog {
+    title: i18n.tr("Authentication required.")
+    // TRANSLATORS: %1 refers to the URL of the current website
+    text: i18n.tr("The website %1 requires authentication.").arg(model.hostname)
+
+    function accept() {
+        return model.accept(usernameInput.text, passwordInput.text)
+    }
+
+    TextField {
+        id: usernameInput
+        placeholderText: i18n.tr("Username")
+        text: model.prefilledUsername
+        onAccepted: accept()
+    }
+
+    TextField {
+        id: passwordInput
+        placeholderText: i18n.tr("Password")
+        echoMode: TextInput.Password
+        onAccepted: accept()
+    }
+
+    Button {
+        text: i18n.tr("OK")
+        color: "green"
+        onClicked: accept()
+    }
+
+    Button {
+        text: i18n.tr("Cancel")
+        color: UbuntuColors.coolGrey
+        onClicked: model.reject()
+    }
+
+    Component.onCompleted: show()
+}

=== modified file 'src/app/Browser.qml'
--- src/app/Browser.qml	2013-09-21 19:56:15 +0000
+++ src/app/Browser.qml	2013-09-23 14:50:13 +0000
@@ -346,6 +346,8 @@
             devicePixelRatio: browser.qtwebkitdpr
 
             experimental.preferences.developerExtrasEnabled: browser.developerExtrasEnabled
+            experimental.authenticationDialog: AuthenticationDialog {}
+            experimental.proxyAuthenticationDialog: ProxyAuthenticationDialog {}
             experimental.alertDialog: AlertDialog { }
 
             selectionActions: ActionList {

=== added file 'src/app/ProxyAuthenticationDialog.qml'
--- src/app/ProxyAuthenticationDialog.qml	1970-01-01 00:00:00 +0000
+++ src/app/ProxyAuthenticationDialog.qml	2013-09-23 14:50:13 +0000
@@ -0,0 +1,25 @@
+/*
+ * Copyright 2013 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
+
+AuthenticationDialog {
+    title: i18n.tr("Proxy authentication required.")
+    // TRANSLATORS: %1 refers to the proxy address, %2 refers to the proxy port
+    text: i18n.tr("The website %1:%2 requires authentication.").arg(model.hostname).arg(model.port)
+}