[Merge] lp:~abreu-alexandre/webbrowser-app/simple-user-agent-override into lp:webbrowser-app

Olivier Tilloy olivier.tilloy at canonical.com
Mon Sep 29 15:12:24 UTC 2014


Review: Needs Fixing

Looks mostly ok. See my comments inline.

Diff comments:

> === 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-26 13:08:01 +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,16 @@
>          }
>      ]
>  
> +    function getLocalUserAgentOverrideIfAny() {
> +        if (localUserAgentOverride.length !== 0)
> +            return localUserAgentOverride
> +
> +        if (webappName && unityWebapps.model.exists(webappName))
> +            return unityWebapps.model.userAgentOverrideFor(webappName)
> +
> +        return ""
> +    }
> +
>      Item {
>          anchors.fill: parent
>  
> @@ -72,8 +83,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()

This assignment and the alias defined at the top of the file are conflicting with each other: either define an alias and do not assign a value inside the WebappContainerWebview instance, or make remove the 'alias' qualifier from the top-level property.

>          }
>  
>          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-26 13:08:01 +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-string=USER_AGENT      overrides the default User Agent with the provided one." << endl;

You’ll also need to add a

    [--user-agent-string=USER_AGENT]

line to the help text for completeness.

>      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-string=")) {
> +            m_userAgentOverride = argument.split("--user-agent-string=")[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-26 13:08:01 +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-26 13:08:01 +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 subscribed to branch lp:webbrowser-app.



More information about the Ubuntu-reviews mailing list