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

Issue 29465623: #1374 - Fix ZTE/UME browser detection (Closed)

Created:
June 14, 2017, 1:43 p.m. by mathias
Modified:
June 14, 2017, 3:08 p.m.
Reviewers:
Felix Dahlke
CC:
f.lopez, Fred, f.nicolaisen
Visibility:
Public.

Description

#1374 - Fix ZTE/UME browser detection

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -3 lines) Patch
M modules/filterserver/files/site.conf View 2 chunks +3 lines, -3 lines 1 comment Download

Messages

Total messages: 3
mathias
June 14, 2017, 1:43 p.m. (2017-06-14 13:43:17 UTC) #1
Felix Dahlke
LGTM https://codereview.adblockplus.org/29465623/diff/29465624/modules/filterserver/files/site.conf File modules/filterserver/files/site.conf (left): https://codereview.adblockplus.org/29465623/diff/29465624/modules/filterserver/files/site.conf#oldcode47 modules/filterserver/files/site.conf:47: set $use_alternative_resource_flags "ZTE"; Naming: We don't have to ...
June 14, 2017, 1:51 p.m. (2017-06-14 13:51:03 UTC) #2
mathias
June 14, 2017, 1:53 p.m. (2017-06-14 13:53:05 UTC) #3
On 2017/06/14 13:51:03, Felix Dahlke wrote:
>
https://codereview.adblockplus.org/29465623/diff/29465624/modules/filterserve...
> modules/filterserver/files/site.conf:47: set $use_alternative_resource_flags
> "ZTE";
> Naming: We don't have to fix that now, but I think this should rather be
> something like LIBABPANDROID, because it doesn't specifically target ZTE
users.
> That's what the `application=com.ume.browser.global` check is for.

Ok, I will keep that in mind and update the names accordingly when we're
applying the next modification or removing the single ZTE-only redirect above
(the one you mentioned in the ticket as should-stay-as-is-for-now).

Powered by Google App Engine
This is Rietveld