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

Issue 29358502: Issue 4547 - ABP edge 0.9.8 re-adds easylist (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years ago by Oleksandr
Modified:
3 years ago
Reviewers:
Sebastian Noack, kzar
Visibility:
Public.

Description

This is slightly bizzare - I have tried to figure out why this was commented out, but I did not find any reasons. It was commented out in the original code Microsoft sent us too, so it seems to be a long living bug already.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+12 lines, -12 lines) Patch
M lib/adblockplus.js View 1 chunk +12 lines, -12 lines 0 comments Download

Messages

Total messages: 4
Oleksandr
3 years ago (2016-10-21 22:40:40 UTC) #1
Sebastian Noack
LGTM. How did that code got commented out? I don't recall this being a change ...
3 years ago (2016-10-21 22:57:56 UTC) #2
Oleksandr
On 2016/10/21 22:57:56, Sebastian Noack wrote: > LGTM. How did that code got commented out? ...
3 years ago (2016-10-21 23:17:31 UTC) #3
Sebastian Noack
3 years ago (2016-10-22 00:52:10 UTC) #4
On 2016/10/21 23:17:31, Oleksandr wrote:
> On 2016/10/21 22:57:56, Sebastian Noack wrote:
> > LGTM. How did that code got commented out? I don't recall this being a
change
> we
> > did in between 0.9.6 and 0.9.7.
> > 
> > (Please prepare a devbuild, I will submit it tomorrow, I'm currently on the
> > phone)
> 
> This was commented out all the way in 0.9.4. That is the result of an upgrade
to
> chrome version 1.12 that was done when you guys went to Microsoft, I think.
> Could have been just an accidental "temporary" commenting out, I guess.

I think you are right, this logic has been disabled ever since (even before
0.9.4) since initially EasyList was downloaded on every startup (by separate
code) and other filter lists weren't supported. I'm still surprised that it
didn't caused a bug that was noticed before the last release.
Sign in to reply to this message.

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