Regression: Websites don't render when Adblock is installed and "Acceptable Ads" is disabled |
||||||||||||
Issue descriptionChrome Version: 66.0.3327.0 Canary OS: macOS 10.12.6 What steps will reproduce the problem? (1) Install AdblockForChrome from https://chrome.google.com/webstore/detail/adblock/gighmmpiobklfepjocnamgkkbiglidom (2) Disable "Acceptable Ads" in Adblock's options (3) Visit e.g. https://www.google.com/ What is the expected result? Website doesn't render. What happens instead? Website should render. Please use labels and text to provide additional information. This is the regression range: https://chromium.googlesource.com/chromium/src/+log/ddbc46dff7954067faa1da9afba766446752a8c4..b9e6a930bcbe67d4aab2febdbbcd25babc5fa7f9
,
Jan 22 2018
Able to reproduce the issue on Windows 10. Mac OS 10.12.6 and Ubuntu 14.04 on latest Canary 66.0.3327.0 by following the steps mentioned in the original comment. Bisect Information: ==================== Good build: 65.0.3325.0 (Revision - 530369) Bad Build : 66.0.3326.0 (Revision - 530735) By executing the per-revision bisect script. below is the Changelog URL. Change Log URL: ================ https://chromium.googlesource.com/chromium/src/+log/edec191b14134554835fbfee2c34324f0850808a..9494d72752c2199295be01cbe066ff4b39a5a6ec From the above change log suspecting the below change Review-Url: https://chromium-review.googlesource.com/778402 m.jethani@ - Can you please check if the this issue is caused with respect to your change, else help us in assigning to the right owner. Adding ReleaseBlock-Stable as this is a recent regression. Please feel to remove the same if it is not applicable. Thanks..
,
Jan 22 2018
Don't know if this is related, but on Chrome Canary 66.0.3328.0 on Mac and 66.0.3327.2 on Windows 10 with AdBlock Plus + easy list enabled ("Acceptable Ads" makes no difference) when accessing any site, being with ads, simple html with no content, etc. the page is being overwritten by the extension CSS.
It started to happen couple of days ago, if not mistaken since 66.0.3327.0 on Mac
,
Jan 22 2018
It seems that Adblock Plus automatically switches to user style sheets after the change, but for some reason the CSS rule it injects is applying to every element on the page. This may very well be a bug in Adblock Plus (more specifically one of the filters). I'm investigating.
,
Jan 22 2018
What I've seen so far is that it happens only when Easylist is active. Using different list does not make pages blank or using other version of Chrome Canary or normal Chrome. I've opened a case (https://github.com/easylist/easylist/issues/851) for them as well, but really don't know is a Canary, Easylist or ABP problem. It all started when got updates at Saturday on both Canary and Easylist. ABP version looks pretty old.
,
Jan 22 2018
Adblock Plus detects user style sheet support and uses the feature if it's available. Since this landed on January 18, Adblock Plus running on Canary now automatically uses user style sheets. But it seems either the implementation in Chromium has a bug or there's something wrong with one of the filters.
,
Jan 22 2018
m.jethani@: Are you working on a fix for this issue?
,
Jan 22 2018
mehmet@ I'm looking into it, but if anybody wants to take it up then go for it.
,
Jan 22 2018
m.jethani@ If it takes too long from your side for a fix, maybe reversing it would help at the moment, since Adblock (+) is one of the most used extensions. Thank you :-)
,
Jan 22 2018
mehmet@ how long is too long? Another option is to do a new release of Adblock Plus (I do work on the extension). Also by the way there's no need to revert the change, we can simply disable user style sheet support with one line of code. You think I should submit a patch for that?
,
Jan 22 2018
I didn't know that you are working for the Adblock (+) extension too. I can't tell you what's the right way (either fixing it in Chrome or in the extension) since I am not a Developer. BTW: I reported a bug report also yesterday at https://help.getadblock.com/support/tickets/19880
,
Jan 22 2018
,
Jan 22 2018
Issue 804407 has been merged into this issue.
,
Jan 22 2018
Issue 804280 has been merged into this issue.
,
Jan 23 2018
Issue 804278 has been merged into this issue.
,
Jan 23 2018
,
Jan 23 2018
Just got an update to 66.0.3329.1 but seams the problem is still there. One difference is that with 66.0.3328.0 if a tab was left open and restored with the next browser opening, "everything" was loading fine. Today this does not happen.
,
Jan 23 2018
This is not a regression, the issue exists in Chrome 63 as well. Please see the attached example. Unpack and load the attached extension, then visit any page. You'll find that the page appears blank. If you remove the last CSS selector in adblock.css, reload the extension and then the page, you'll find that the page renders correctly. Since this is not regression due to any of my changes, I'll remove myself as the owner for now.
,
Jan 23 2018
There appears to be a limit on the number of selectors a rule can have. In the attached example, there are 10,222 selectors. If we remove one, it works. I haven't been able to figure out yet what this limit is, whether it's the number of selectors, the total number of characters that make up the list of selectors, or something else. We may have to implement a workaround for this in Adblock Plus.
,
Jan 23 2018
As far as I know/remember different browsers (years ago) used to have some limitations for selectors and file sizes for CSS (GGL search and will get some numbers) but currently don't know. For this reason if something goes further or closer to that number, better split it on few chunks, lines or files even it applies for the same property. This can prevent from such happenings. Just to comment, I think is insane to keep such amount of selectors within a single space.
,
Jan 23 2018
futhark@ is there a limit to the number of selectors that can be used in a style sheet? Also any clues what part of the code to look into for this?
,
Jan 23 2018
It turns out you don't even need an extension to reproduce this issue, a simple web page would do: <!DOCTYPE html> <!-- Link to adblock.css from the example --> <link rel="stylesheet" href="adblock.css"> Hello Anyway, now since Adblock Plus is running into this, we are thinking of doing a new release with a patch.
,
Jan 23 2018
Issue 804704 has been merged into this issue.
,
Jan 24 2018
Things start breaking when you have more than 8191 simple selectors in a single StyleRule: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/RuleSet.h?l=120-124 This has typically been WontFixed before. I don't know what caused the regression that this issue claims.
,
Jan 24 2018
Issue 805250 has been merged into this issue.
,
Jan 24 2018
@futhark there is bad bug here. Suppose you have a selector list larger than 8,192 selectors, in which the selector at index 0 is "#foo:not(textarea)" and the one at index 8192 is "#bar". Internally all the selectors are parsed, but RuleData can only look up selectors up to index 8191. When the code tries to look up the selector at index 8192 via RuleData, it returns the text for "#foo:not(textarea)". This is OK. But now if there's a selector at index 8193 like "#baz", its RuleData returns ":not(textarea)" instead, because the previous selector at 8192 has its "last tag in history" set to true. The code in ElementRuleCollector then assumes that ":not(textarea)" is a complete selector -- and of course it matches almost everything on the page. So that's why the pages are coming up blank.
,
Jan 24 2018
Excuse me for maybe my stupid mortal question :) but as I suggested before and asking now, isn't it possible and wouldn't it work to split selectors in few so such things won't occur?
I mean, instead of having:
#selector1, #selector2.....#selector20000 { some css here; }
make it:
#selector1, ... #selector5000 {some css here;}
#selector5001, ... #selector10000 {same css here;}
#selector10001, ... #selector15000 {same css here;}
etc.
Or the limit is per selectors in file and does not matter how many "chunks" are there? (something I doubt because definitely 've seen large css files)
,
Jan 24 2018
georgi@ splitting the selector list will work as a workaround but that doesn't mean there's no bug in Chromium. Even if more than 8,192 selectors are not supported, it should fail gracefully, not like what it's doing now.
,
Jan 24 2018
Alright, I've uploaded a patch crrev.com/c/883702
,
Jan 24 2018
Well, I'd say that these limits are there for a reason, but if something is returned not as expected as in the current case, I can't comment because the background is unknown to me. But will repeat myself and say that having such a huge amount of selectors is insane and difficult to maintain. Don't know also how they are stored currently, but would say that if is a single file, some editors easily would go out of memory as well.
,
Jan 25 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5814e68d9cf56da28568855881c3d3944ada159e commit 5814e68d9cf56da28568855881c3d3944ada159e Author: Manish Jethani <m.jethani@eyeo.com> Date: Thu Jan 25 17:38:22 2018 Ignore any selectors at index 8192 or beyond Since the selector index field in RuleData is only 13 bits, we can only support lookups in the range 0-8191. We must ignore any selectors outside this range. BUG= 804179 , 632009 Change-Id: I11c0acfe27813d98f24f72fb7e50f13b326a0e84 Reviewed-on: https://chromium-review.googlesource.com/883702 Reviewed-by: Rune Lillesveen <futhark@chromium.org> Commit-Queue: Manish Jethani <m.jethani@eyeo.com> Cr-Commit-Position: refs/heads/master@{#531931} [modify] https://crrev.com/5814e68d9cf56da28568855881c3d3944ada159e/third_party/WebKit/Source/core/css/RuleSet.cpp
,
Jan 26 2018
There is still a problem. It is not happening on all pages, but on some. An example is Stack Overflow. Pick up a random question (https://stackoverflow.com/questions/48460483/android-studio-buttons-across-activities) and the content of the page is blank due same stylesheet as before. The header and footer are not interfered. Happened also to all wordpress installations: Example https://ma.tt/
,
Jan 26 2018
georgi@ the fix isn't in Canary yet https://chromium.googlesource.com/chromium/src.git
,
Jan 26 2018
hmmm thought it was already as things started to work correctly, but now saw that is because of what I previously commented about leaving tabs to restore :) Thanks and sorry!
,
Jan 27 2018
Issue 805355 has been merged into this issue.
,
Jan 29 2018
Tested this issue on Windows 10, Mac OS 10.12.6 and Ubuntu 14.04 on the latest Chrome Build 66.0.3334.0 by following the steps mentioned in the original comment. Can observe that on installing AdblockForChrome -> disabling "Acceptable Ads" in Adblock's options and navigating to a site in a new tab, the site is rendering as expected. Attached is the screen cast for reference. Hence adding TE verified labels as the fix is working as intended. Thanks..
,
Jan 30 2018
,
Jan 30 2018
Thank you for fixing the issue, m.jethani@.
,
Nov 28
Seems this bug is now back.... Ive had to disable AdBlock on five of my computers. Most of the time the issues is related to loading office 365.
,
Nov 28
,
Nov 28
,
Nov 29
More info on my issue: I get "Waiting for extension Adblock" at the bottom left of Chrome. Chrome version is: 70.0.3538.110 (official Build) (64-bit). AdBlock is the latest version just downloaded from the Chrome store this morning. Selecting "Allow some non-intrusive advertising" does not seem to make a difference at this point.
,
Nov 29
We're currently testing a fix for the "Waiting for extension..." issue. I hope to have it released quickly. https://help.getadblock.com/support/solutions/articles/6000210609-chrome-stops-responding-and-waiting-for-extension-adblock-appears-in-the-status-bar
,
Nov 29
abrown721@ the issue here was about CSS rules with too many selectors. We implemented fixes for this in both Chromium and Adblock Plus. The issue you're reporting now is a different one, see https://issues.adblockplus.org/ticket/7065 We have a fix for this and are going to roll it out soon.
,
Dec 3
So question, this is happening in Windows 7, not 10. And has the fix been released? And do I need to update ABlock Plus or Chrome?
,
Dec 4
abrown721@ please try out the latest version of Adblock Plus for Chrome: https://adblockplus.org/releases/adblock-plus-342-for-chrome-firefox-and-opera-released |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by meh...@chromium.org
, Jan 21 2018