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

chromeInfo.js.tmpl

 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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/. */
 
 (function() {
   var platformVersion = null;
   var application = null;
   var 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.

   {
     var app = match[1];
     var ver = match[2];
 
     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)
   {
     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
   };
 })();