Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(398)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 10 months ago by Jon Sonesen
Modified:
2 years, 10 months ago
Reviewers:
Sebastian Noack
CC:
kzar, Thomas Greiner
Visibility:
Public.

Description

Issue 5085 - Add edgeInfo.info template for edge specific builds

Patch Set 1 : #

Total comments: 2

Patch Set 2 : remove chrome specific code from edge template, and edge specific from chrome #

Total comments: 7

Patch Set 3 : readd chrome logic, remove more chrome specific code from edge tmpl #

Total comments: 4

Patch Set 4 : fix regexp in edge tmpl, remove platformVersion from chrome tmpl #

Total comments: 4

Patch Set 5 : remove unecessary variable #

Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -10 lines) Patch
M templates/chromeInfo.js.tmpl View 1 2 3 2 chunks +1 line, -9 lines 0 comments Download
A templates/edgeInfo.js.tmpl View 1 2 3 4 1 chunk +18 lines, -0 lines 0 comments Download
M templates/modules.js.tmpl View 1 chunk +3 lines, -1 line 0 comments Download

Messages

Total messages: 15
Jon Sonesen
2 years, 10 months ago (2017-06-02 16:00:22 UTC) #1
Jon Sonesen
Perhaps the commit message can be improved, let me know what you think, Thanks!
2 years, 10 months ago (2017-06-02 16:05:37 UTC) #2
Jon Sonesen
On 2017/06/02 16:05:37, Jon Sonesen wrote: > Perhaps the commit message can be improved, let ...
2 years, 10 months ago (2017-06-02 16:07:16 UTC) #3
Sebastian Noack
https://codereview.adblockplus.org/29454678/diff/29454684/templates/edgeInfo.js.tmpl File templates/edgeInfo.js.tmpl (right): https://codereview.adblockplus.org/29454678/diff/29454684/templates/edgeInfo.js.tmpl#newcode21 templates/edgeInfo.js.tmpl:21: if (app != "Mozilla" && app != "AppleWebKit" && ...
2 years, 10 months ago (2017-06-06 16:36:39 UTC) #4
Jon Sonesen
https://codereview.adblockplus.org/29454678/diff/29454684/templates/edgeInfo.js.tmpl File templates/edgeInfo.js.tmpl (right): https://codereview.adblockplus.org/29454678/diff/29454684/templates/edgeInfo.js.tmpl#newcode21 templates/edgeInfo.js.tmpl:21: if (app != "Mozilla" && app != "AppleWebKit" && ...
2 years, 10 months ago (2017-06-07 11:08:00 UTC) #5
Jon Sonesen
https://codereview.adblockplus.org/29454678/diff/29458565/templates/edgeInfo.js.tmpl File templates/edgeInfo.js.tmpl (right): https://codereview.adblockplus.org/29454678/diff/29458565/templates/edgeInfo.js.tmpl#newcode22 templates/edgeInfo.js.tmpl:22: // not a Chromium-based UA, probably modifed by the ...
2 years, 10 months ago (2017-06-07 11:08:43 UTC) #6
Sebastian Noack
https://codereview.adblockplus.org/29454678/diff/29458565/templates/chromeInfo.js.tmpl File templates/chromeInfo.js.tmpl (left): https://codereview.adblockplus.org/29454678/diff/29458565/templates/chromeInfo.js.tmpl#oldcode20 templates/chromeInfo.js.tmpl:20: if (app == "Chrome") This part is still relevant ...
2 years, 10 months ago (2017-06-07 11:14:13 UTC) #7
Jon Sonesen
https://codereview.adblockplus.org/29454678/diff/29458565/templates/chromeInfo.js.tmpl File templates/chromeInfo.js.tmpl (left): https://codereview.adblockplus.org/29454678/diff/29458565/templates/chromeInfo.js.tmpl#oldcode20 templates/chromeInfo.js.tmpl:20: if (app == "Chrome") On 2017/06/07 11:14:13, Sebastian Noack ...
2 years, 10 months ago (2017-06-07 11:41:38 UTC) #8
Sebastian Noack
https://codereview.adblockplus.org/29454678/diff/29458565/templates/edgeInfo.js.tmpl File templates/edgeInfo.js.tmpl (right): https://codereview.adblockplus.org/29454678/diff/29458565/templates/edgeInfo.js.tmpl#newcode32 templates/edgeInfo.js.tmpl:32: exports.application = application; On 2017/06/07 11:41:38, Jon Sonesen wrote: ...
2 years, 10 months ago (2017-06-07 11:51:46 UTC) #9
Jon Sonesen
https://codereview.adblockplus.org/29454678/diff/29458565/templates/edgeInfo.js.tmpl File templates/edgeInfo.js.tmpl (right): https://codereview.adblockplus.org/29454678/diff/29458565/templates/edgeInfo.js.tmpl#newcode32 templates/edgeInfo.js.tmpl:32: exports.application = application; On 2017/06/07 11:51:46, Sebastian Noack wrote: ...
2 years, 10 months ago (2017-06-07 13:09:29 UTC) #10
Sebastian Noack
https://codereview.adblockplus.org/29454678/diff/29458612/templates/chromeInfo.js.tmpl File templates/chromeInfo.js.tmpl (left): https://codereview.adblockplus.org/29454678/diff/29458612/templates/chromeInfo.js.tmpl#oldcode16 templates/chromeInfo.js.tmpl:16: { The "platform" variable defined above isn't necessary anymore. ...
2 years, 10 months ago (2017-06-07 13:26:13 UTC) #11
Jon Sonesen
https://codereview.adblockplus.org/29454678/diff/29458612/templates/chromeInfo.js.tmpl File templates/chromeInfo.js.tmpl (left): https://codereview.adblockplus.org/29454678/diff/29458612/templates/chromeInfo.js.tmpl#oldcode16 templates/chromeInfo.js.tmpl:16: { On 2017/06/07 13:26:12, Sebastian Noack wrote: > The ...
2 years, 10 months ago (2017-06-07 13:56:07 UTC) #12
Sebastian Noack
https://codereview.adblockplus.org/29454678/diff/29458638/templates/edgeInfo.js.tmpl File templates/edgeInfo.js.tmpl (right): https://codereview.adblockplus.org/29454678/diff/29458638/templates/edgeInfo.js.tmpl#newcode9 templates/edgeInfo.js.tmpl:9: let match = /\bEdge\/(\S+(?:\.\d+)?)\b/.exec(navigator.userAgent); \S matches any non-whiespace characters, ...
2 years, 10 months ago (2017-06-08 08:52:30 UTC) #13
Jon Sonesen
https://codereview.adblockplus.org/29454678/diff/29458638/templates/edgeInfo.js.tmpl File templates/edgeInfo.js.tmpl (right): https://codereview.adblockplus.org/29454678/diff/29458638/templates/edgeInfo.js.tmpl#newcode9 templates/edgeInfo.js.tmpl:9: let match = /\bEdge\/(\S+(?:\.\d+)?)\b/.exec(navigator.userAgent); On 2017/06/08 08:52:30, Sebastian Noack ...
2 years, 10 months ago (2017-06-08 15:47:29 UTC) #14
Sebastian Noack
2 years, 10 months ago (2017-06-08 15:51:43 UTC) #15
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5