[chrome] Even with no filters, breaks styling of multiple disabled style sheets

[Lucent]

Environment

Mac OS / Debian
Chrome 34, 35 and 36 tested.
AdBlock Plus 1.7.4 is the only enabled extension.
Zero filter subscriptions.
Zero filter rules.

How to reproduce

1. Load www.ptable.com
2. Many styles are broken compared to the site with ABP disabled for the domain. Notice the tiny fonts for the legend area just below the tabs. Reload if all looks normal.
3. Click "Electrons" checkbox in top right.

Observed behaviour

Site loses all styles.

Expected behaviour

Site looks normal and is properly styled.

This is reproducible every time for me. It may take one reload to show the damaged site.

I believe this has to do with the way this site uses style sheets. It has 4 sheets which are then selectively disabled when the checkboxes are clicked. ABP is interfering with this disabling and enabling in some way.

[Lucent]

Further confirmed it happens every time with Chrome 33 and 34 on Mac OS.

[Lucent]

Environment is not limited to Mac OS or those Chrome versions. Occurs on Windows Chrome 35 and 36 and Mac OS Chrome 33 and 34, in my testing.

[sebastian]

I reproduced it with Chrome 34 on Linux. Also I figured out:

• It has something to do with injecting a style tag into the document. If I remove that line from include.preload.js the website renders correctly.
• For some reason the website also renders correctly after you focused the address bar and pressed enter (but not when you reload it another way).

[Lucent]

Now I know why it breaks. This site references its style sheets by document order to enable and disable. AdBlock must be injecting a sheet before the site's first sheet, so when the site turns off stylesheets[0] or [1] when clicking Electrons, it's actually disabling the AdBlock sheets rather than its own.

I can't explain the second bullet point.

[sebastian]

Replying to Lucent:

Now I know why it breaks. This site references its style sheets by document order to enable and disable.

I also assume something like that. Though I didn't found yet any code like that on ptable.com. So If you know more than I do, could you please point me to the responsible code?

AdBlock must be injecting a sheet before the site's first sheet, so when the site turns off stylesheets[0] or [1] when clicking Electrons, it's actually disabling the AdBlock sheets rather than its own.

Our style tag is actually always the first. I think in this case it should be the last. However we can't do that, because when our styles are injected there are no other stylesheets in the DOM yet. And if we would insert our styles later, this will have the effect of showing ads for a short moment, before hiding them on some web pages. Also other webpages might have issues if our stylesheet is the last instead of the first.

I can't explain the second bullet point.

Yeah, after a little more testing, it seems to be a random phenomenon. I can't reliable reproduce it anymore.

[Lucent]

Here is the relevant code:

var click_checkbox = function(obj) {
    var sheet_ids = {"Electrons": 0, "Name": 1, "Weight": 2, "Borders": 3}
    var reverse_ids = ["Electrons", "Name", "Weight", "Borders"];
    for (var i = sheet_ids[obj.id]; i >= 0 && i < reverse_ids.length; i += (obj.checked - 0.5) * 2)
        document.styleSheets[i + 1].disabled = document.getElementById(reverse_ids[i]).checked = obj.checked;

The specifics don't seem too relevant. Any code that hits document.styleSheets[0] expecting to manipulate its own sheets is going to have problems, no?

[sebastian]

Replying to Lucent:

Here is the relevant code:

Thanks.

The specifics don't seem too relevant. Any code that hits document.styleSheets[0] expecting to manipulate its own sheets is going to have problems, no?

Yes it is, but inserting our stylesheet at the end, would break web pages that do something like document.styleSheets[document.styleSheets - 1] and expect their stylesheet to be the last stylesheet in the document. But more importantly, in order to already block ads while the page is loading, we have to insert our stylesheet just after the web page began to load and the DOM is still nearly empty.

However we can use the ​Shadow DOM in order to hide our stylesheet from the web page, that it doesn't show up at all in document.styleSheets.

[mapx]

Adblock Plus 1.7.4.1151 broken for chrome 32
it's ok for chrome 33, chrome 34

-clean installing chrome 32 portable
-easylist
-no custom filters

any page you visit you can see the content loading and after a quick flash the page will turn white (empty)

[trev]

The commit here was ​https://hg.adblockplus.org/adblockpluschrome/rev/b327c6e0fafe

[trev]

Sounds like Chrome 32 doesn't support the <shadow> element. Checking typeof HTMLShadowElement might be good enough to distinguish versions with proper support from those without it.

[trev]

Actually, my suspicion above is wrong - Chrome 32 does support the <shadow> element, even Chrome 28 does. The approach with the Shadow DOM implemented here seems to work correctly for me in Chrome 28.

[sebastian]

Replying to mapx:

Adblock Plus 1.7.4.1151 broken for chrome 32
it's ok for chrome 33, chrome 34

-clean installing chrome 32 portable
-easylist
-no custom filters

any page you visit you can see the content loading and after a quick flash the page will turn white (empty)

I can't reproduce that with Chromium 32.0.1659.0 on Windows. Can you please show me where you downloaded Chrome 32?

Replying to trev:

Sounds like Chrome 32 doesn't support the <shadow> element. Checking typeof HTMLShadowElement might be good enough to distinguish versions with proper support from those without it.

Nice try, but HTMLShadowElement exists on Chrome 32.

[mapx]

chrome portable 32 from:
​http://sourceforge.net/projects/portableapps/files/Google%20Chrome%20Portable/Additional%20Versions/

or srware iron 32 from:
​http://sourceforge.net/projects/portableapps/files/Iron%20Portable/

iron 33 it's ok, no issues

[mapx]

the strange thing is chrome (iron) 31 is ok !
I can test here only iron (but iron = chrome ...)

all the tests were done using fresh portable installations

[sebastian]

Replying to mapx:

chrome portable 32 from:
​http://sourceforge.net/projects/portableapps/files/Google%20Chrome%20Portable/Additional%20Versions/

The installer for Chrome 32 fails.

or srware iron 32 from:
​http://sourceforge.net/projects/portableapps/files/Iron%20Portable/

Iron 32 crashes on first startup on my Windows 7 64-bit VM.

Portable apps are installed in a contained directory, right? So could you please attach a zip file with the affected version of Chrome to this ticket?

Also what is your exact Chrome version including build number (see chrome://version)

[mapx]

well, chrome installer fails too for me (I thought was due to the corporate proxy ..)

I tested on the stable installed version of iron 32 ==> Version 32.0.1750.1 (250000)
and IronPortable_32.0.1750.1.paf.exe in windows xp

The portable was installed in a folder on my desktop (windows xp)
What do you want to put in the zip file ? the whole folder ? there are 140 MB

[mapx]

perhaps you should run it (iron portable) in the normal windows (no VM)

[sebastian]

Same with Windows 7 (64-bit) on a real machine.

Since Chromium, I have tested with, comes without Flash, can you please try whether this also occurs when you disable Flash and similar plugins under chrome://plugins.

Following Chromium built seems to be closest to the version you have. I can't reproduce it with that built. Can you please try whether you can reproduce with the same Chromium built?
​https://commondatastorage.googleapis.com/chromium-browser-snapshots/Win/248367/chrome-win32.zip

Also does that happen on all websites all the time? Otherwise please provide some scenarios where you could reliable reproduce it.

[mapx]

the chromium downloaded from the link above is Version 34.0.1818.0 (248367)

I tried with the flash disabled ==> the same issue

yes, it happens for all sites.

on google is something interesting:
-if I check "allow some non intrusing advertising" ==> the issue is gone

on google site in inspect, some parameters for a table section:

(ABP 1149) or (ABP 1155 with non intrusing ADS)
<table cellspacing="0" cellpadding="0" class="gstl_0 gssb_c" style="width: 484px; display: none; top: 339px; left: 408px; position: absolute;"><tbody><tr><td class="gssb_f"></td><td class="gssb_e" style="width: 100%;"></td></tr></tbody></table>

(ABP 1155 without non intrusing ADS)
<table cellspacing="0" cellpadding="0" class="gstl_0 gssb_c" style="width: 100%; display: none; top: 27px; left: 0px; position: absolute;"><tbody><tr><td class="gssb_f"></td><td class="gssb_e" style="width: 100%;"></td></tr></tbody></table>

[mapx]

I found a chromium 32 portable ==> the same bug ...
​https://downloads.sourceforge.net/crportable/ChromiumPortable_32.0.1700.6.paf.exe

the table section in this case:

ABP 1155 (empty page)

<table cellspacing="0" cellpadding="0" class="gstl_0 gssb_c" style="width: 1px; top: 29px; position: absolute; left: 0px;"><tbody><tr><td class="gssb_f"></td><td class="gssb_e" style="width: 100%; display: none;"></td></tr><tr style="height: 0px;"><td></td><td><div id="pocs" style="display: none; left: 0px; white-space: nowrap; position: absolute;"><div id="pocs0"><span><span>Google</span>

ABP 1155 with non intrusing ads (normal page)

<table cellspacing="0" cellpadding="0" class="gstl_0 gssb_c" style="width: 571px; top: 339px; position: absolute; left: 357px;"><tbody><tr><td class="gssb_f"></td><td class="gssb_e" style="width: 100%; display: none;"></td></tr><tr style="height: 0px;"><td></td><td><div id="pocs" style="display: none; left: 0px; white-space: nowrap; position: absolute;"><div id="pocs0"><span><span>Google</span>

[sebastian]

Replying to mapx:

the chromium downloaded from the link above is Version 34.0.1818.0 (248367)

You are right, sorry.

on google is something interesting:
-if I check "allow some non intrusing advertising" ==> the issue is gone

That's because of no Shadow DOM is created, since no element hiding styles are injected, in that case.

Replying to mapx:

I found a chromium 32 portable ==> the same bug ...
​https://downloads.sourceforge.net/crportable/ChromiumPortable_32.0.1700.6.paf.exe

Thanks. Finally an affected build that works in my Windows 7 VM.

I have reproduced the issue and the problem is that the <shadow> element is ignored.

So far this seems to only affect Chrome 32.0.1700, while other Chrome 32.0.* versions and even older versions of Chrome seem to be not affected. Please let me know if you can reproduce that issue with other versions of Chrome.

We could workaround that issue by using the <content> instead of <shadow> element, but this would break pages that create an own shadow root on the document element. So I would rather like to don't use the Shadow DOM code and inject a regular style tag to the DOM as we did before, on the affected version(s) of Chrome. However detecting broken <shadow> behavior seems to be tricky.

[sebastian]

There don't seem to be a way to detect the broken <shadow> behavior. However since it was only reproduced in Chrome 32.0.1700 so far, I have hardcoded that version, in order to fall back to the legacy element hiding code, that works without Shadow DOM.

Note that the original bug from the description of this issue, is only fixed when we can use Shadow DOM, and therefore won't work on Chrome 32.0.1700 and all versions of Safari.

​https://hg.adblockplus.org/adblockpluschrome/rev/91c1f3f4b896

[mapx]

well, ... it does not work for iron Version 32.0.1750.1 (250000) for hard coding I suppose but ... doesnt matter, I'll update my iron or pass to chrome 34.

chromium 32 is ok.

[mapx]

my User Agent String is:
Mozilla/5.0 (Windows NT 5.1) AppleWebKit/537.36 (KHTML, like Gecko) Iron/32.0.1750.1 Chrome/32.0.1750.1 Safari/537.36

[sebastian]

I just reproduced it with Chromium 32.0.1690. So probably even more builds are affected. In order to be safe I changed the code to only check for the major version 32, ignoring the build version. Most of the earlier builds of Chrome 32 that aren't broken, are probably unreleased and beta builds anyway.

​https://hg.adblockplus.org/adblockpluschrome/rev/1ac2c27b3893

[mapx]

yep, I think it's the best approach

[whydoyouaddashadowroot]

copied from ​https://adblockplus.org/forum/viewtopic.php?f=10&t=25683&p=108844#p108844;

When Adblock Plus is enabled, every webpage has "#shadow-root" being added to the markup in Inspect Element / F12 in Chrome (but not FF, IE etc) and odd shadow-dom behaviour. Disabling the extension removes the shadow-root problem for all pages upon refresh.
This has been extremely fustrating when webdeveloping trying to figure where the heck the problem was coming from. >:(

IMO this 'fix' behaviour is far worse than the original bug which affected seemingly a few sites using a weird css listing/disabling method.

So now instead of the 4 in a billion sites that want to use some css obscuring method for their stylesheets, a #shadow-root is turned on for every single webpage in chrome?
This is crazy for anyone trying to troubleshoot site web development.

If this is the only way to fix the issue with the sites, surely the #shadow-root feature should/could be an optional setting for the sites that require it?
Currently I must disable adblock as this is too irritating to deal with it messing with several <head> elements.

Please consider reverting or at least making this optional with the default as setting as disabled.

[sebastian]

Replying to whydoyouaddashadowroot:

When Adblock Plus is enabled, every webpage has "#shadow-root" being added to the markup in Inspect Element / F12 in Chrome (but not FF, IE etc)

This isn't necessary on Firefox, since we can just add user stylesheets there.

As soon (if ever) Chrome lets extensions add user stylesheets (per frame) we will use them there as well. But until then shadow DOM is the only way to eliminate side-effects of element hiding.

Internet Explorer and Safari don't have shadow DOM, neither do they allow extensions to add user stylesheets. Hence some websites like this one are broken there, and we can't do anything about it.

and odd shadow-dom behaviour. Disabling the extension removes the shadow-root problem for all pages upon refresh.
This has been extremely fustrating when webdeveloping trying to figure where the heck the problem was coming from. >:(
IMO this 'fix' behaviour is far worse than the original bug which affected seemingly a few sites

I could also argue that the issue (if it can be considered as such) with using shadow DOM, affects virtually no users, but only those which are web developers, and are more confused by a shadow root than by a <style> element injected by an extension. ;)

using a weird css listing/disabling method

It is even more weird when an extension breaks websites that rely on their DOM hierarchy, regardless whether it is a good idea in general to write JavaScript like they do.

If this is the only way to fix the issue with the sites, surely the #shadow-root feature should/could be an optional setting for the sites that require it?

Most users don't care about how we modify the DOM, but don't want any unexpected side-effects when browsing the web. So using shadow DOM makes sense for them.

We could still consider adding an option for advanced users to disable Shadow DOM. But I'm not convinced yet that it would be useful. The problem (as I understand it) is that some web developers are confused by DOM modifications our extension does. However, whether we add a shadow root or modify the light DOM directly shouldn't matter much.