Issue 3415 - Detect application based on UA for Chromium-based browsers

chromeInfo.js.tmpl

Index: chromeInfo.js.tmpl
===================================================================
--- a/chromeInfo.js.tmpl
+++ b/chromeInfo.js.tmpl
@@ -2,26 +2,56 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-require.scopes.info = {
-  addonName: {{metadata.get('general', 'basename')|json}},
-  addonVersion: {{version|json}},
-  addonRoot: "",
+(function() {
+  var platformVersion = null;
+  var application = null;
+  var applicationVersion;
 
-  application: {{type|json}},
-  get applicationVersion()
+  var regexp = /(\S+)\/(\S+)(?:\s*\(.*?\))?/g;

[kzar, 2015/12/15 17:26:00]

Nit: More descriptive name would be nice, perhaps app_ver_regexp?

[Sebastian Noack, 2015/12/15 17:31:31]

Well, the context here is limitted. And after all we are used to call match
simply match as well.

+  var match;
+
+  while (match = regexp.exec(navigator.userAgent))

[kzar, 2015/12/15 17:26:00]

Maybe a comment for this loop explaining that we're inspecting the browser's
user agent string to make our best guess of platform and version?

[Sebastian Noack, 2015/12/15 17:31:31]

Well, how could a comment make it more obvious that we inspect the user agent
string, while this line literally says execute a regular expression on
navigator.userAgent? ;) I think the less intuitive bits are already in the
comments below.

   {
-    {%- if type == 'opera' %}
-    var match = /\bOPR\/(\S+)/.exec(navigator.userAgent);
-    return (match ? match[1] : "0");
-    {%- else %}
-    return this.platformVersion;
-    {%- endif %}
-  },
+    var app = match[1];
+    var ver = match[2];
 
-  platform: "chromium",
-  get platformVersion()
+    if (app == "Chrome")
+    {
+      platformVersion = ver;
+    }
+    else if (app != "Mozilla" && app != "AppleWebKit" && app != "Safari")
+    {
+      // For compatibility with legacy websites, Chrome's UA
+      // also includes a Mozilla, AppleWebKit and Safari token.
+      // Any further name/version pair indicates a fork.
+      application = app == "OPR" ? "opera" : app.toLowerCase();

[Sebastian Noack, 2015/12/15 16:36:58]

For reference, some UAs, I tested the logic against:

http://www.webapps-online.com/online-tools/user-agent-strings/dv/browser51856...
http://www.webapps-online.com/online-tools/user-agent-strings/dv/browser25589...
http://www.webapps-online.com/online-tools/user-agent-strings/dv/browser19693...
http://www.webapps-online.com/online-tools/user-agent-strings/dv/browser11563...
http://www.webapps-online.com/online-tools/user-agent-strings/dv/plugin55210/...

[kzar, 2015/12/15 17:26:00]

Nit: Maybe just do `application = app.toLowerCase()` here and then later change
"opr" to "opera"?

[Sebastian Noack, 2015/12/15 17:31:31]

I don't think that spreading these conversions across the code makes anything
easier to understand.

+      applicationVersion = ver;
+    }
+  }
+
+  // not a Chromium-based UA, probably modifed by the user
+  if (!platformVersion)
   {
-    var match = /\bChrome\/(\S+)/.exec(navigator.userAgent);
-    return (match ? match[1] : "0");
+    application = "unknown";
+    applicationVersion = platformVersion = "0";
   }
-};
+
+  // no additional name/version, so this is upstream Chrome
+  if (!application)
+  {
+    application = "chrome";
+    applicationVersion = platformVersion;
+  }
+
+  require.scopes.info = {
+    addonName: {{ metadata.get('general', 'basename')|json }},
+    addonVersion: {{ version|json }},
+    addonRoot: "",
+
+    application: application,
+    applicationVersion: applicationVersion,
+
+    platform: "chromium",
+    platformVersion: platformVersion
+  };
+})();