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

Issue 29392555: Issue 5015 - Better handle broken element hide filters (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 8 months ago by kzar
Modified:
2 years, 6 months ago
Reviewers:
Sebastian Noack
CC:
Wladimir Palant
Visibility:
Public.

Description

Issue 5015 - Better handle broken element hide filters Based upon the ESLint changes in https://codereview.adblockplus.org/29374674/

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -5 lines) Patch
M include.preload.js View 2 chunks +33 lines, -5 lines 0 comments Download

Messages

Total messages: 2
kzar
Patch Set 1 To my surprise this didn't seem to have much, if any, effect ...
2 years, 8 months ago (2017-03-23 06:31:21 UTC) #1
Sebastian Noack
2 years, 8 months ago (2017-03-23 07:11:43 UTC) #2
As I mentioned in the issue, there isn't much of a point anymore to inject the
selectors in batches. In fact, Microsoft even asked us a while ago to inject all
selectors at once (for performance reasons), which however we couldn't do back
then because of Safari, but we probably should do now.

But then, if a filter is invalid, and we fall back to insert each out of ~20k
(instead of just 200) filters individually, this will quite impact the page load
and memory usage of every website (the invalid filter is active on, which likely
is every website). So if we'd do something like that I think we should rather
start injecting all selectors at once, and then bisect if it fails.

However, as I also pointed out in the issue, when (hopefully soon)
chrome.tabs.insertCSS starts using user stylesheets, and we move to that API, we
won't get any error in case there is an invalid filter anymore. Hence this
appears to be only a temporary solution anyway.
Sign in to reply to this message.

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