templates/edgeInfo.js.tmpl - Issue 29454678: Issue 5085 - Add edgeInfo.js template for edge specific builds

Delta Between Two Patch Sets: templates/edgeInfo.js.tmpl

Issue 29454678: Issue 5085 - Add edgeInfo.js template for edge specific builds (Closed)

Left Patch Set: remove chrome specific code from edge template, and edge specific from chrome Created June 7, 2017, 11:06 a.m.

Right Patch Set: remove unecessary variable Created June 8, 2017, 3:46 p.m.

Left:
Right:

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

Left: Side by side diff | Download
Right: Side by side diff | Download

LEFT	RIGHT
1 /* This Source Code Form is subject to the terms of the Mozilla Public	1 /* This Source Code Form is subject to the terms of the Mozilla Public

2 * License, v. 2.0. If a copy of the MPL was not distributed with this	2 * License, v. 2.0. If a copy of the MPL was not distributed with this

3 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */	3 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

4	4

5 "use strict";	5 "use strict";

6	6

7 let platform = "edgehtml";

8 let platformVersion = null;

9 let application = "edge";

10 let applicationVersion = "0";

11

12 let regexp = /(\S+)\/(\S+)(?:\s\(.?\))?/g;

13 let match;

14

15 while (match = regexp.exec(navigator.userAgent))

16 {

17 let app = match[1];

18 let ver = match[2];

19 platformVersion = ver;

20 }

21

22 // not a Chromium-based UA, probably modifed by the user
Jon Sonesen 2017/06/07 11:08:43 Not sure if I should change the comment, seems it Not sure if I should change the comment, seems it should have chromium replaed with edge?
23 if (!platformVersion)

24 {

25 application = "unknown";

26 applicationVersion = platformVersion = "0";

27 }

28

29 exports.addonName = {{ basename\|json }};	7 exports.addonName = {{ basename\|json }};

30 exports.addonVersion = {{ version\|json }};	8 exports.addonVersion = {{ version\|json }};

31	9

32 exports.application = application;	10 exports.application = "edge";
Sebastian Noack 2017/06/07 11:14:13 Since this code is only bundled with the build for Since this code is only bundled with the build for Microsoft Edge, there is no way that the application (this code is running on) is anything. If the user agent string indicates some other browser, it is wrong. So just hard-code "edge" here, and "edgehtml" for the platform below. Then the logic above can be simplified to merely extract the version out of the user agent string, using an inlined regexp, no need for a loop. Jon Sonesen 2017/06/07 11:41:38 Acknowledged. What about the logic if platform ver Show quoted text On 2017/06/07 11:14:13, Sebastian Noack wrote: > Since this code is only bundled with the build for Microsoft Edge, there is no > way that the application (this code is running on) is anything. If the user > agent string indicates some other browser, it is wrong. So just hard-code "edge" > here, and "edgehtml" for the platform below. > > Then the logic above can be simplified to merely extract the version out of the > user agent string, using an inlined regexp, no need for a loop. Acknowledged. What about the logic if platform version is null (where my comment above is)? would this just be wrong and we would want it to fail? Should it be removed? Sebastian Noack 2017/06/07 11:51:46 If the regular expression fails we should still se Show quoted text On 2017/06/07 11:41:38, Jon Sonesen wrote: > On 2017/06/07 11:14:13, Sebastian Noack wrote: > > Since this code is only bundled with the build for Microsoft Edge, there is no > > way that the application (this code is running on) is anything. If the user > > agent string indicates some other browser, it is wrong. So just hard-code > "edge" > > here, and "edgehtml" for the platform below. > > > > Then the logic above can be simplified to merely extract the version out of > the > > user agent string, using an inlined regexp, no need for a loop. > > Acknowledged. What about the logic if platform version is null (where my comment > above is)? would this just be wrong and we would want it to fail? Should it be > removed? If the regular expression fails we should still set "platformVersion" to "0" to indicate that it is unknown, "platform" should still be "edgehtml". Jon Sonesen 2017/06/07 13:09:28 Acknowledged. Show quoted text On 2017/06/07 11:51:46, Sebastian Noack wrote: > On 2017/06/07 11:41:38, Jon Sonesen wrote: > > On 2017/06/07 11:14:13, Sebastian Noack wrote: > > > Since this code is only bundled with the build for Microsoft Edge, there is > no > > > way that the application (this code is running on) is anything. If the user > > > agent string indicates some other browser, it is wrong. So just hard-code > > "edge" > > > here, and "edgehtml" for the platform below. > > > > > > Then the logic above can be simplified to merely extract the version out of > > the > > > user agent string, using an inlined regexp, no need for a loop. > > > > Acknowledged. What about the logic if platform version is null (where my > comment > > above is)? would this just be wrong and we would want it to fail? Should it be > > removed? > > If the regular expression fails we should still set "platformVersion" to "0" to > indicate that it is unknown, "platform" should still be "edgehtml". Acknowledged.
33 exports.applicationVersion = applicationVersion;	11 exports.applicationVersion = "0";

34	12

35 exports.platform = platform;	13 exports.platform = "edgehtml"

36 exports.platformVersion = platformVersion;	14 exports.platformVersion = "0";

	15

	16 let match = /\bEdge\/(\d+(?:\.\d+)?)\b/.exec(navigator.userAgent);

	17 if (match)

	18 exports.platformVersion = match[1];

LEFT	RIGHT