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

Issue 6324527734718464: Issue 424 - Anti-adblock filterlist disabled on extension update (Platform) (Closed)

Created:
May 5, 2014, 2:34 p.m. by Thomas Greiner
Modified:
May 5, 2014, 3:59 p.m.
Reviewers:
Wladimir Palant
Visibility:
Public.

Description

Issue 424 - Anti-adblock filterlist disabled on extension update (Platform)

Patch Set 1 #

Patch Set 2 : #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -7 lines) Patch
M background.js View 1 1 chunk +10 lines, -7 lines 2 comments Download

Messages

Total messages: 2
Thomas Greiner
May 5, 2014, 2:35 p.m. (2014-05-05 14:35:33 UTC) #1
Wladimir Palant
May 5, 2014, 3:23 p.m. (2014-05-05 15:23:14 UTC) #2
LGTM with the comments below addressed. Please test and land ASAP.

http://codereview.adblockplus.org/6324527734718464/diff/5685265389584384/back...
File background.js (right):

http://codereview.adblockplus.org/6324527734718464/diff/5685265389584384/back...
background.js:224: if (!prevVersion || parseFloat(prevVersion) < 1.8)
Please use Services.vc.compare() here as well.

http://codereview.adblockplus.org/6324527734718464/diff/5685265389584384/back...
background.js:227: if (subscription)
As with the other review, this should be:

if (subscription && !(subscription.url in FilterStorage.knownSubscriptions))

Powered by Google App Engine
This is Rietveld