New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 19 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 30
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Websites don't render when Adblock is installed and "Acceptable Ads" is disabled

Project Member Reported by meh...@chromium.org, Jan 21 Back to list

Issue description

Chrome 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


 
upps typo:

What is the expected result?
Website should render.

What happens instead?
Website doesn't render.


Cc: susanjun...@techmahindra.com
Components: Platform>Extensions
Labels: -Needs-Bisect hasbisect-per-revision ReleaseBlock-Stable Triaged-ET M-66 FoundIn-66 Target-66 RegressedIn-66 Needs-Triage-M66 OS-Linux OS-Windows
Owner: m.jeth...@eyeo.com
Status: Assigned
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..
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
Captura de pantalla 2018-01-22 a las 13.16.09.png
91.3 KB View Download
Captura de pantalla 2018-01-22 a las 13.15.57.png
93.4 KB View Download
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.
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.
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.
m.jethani@: Are you working on a fix for this issue? 
mehmet@ I'm looking into it, but if anybody wants to take it up then go for it.
Cc: shrike@chromium.org
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 :-)
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?
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
Cc: kbr@chromium.org piman@chromium.org
 Issue 804424  has been merged into this issue.
 Issue 804407  has been merged into this issue.
 Issue 804280  has been merged into this issue.
 Issue 804278  has been merged into this issue.
Status: Started
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.
Owner: ----
Status: Available
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.
bug-804179-example.zip
47.3 KB Download
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.
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.
Cc: futhark@chromium.org
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?
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.
 Issue 804704  has been merged into this issue.
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.

 Issue 805250  has been merged into this issue.
@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.
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)
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.
Owner: m.jeth...@eyeo.com
Status: Started
Alright, I've uploaded a patch crrev.com/c/883702
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.
Project Member

Comment 31 by bugdroid1@chromium.org, Jan 25

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

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/


georgi@ the fix isn't in Canary yet

https://chromium.googlesource.com/chromium/src.git
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!
Cc: apaci...@chromium.org nyerramilli@chromium.org rbasuvula@chromium.org
 Issue 805355  has been merged into this issue.
Labels: TE-Verified-M66 TE-Verified-66.0.3334.0
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..
804179_CL.webm
5.0 MB View Download
Status: Fixed
Thank you for fixing the issue, m.jethani@.

Sign in to add a comment