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

Proposed by Adnane Belmadiaf
Status: Merged
Approved by: Olivier Tilloy
Approved revision: 317
Merged at revision: 328
Proposed branch: lp:~daker/webbrowser-app/fix.1212980
Merge into: lp:webbrowser-app
Diff against target: 293 lines (+144/-22)
4 files modified
po/webbrowser-app.pot (+58/-22)
src/app/AuthenticationDialog.qml (+59/-0)
src/app/Browser.qml (+2/-0)
src/app/ProxyAuthenticationDialog.qml (+25/-0)
To merge this branch: bzr merge lp:~daker/webbrowser-app/fix.1212980
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Olivier Tilloy Approve
Review via email: mp+185932@code.launchpad.net

Commit message

Added support for HTTP/Proxy auth

To post a comment you must log in.
Revision history for this message
Olivier Tilloy (osomon) wrote :

The new QML files are missing copyright and license headers.

review: Needs Fixing
Revision history for this message
Olivier Tilloy (osomon) wrote :

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
Revision history for this message
Olivier Tilloy (osomon) wrote :

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
Revision history for this message
Olivier Tilloy (osomon) wrote :

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.

Revision history for this message
Adnane Belmadiaf (daker) wrote :

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 ?

Revision history for this message
Olivier Tilloy (osomon) wrote :

> 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.

Revision history for this message
Olivier Tilloy (osomon) wrote :

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.

Revision history for this message
Olivier Tilloy (osomon) wrote :

Looks great now!

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
lp:~daker/webbrowser-app/fix.1212980 updated
317. By Adnane Belmadiaf

Merged trunk
Fixed merge conflicts

Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'po/webbrowser-app.pot'
2--- po/webbrowser-app.pot 2013-09-13 11:19:15 +0000
3+++ po/webbrowser-app.pot 2013-09-23 14:50:13 +0000
4@@ -8,7 +8,7 @@
5 msgstr ""
6 "Project-Id-Version: webbrowser-app\n"
7 "Report-Msgid-Bugs-To: \n"
8-"POT-Creation-Date: 2013-09-13 13:16+0200\n"
9+"POT-Creation-Date: 2013-09-20 17:41+0200\n"
10 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
11 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
12 "Language-Team: LANGUAGE <LL@li.org>\n"
13@@ -17,7 +17,7 @@
14 "Content-Type: text/plain; charset=UTF-8\n"
15 "Content-Transfer-Encoding: 8bit\n"
16
17-#: src/app/ActivityView.qml:36 src/app/Chrome.qml:120
18+#: src/app/ActivityView.qml:36 src/app/Chrome.qml:127
19 msgid "Activity"
20 msgstr ""
21
22@@ -25,89 +25,115 @@
23 msgid "Bookmarks"
24 msgstr ""
25
26-#: src/app/Browser.qml:53
27+#: src/app/AuthenticationDialog.qml:24
28+msgid "Authentication required."
29+msgstr ""
30+
31+#. TRANSLATORS: %1 refers to the URL of the current website
32+#: src/app/AuthenticationDialog.qml:26
33+#, qt-format
34+msgid "The website %1 requires authentication."
35+msgstr ""
36+
37+#: src/app/AuthenticationDialog.qml:34
38+msgid "Username"
39+msgstr ""
40+
41+#: src/app/AuthenticationDialog.qml:41
42+msgid "Password"
43+msgstr ""
44+
45+#: src/app/AuthenticationDialog.qml:47
46+msgid "OK"
47+msgstr ""
48+
49+#: src/app/AuthenticationDialog.qml:53
50+msgid "Cancel"
51+msgstr ""
52+
53+#: src/app/Browser.qml:62
54 msgid "Goto"
55 msgstr ""
56
57 #. TRANSLATORS: This is a free-form list of keywords associated to the 'Goto' action.
58 #. Keywords may actually be sentences, and must be separated by semi-colons.
59-#: src/app/Browser.qml:56
60+#: src/app/Browser.qml:65
61 msgid "Address;URL;www"
62 msgstr ""
63
64-#: src/app/Browser.qml:62 src/app/Chrome.qml:67
65+#: src/app/Browser.qml:71 src/app/Chrome.qml:72
66 msgid "Back"
67 msgstr ""
68
69 #. TRANSLATORS: This is a free-form list of keywords associated to the 'Back' action.
70 #. Keywords may actually be sentences, and must be separated by semi-colons.
71-#: src/app/Browser.qml:65
72+#: src/app/Browser.qml:74
73 msgid "Older Page"
74 msgstr ""
75
76-#: src/app/Browser.qml:70 src/app/Chrome.qml:85
77+#: src/app/Browser.qml:79 src/app/Chrome.qml:91
78 msgid "Forward"
79 msgstr ""
80
81 #. TRANSLATORS: This is a free-form list of keywords associated to the 'Forward' action.
82 #. Keywords may actually be sentences, and must be separated by semi-colons.
83-#: src/app/Browser.qml:73
84+#: src/app/Browser.qml:82
85 msgid "Newer Page"
86 msgstr ""
87
88-#: src/app/Browser.qml:78
89+#: src/app/Browser.qml:87
90 msgid "Reload"
91 msgstr ""
92
93 #. TRANSLATORS: This is a free-form list of keywords associated to the 'Reload' action.
94 #. Keywords may actually be sentences, and must be separated by semi-colons.
95-#: src/app/Browser.qml:81
96+#: src/app/Browser.qml:90
97 msgid "Leave Page"
98 msgstr ""
99
100-#: src/app/Browser.qml:86
101+#: src/app/Browser.qml:95
102 msgid "Bookmark"
103 msgstr ""
104
105 #. TRANSLATORS: This is a free-form list of keywords associated to the 'Bookmark' action.
106 #. Keywords may actually be sentences, and must be separated by semi-colons.
107-#: src/app/Browser.qml:89
108+#: src/app/Browser.qml:98
109 msgid "Add This Page to Bookmarks"
110 msgstr ""
111
112-#: src/app/Browser.qml:94
113+#: src/app/Browser.qml:103
114 msgid "New Tab"
115 msgstr ""
116
117 #. TRANSLATORS: This is a free-form list of keywords associated to the 'New Tab' action.
118 #. Keywords may actually be sentences, and must be separated by semi-colons.
119-#: src/app/Browser.qml:97
120+#: src/app/Browser.qml:106
121 msgid "Open a New Tab"
122 msgstr ""
123
124-#: src/app/Browser.qml:102
125+#: src/app/Browser.qml:111
126 msgid "Clear History"
127 msgstr ""
128
129 #. TRANSLATORS: This is a free-form list of keywords associated to the 'Clear History' action.
130 #. Keywords may actually be sentences, and must be separated by semi-colons.
131-#: src/app/Browser.qml:105
132+#: src/app/Browser.qml:114
133 msgid "Clear Navigation History"
134 msgstr ""
135
136-#: src/app/Browser.qml:338
137+#: src/app/Browser.qml:354
138 msgid "Share"
139 msgstr ""
140
141-#: src/app/Browser.qml:342
142+#: src/app/Browser.qml:358
143 msgid "Save"
144 msgstr ""
145
146-#: src/app/Browser.qml:346
147+#: src/app/Browser.qml:362
148 msgid "Copy"
149 msgstr ""
150
151-#: src/app/Browser.qml:353
152+#: src/app/Browser.qml:369
153 msgid "This page wants to know your device’s location."
154 msgstr ""
155
156@@ -142,6 +168,16 @@
157 msgid "Allow"
158 msgstr ""
159
160+#: src/app/ProxyAuthenticationDialog.qml:22
161+msgid "Proxy authentication required."
162+msgstr ""
163+
164+#. TRANSLATORS: %1 refers to the proxy address, %2 refers to the proxy port
165+#: src/app/ProxyAuthenticationDialog.qml:24
166+#, qt-format
167+msgid "The website %1:%2 requires authentication."
168+msgstr ""
169+
170 #. TRANSLATORS: %1 refers to the number of open tabs
171 #: src/app/TabsList.qml:34
172 #, qt-format
173@@ -197,11 +233,11 @@
174 msgstr ""
175
176 #. TRANSLATORS: %1 refers to the current page’s title
177-#: src/app/webbrowser-app.qml:37
178+#: src/app/webbrowser-app.qml:49
179 #, qt-format
180 msgid "%1 - Ubuntu Web Browser"
181 msgstr ""
182
183-#: src/app/webbrowser-app.qml:38
184+#: src/app/webbrowser-app.qml:51
185 msgid "Ubuntu Web Browser"
186 msgstr ""
187
188=== added file 'src/app/AuthenticationDialog.qml'
189--- src/app/AuthenticationDialog.qml 1970-01-01 00:00:00 +0000
190+++ src/app/AuthenticationDialog.qml 2013-09-23 14:50:13 +0000
191@@ -0,0 +1,59 @@
192+/*
193+ * Copyright 2013 Canonical Ltd.
194+ *
195+ * This file is part of webbrowser-app.
196+ *
197+ * webbrowser-app is free software; you can redistribute it and/or modify
198+ * it under the terms of the GNU General Public License as published by
199+ * the Free Software Foundation; version 3.
200+ *
201+ * webbrowser-app is distributed in the hope that it will be useful,
202+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
203+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
204+ * GNU General Public License for more details.
205+ *
206+ * You should have received a copy of the GNU General Public License
207+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
208+ */
209+
210+import QtQuick 2.0
211+import Ubuntu.Components 0.1
212+import Ubuntu.Components.Popups 0.1 as Popups
213+
214+Popups.Dialog {
215+ title: i18n.tr("Authentication required.")
216+ // TRANSLATORS: %1 refers to the URL of the current website
217+ text: i18n.tr("The website %1 requires authentication.").arg(model.hostname)
218+
219+ function accept() {
220+ return model.accept(usernameInput.text, passwordInput.text)
221+ }
222+
223+ TextField {
224+ id: usernameInput
225+ placeholderText: i18n.tr("Username")
226+ text: model.prefilledUsername
227+ onAccepted: accept()
228+ }
229+
230+ TextField {
231+ id: passwordInput
232+ placeholderText: i18n.tr("Password")
233+ echoMode: TextInput.Password
234+ onAccepted: accept()
235+ }
236+
237+ Button {
238+ text: i18n.tr("OK")
239+ color: "green"
240+ onClicked: accept()
241+ }
242+
243+ Button {
244+ text: i18n.tr("Cancel")
245+ color: UbuntuColors.coolGrey
246+ onClicked: model.reject()
247+ }
248+
249+ Component.onCompleted: show()
250+}
251
252=== modified file 'src/app/Browser.qml'
253--- src/app/Browser.qml 2013-09-21 19:56:15 +0000
254+++ src/app/Browser.qml 2013-09-23 14:50:13 +0000
255@@ -346,6 +346,8 @@
256 devicePixelRatio: browser.qtwebkitdpr
257
258 experimental.preferences.developerExtrasEnabled: browser.developerExtrasEnabled
259+ experimental.authenticationDialog: AuthenticationDialog {}
260+ experimental.proxyAuthenticationDialog: ProxyAuthenticationDialog {}
261 experimental.alertDialog: AlertDialog { }
262
263 selectionActions: ActionList {
264
265=== added file 'src/app/ProxyAuthenticationDialog.qml'
266--- src/app/ProxyAuthenticationDialog.qml 1970-01-01 00:00:00 +0000
267+++ src/app/ProxyAuthenticationDialog.qml 2013-09-23 14:50:13 +0000
268@@ -0,0 +1,25 @@
269+/*
270+ * Copyright 2013 Canonical Ltd.
271+ *
272+ * This file is part of webbrowser-app.
273+ *
274+ * webbrowser-app is free software; you can redistribute it and/or modify
275+ * it under the terms of the GNU General Public License as published by
276+ * the Free Software Foundation; version 3.
277+ *
278+ * webbrowser-app is distributed in the hope that it will be useful,
279+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
280+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
281+ * GNU General Public License for more details.
282+ *
283+ * You should have received a copy of the GNU General Public License
284+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
285+ */
286+
287+import QtQuick 2.0
288+
289+AuthenticationDialog {
290+ title: i18n.tr("Proxy authentication required.")
291+ // TRANSLATORS: %1 refers to the proxy address, %2 refers to the proxy port
292+ text: i18n.tr("The website %1:%2 requires authentication.").arg(model.hostname).arg(model.port)
293+}

Subscribers

People subscribed via source and target branches

to status/vote changes: