[Merge] lp:~abreu-alexandre/webbrowser-app/simple-user-agent-override into lp:webbrowser-app
Olivier Tilloy
olivier.tilloy at canonical.com
Fri Sep 26 06:05:43 UTC 2014
Looks good overall (I still need to test the functionality). See a couple of inline comments.
Diff comments:
> === modified file 'po/webbrowser-app.pot'
> --- po/webbrowser-app.pot 2014-08-21 11:28:58 +0000
> +++ po/webbrowser-app.pot 2014-09-25 20:45:00 +0000
> @@ -8,7 +8,7 @@
> msgstr ""
> "Project-Id-Version: webbrowser-app\n"
> "Report-Msgid-Bugs-To: \n"
> -"POT-Creation-Date: 2014-08-21 13:27+0200\n"
> +"POT-Creation-Date: 2014-09-25 16:35-0400\n"
> "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
> "Last-Translator: FULL NAME <EMAIL at ADDRESS>\n"
> "Language-Team: LANGUAGE <LL at li.org>\n"
> @@ -245,23 +245,23 @@
> msgid "Share…"
> msgstr ""
>
> -#: src/app/webbrowser/AddressBar.qml:148
> +#: src/app/webbrowser/AddressBar.qml:149
> msgid "search or enter an address"
> msgstr ""
>
> -#: src/app/webbrowser/Browser.qml:154
> +#: src/app/webbrowser/Browser.qml:161
> msgid "Share"
> msgstr ""
>
> -#: src/app/webbrowser/Browser.qml:168
> +#: src/app/webbrowser/Browser.qml:175
> msgid "History"
> msgstr ""
>
> -#: src/app/webbrowser/Browser.qml:174
> +#: src/app/webbrowser/Browser.qml:182
> msgid "Open tabs"
> msgstr ""
>
> -#: src/app/webbrowser/Browser.qml:180 src/app/webbrowser/TabsView.qml:57
> +#: src/app/webbrowser/Browser.qml:188 src/app/webbrowser/TabsView.qml:57
> msgid "New tab"
> msgstr ""
>
> @@ -330,25 +330,25 @@
>
> #. TRANSLATORS: %1 refers to the current page’s title
> #: src/app/webbrowser/webbrowser-app.qml:39
> -#: src/app/webcontainer/webapp-container.qml:57
> +#: src/app/webcontainer/webapp-container.qml:61
> #, qt-format
> msgid "%1 - Ubuntu Web Browser"
> msgstr ""
>
> #: src/app/webbrowser/webbrowser-app.qml:41
> -#: src/app/webcontainer/webapp-container.qml:59
> +#: src/app/webcontainer/webapp-container.qml:63
> msgid "Ubuntu Web Browser"
> msgstr ""
>
> -#: src/app/webcontainer/AccountsLoginPage.qml:70
> +#: src/app/webcontainer/AccountsLoginPage.qml:87
> msgid "No local account found for "
> msgstr ""
>
> -#: src/app/webcontainer/AccountsLoginPage.qml:75
> +#: src/app/webcontainer/AccountsLoginPage.qml:92
> msgid "Skip account creation step"
> msgstr ""
>
> -#: src/app/webcontainer/AccountsLoginPage.qml:124
> +#: src/app/webcontainer/AccountsLoginPage.qml:141
> msgid "Add account"
> msgstr ""
>
>
> === modified file 'src/app/webcontainer/WebApp.qml'
> --- src/app/webcontainer/WebApp.qml 2014-08-22 16:08:20 +0000
> +++ src/app/webcontainer/WebApp.qml 2014-09-25 20:45:00 +0000
> @@ -39,6 +39,7 @@
> property alias webappUrlPatterns: webview.webappUrlPatterns
> property alias popupRedirectionUrlPrefix: webview.popupRedirectionUrlPrefix
> property alias webviewOverrideFile: webview.webviewOverrideFile
> + property alias localUserAgentOverride: webview.localUserAgentOverride
>
> property bool backForwardButtonsVisible: false
> property bool chromeVisible: false
> @@ -58,6 +59,14 @@
> }
> ]
>
> + function getLocalUserAgentOverrideIfAny() {
> + if (localUserAgentOverride.length !== 0)
> + return localUserAgentOverride
> +
> + return webappName && unityWebapps.model.exists(webappName) ?
> + unityWebapps.model.userAgentOverrideFor(webappName) : ""
Now that you’ve made this a full-blown function, how about unwinding this conditional statement for extra readability:
if (webappName && unityWebapps.model.exists(webappName))
return unityWebapps.model.userAgentOverrideFor(webappName)
return ""
> + }
> +
> Item {
> anchors.fill: parent
>
> @@ -72,8 +81,7 @@
> }
> height: parent.height - osk.height - (webapp.chromeless ? 0 : chromeLoader.item.visibleHeight)
> developerExtrasEnabled: webapp.developerExtrasEnabled
> - localUserAgentOverride: webappName && unityWebapps.model.exists(webappName) ?
> - unityWebapps.model.userAgentOverrideFor(webappName) : ""
> + localUserAgentOverride: getLocalUserAgentOverrideIfAny()
> }
>
> Loader {
>
> === modified file 'src/app/webcontainer/webapp-container.cpp'
> --- src/app/webcontainer/webapp-container.cpp 2014-08-26 07:15:22 +0000
> +++ src/app/webcontainer/webapp-container.cpp 2014-09-25 20:45:00 +0000
> @@ -137,6 +137,10 @@
> m_window->setProperty("popupRedirectionUrlPrefix", m_popupRedirectionUrlPrefix);
> }
>
> + if (!m_userAgentOverride.isEmpty()) {
> + m_window->setProperty("localUserAgentOverride", m_userAgentOverride);
> + }
> +
> // Experimental, unsupported API, to override the webview
> QFileInfo overrideFile("webview-override.qml");
> if (overrideFile.exists()) {
> @@ -208,6 +212,7 @@
> out << " --webappUrlPatterns=URL_PATTERNS list of comma-separated url patterns (wildcard based) that the webapp is allowed to navigate to" << endl;
> out << " --accountProvider=PROVIDER_NAME Online account provider for the application if the application is to reuse a local account." << endl;
> out << " --store-session-cookies store session cookies on disk" << endl;
> + out << " --user-agent=USER_AGENT overrides the default User Agent with the provided one." << endl;
The alignment of the description of the parameter is off by one whitespace to the right.
Also, how about making that parameter "--user-agent-string" for extra clarity? (that’s only a suggestion, feel free to ignore if you prefer the short version)
> out << "Chrome options (if none specified, no chrome is shown by default):" << endl;
> out << " --enable-back-forward enable the display of the back and forward buttons (implies --enable-addressbar)" << endl;
> out << " --enable-addressbar enable the display of a minimal chrome (favicon and title)" << endl;
> @@ -246,9 +251,11 @@
> } else if (argument == "--local-webapp-manifest") {
> m_localWebappManifest = true;
> } else if (argument.startsWith("--popup-redirection-url-prefix=")) {
> - m_popupRedirectionUrlPrefix = argument.split("--popup-redirection-url-prefix=")[1];;
> + m_popupRedirectionUrlPrefix = argument.split("--popup-redirection-url-prefix=")[1];
> } else if (argument.startsWith("--local-cookie-db-path=")) {
> - m_localCookieStoreDbPath = argument.split("--local-cookie-db-path=")[1];;
> + m_localCookieStoreDbPath = argument.split("--local-cookie-db-path=")[1];
> + } else if (argument.startsWith("--user-agent=")) {
> + m_userAgentOverride = argument.split("--user-agent=")[1];
> }
> }
> }
>
> === modified file 'src/app/webcontainer/webapp-container.h'
> --- src/app/webcontainer/webapp-container.h 2014-08-13 16:23:36 +0000
> +++ src/app/webcontainer/webapp-container.h 2014-09-25 20:45:00 +0000
> @@ -58,6 +58,7 @@
> bool m_localWebappManifest;
> QString m_popupRedirectionUrlPrefix;
> QString m_localCookieStoreDbPath;
> + QString m_userAgentOverride;
> QScopedPointer<WebappContainerHelper> m_webappContainerHelper;
>
> static const QString URL_PATTERN_SEPARATOR;
>
> === modified file 'src/app/webcontainer/webapp-container.qml'
> --- src/app/webcontainer/webapp-container.qml 2014-08-27 13:21:34 +0000
> +++ src/app/webcontainer/webapp-container.qml 2014-09-25 20:45:00 +0000
> @@ -44,6 +44,7 @@
> property string popupRedirectionUrlPrefix: ""
> property url webviewOverrideFile: ""
> property var __webappCookieStore: null
> + property string localUserAgentOverride: ""
>
> contentOrientation: Screen.orientation
>
> @@ -80,6 +81,8 @@
> webappModelSearchPath: root.webappModelSearchPath
> webappUrlPatterns: root.webappUrlPatterns
>
> + localUserAgentOverride: root.localUserAgentOverride
> +
> popupRedirectionUrlPrefix: root.popupRedirectionUrlPrefix
> webviewOverrideFile: root.webviewOverrideFile
>
>
--
https://code.launchpad.net/~abreu-alexandre/webbrowser-app/simple-user-agent-override/+merge/236030
Your team Ubuntu Phablet Team is requested to review the proposed merge of lp:~abreu-alexandre/webbrowser-app/simple-user-agent-override into lp:webbrowser-app.
More information about the Ubuntu-reviews
mailing list