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

Issue 29332680: Issue 3415 - Detect application based on UA for Chromium-based browsers (Closed)

Created:
Dec. 15, 2015, 4:25 p.m. by Sebastian Noack
Modified:
Dec. 15, 2015, 6:07 p.m.
Reviewers:
kzar
CC:
Wladimir Palant
Visibility:
Public.

Description

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

Patch Set 1 : #

Total comments: 7

Patch Set 2 : Rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+47 lines, -17 lines) Patch
M chromeInfo.js.tmpl View 1 1 chunk +47 lines, -17 lines 0 comments Download

Messages

Total messages: 4
Sebastian Noack
https://codereview.adblockplus.org/29332680/diff/29332687/chromeInfo.js.tmpl File chromeInfo.js.tmpl (right): https://codereview.adblockplus.org/29332680/diff/29332687/chromeInfo.js.tmpl#newcode27 chromeInfo.js.tmpl:27: application = app == "OPR" ? "opera" : app.toLowerCase(); ...
Dec. 15, 2015, 4:36 p.m. (2015-12-15 16:36:58 UTC) #1
kzar
https://codereview.adblockplus.org/29332680/diff/29332687/chromeInfo.js.tmpl File chromeInfo.js.tmpl (right): https://codereview.adblockplus.org/29332680/diff/29332687/chromeInfo.js.tmpl#newcode10 chromeInfo.js.tmpl:10: var regexp = /(\S+)\/(\S+)(?:\s*\(.*?\))?/g; Nit: More descriptive name would ...
Dec. 15, 2015, 5:26 p.m. (2015-12-15 17:26:01 UTC) #2
Sebastian Noack
https://codereview.adblockplus.org/29332680/diff/29332687/chromeInfo.js.tmpl File chromeInfo.js.tmpl (right): https://codereview.adblockplus.org/29332680/diff/29332687/chromeInfo.js.tmpl#newcode10 chromeInfo.js.tmpl:10: var regexp = /(\S+)\/(\S+)(?:\s*\(.*?\))?/g; On 2015/12/15 17:26:00, kzar wrote: ...
Dec. 15, 2015, 5:31 p.m. (2015-12-15 17:31:31 UTC) #3
kzar
Dec. 15, 2015, 5:40 p.m. (2015-12-15 17:40:26 UTC) #4 
Don't want to argue about nits so LGTM

Powered by Google App Engine
This is Rietveld