Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: chromeInfo.js.tmpl

Issue 29332680: Issue 3415 - Detect application based on UA for Chromium-based browsers (Closed)
Patch Set: Created Dec. 15, 2015, 4:29 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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
Sebastian Noack 2015/12/15 17:31:31 Well, the context here is limitted. And after all
+ var match;
+
+ while (match = regexp.exec(navigator.userAgent))
kzar 2015/12/15 17:26:00 Maybe a comment for this loop explaining that we'r
Sebastian Noack 2015/12/15 17:31:31 Well, how could a comment make it more obvious tha
{
- {%- 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();
kzar 2015/12/15 17:26:00 Nit: Maybe just do `application = app.toLowerCase(
Sebastian Noack 2015/12/15 17:31:31 I don't think that spreading these conversions acr
+ 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
+ };
+})();
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld