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

Issue 29397638: Issue 5057 - Update adblockpluscore dependency to revision 3bdddf0e8343 (Closed)

Created:
March 29, 2017, 2:21 p.m. by Wladimir Palant
Modified:
March 30, 2017, 10:13 a.m.
Reviewers:
kzar
Base URL:
https://hg.adblockplus.org/adblockplus
Visibility:
Public.

Description

Issue 5057 - Update adblockpluscore dependency to revision 3bdddf0e8343

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -13 lines) Patch
M chrome/content/ui/filters-backup.js View 1 chunk +14 lines, -12 lines 4 comments Download
M dependencies View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 3
Wladimir Palant
March 29, 2017, 2:21 p.m. (2017-03-29 14:21:20 UTC) #1
kzar
Again not too familiar with the code but it LGTM. Luckily only your adblockpluscore changes ...
March 30, 2017, 5:45 a.m. (2017-03-30 05:45:51 UTC) #2
Wladimir Palant
March 30, 2017, 10:13 a.m. (2017-03-30 10:13:12 UTC) #3
https://codereview.adblockplus.org/29397638/diff/29397639/chrome/content/ui/f...
File chrome/content/ui/filters-backup.js (right):

https://codereview.adblockplus.org/29397638/diff/29397639/chrome/content/ui/f...
chrome/content/ui/filters-backup.js:109: item.addEventListener("command",
function()
On 2017/03/30 05:45:51, kzar wrote:
> Nit: Any reason not to use an arrow function here?

I'd rather not make unnecessary changes to this code.

https://codereview.adblockplus.org/29397638/diff/29397639/chrome/content/ui/f...
chrome/content/ui/filters-backup.js:113:
this.restoreInsertionPoint.parentNode.insertBefore(item,
this.restoreInsertionPoint.nextSibling);
On 2017/03/30 05:45:51, kzar wrote:
> Nit: Long line.

Same here, I'd rather not make unnecessary changes to this code.

Powered by Google App Engine
This is Rietveld