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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 12 months ago by Sebastian Noack
Modified:
3 years, 12 months ago
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(); ...
3 years, 12 months ago (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 ...
3 years, 12 months ago (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: ...
3 years, 12 months ago (2015-12-15 17:31:31 UTC) #3
kzar
3 years, 12 months ago (2015-12-15 17:40:26 UTC) #4
Don't want to argue about nits so LGTM
Sign in to reply to this message.

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