AdguardTeam / AdGuardForSafari

AdGuard for Safari app extension
GNU General Public License v3.0
1.05k stars 74 forks source link

WebAnnoyancesUltralist issues #390

Closed photonzoo closed 3 years ago

photonzoo commented 4 years ago

Prerequisites

Issue Details

Expected Behavior

Youtube channel pages (i.e. the page coming up when clicking on any video's channel name) should show up like before the upgrade from Safari 13 to 14. Specifically, when clicking on any of the channel's tabs (e.g. "Home" or "Videos") on any YT channel, the page's content (e.g. list of videos) should be fully visible.

Actual Behavior

Youtube channel pages are broken: the top banner and "menu bar" on a channel's page partly obscure the content below. The top-most content, e.g. from the list of the channel's videos, is hidden behind the banner and menu. The partly obscured content appears to be under the HTML tag '< ytd-grid-renderer class="style-scope ytd-item-section-renderer" mini-guide-visible="" >'. This behaviour occurs whether logged on to YT with a Google account or not, and on two macOS 10.15 systems, one of which has been completely freshly set up only now and is virtually pristine. Similarly broken layout can be observed on other web sites, too, but only on certain ones (no example at hand). I suspect Safari 14 breaking something, as this behaviour didn't show before the upgrade from 13. Only disabling AdGuard Advanced Blocking extension remedies the problem .

Screenshots

Screenshot 2020-10-09 at 01 21 05 Screenshot 2020-10-09 at 01 21 18 Screenshot 2020-10-09 at 01 21 42

Additional Information

Only disabling the AdGuard Advanced Blocking extension remedies the problem (although a warning shows up in AdGuard). Trying to disable any other AdGuard extension, or any filter list, does not help, though.

ameshkov commented 4 years ago

Could you please try disabling filter lists to see which one does that?

I suspect "Web Annoyances Ultralist".

photonzoo commented 4 years ago

Hi Andrey, Thank you for your quick response. It appears you're right, as disabling that Annoyances list makes the issue go away.

I've toggled that switch and then refreshed the browser tab, but sometimes (more often than not) I had to refresh several times for the change to take effect. The same is true when re-enabling that list again.

FYI, I also toggled it, while trying any of the following: deleted all browsing history (including cookies) and only then refreshing the page, switching AdGuard on/off from its browser icon before refreshing, and also shutting the browser down before restarting and reloading the page. None of these actions worked reliably upon he first page reload/refresh, usually I had to refresh the page a few times before the change was visible. It appears as if there's some kind of internal timing issue / race condition occurring, but not always.

Anyway, the problem is clearly correlated with that list. Could it be that I've loaded too many rules which may take too long to get parsed? Although no blocker shows more than ~33,000 rules activated, i.e. clearly less than Safari's limit of 50,000, and the "Social" one containing the "Web Annoyances Ultralist" has only 29,000 even with this list active.

Lastly one thing I forgot to mention earlier: the YT main page's search entry field appears to be affected by that issue, too. Upon page load, it briefly allows to enter text while the page is still getting fully loaded, but then won't accept any characters to be entered any more. This behaviour also correlates with toggling said filter list.

ameshkov commented 4 years ago

@yourduskquibbles could you please take a look? Is this our bug or is it a false positive by Ultralist?

yourduskquibbles commented 4 years ago

Hi @ameshkov I do not have access to MacOS computer to fully test but will dig in and verify not working incorrectly on iOS or windows PC (devices I have access too)

From my limited understanding of content blockers on Safari though, CSS style modifying filters do not get applied? If that is the case, it could be that the non-CSS modifying filters remaining from ultralist are removing some "empty" placeholder elements that are only needing to be removed when CSS filters do get applied.

If that is the case I would probably need to create another exception list to unbreak sites specifically for MacOS safari extension similar to what I have done with a couple sites for iOS that get loaded through an exclusion sublist included in ultralist.txt.

photonzoo commented 4 years ago

Hi @yourduskquibbles I am prepared to help by testing any beta or debug versions you may have available, or by sending log files etc. I want to send my freshly set-up system away in a week, and would very much welcome if this issue could be resolved by then.

ameshkov commented 4 years ago

@yourduskquibbles well, AG for Safari (unlike AG for iOS) is able to apply CSS rules so if there are no issues with other blockers, this might be our bug. Is there any particular place where you keep Youtube rules?

yourduskquibbles commented 4 years ago

Hi @ameshkov I don't see any issues with uBO in firefox or Chrome nor Adguard in Chrome with display on the test link https://www.youtube.com/channel/UC3EnRkFH1foTeapIY6ixijQ so leads me to believe this is a bug with AG for Safari.

Youtube rules are in a few places:

  1. cookie_filters.txt line 442

    youtube.com###consent-bump:remove()
  2. floating_filters.txt Lines 8846-8864 (Can't direct link to the line #'s as the file size is greater than github allows to view in their web editor)

    youtube.com###backgroundFrontLayer, #backgroundRearLayer:style(transform: translate3d(0px, 0px, 0px) !important;)
    youtube.com###chips-wrapper:style(position: static !important;)
    youtube.com###contentContainer.app-drawer:style(padding-top: 120px !important; padding-bottom: 0 !important;)
    youtube.com###contentContainer:style(padding-top: 0 !important;)
    youtube.com###header.ytd-c4-tabbed-header-renderer:style(position: absolute !important; left: 0 !important;)
    youtube.com###header:style(transform: none !important; margin-top: 0 !important;)
    youtube.com###masthead-container:style(position: absolute !important; transition: none !important; transform: none !important;)
    youtube.com###masthead-positioner, #yt-masthead-container:style(position: relative !important; top: 0 !important;)
    youtube.com###masthead-positioner-height-offset, .iv-branding.annotation-type-custom.annotation
    youtube.com###wrapper.app-header-layout > [slot="header"]:style(position: relative !important; left: 0 !important;)
    youtube.com##.appbar-guide-menu-layout, .gstl_50.sbdd_a, .js-yt-header.yt-header, ytd-mini-guide-renderer.ytd-app, app-drawer[opened=""][persistent=""], ytd-two-column-browse-results-renderer[page-subtype="history"] #secondary.ytd-two-column-browse-results-renderer:style(position: absolute !important;)
    youtube.com##.no-scroll #page-manager:style(margin-top: 0px !important)
    youtube.com##app-drawer:style(bottom: 0 !important; padding-bottom: 0 !important;)
    youtube.com##app-drawer[opened=""]:style(position: fixed !important;)
    youtube.com##ytd-app[is-watch-page][scrolling_][style^="--ytd-app-fullerscreen-scrollbar-width:17px; --ytd-masthead-height:0px;"] #masthead-container, ytd-app[scrolling_=""] #masthead-container:style(opacity: 0 !important;)
    youtube.com##ytd-feed-filter-chip-bar-renderer:style(margin-bottom: 0 !important;)
    youtube.com##ytd-guide-entry-renderer:style(display: block !important;)
    youtube.com##ytd-playlist-sidebar-renderer.ytd-browse:style(position: absolute !important; left: unset !important; height: 100% !important;)
    youtube.com##ytd-settings-sidebar-renderer.ytd-browse:style(position: absolute !important; left: 0 !important; height: 100% !important;)
  3. modal_filters.txt Lines 1002:1006

    youtube.com##.opened:remove()
    youtube.com##ytd-popup-container > paper-dialog > yt-upsell-dialog-renderer:remove()
    youtube.com##ytd-popup-container paper-dialog:has-text(/Changes to YouTube’s Terms of Service|Check out your new Library|Get access to exclusive perks|Get YouTube without the ads|How is your experience|How is YouTube today|Our Terms of Service have been updated|Rate your ads|Try it free|Try the YouTube Kids app|What is your biggest complaint|Which problems are you experiencing|You're signed out of YouTube|YouTube TV/)
    youtube.com##ytd-popup-container yt-bubble-hint-renderer:has-text(/Get access to exclusive perks/)
    youtube.com##ytd-popup-container yt-tooltip-renderer:has-text(/Choose what to play next|Join this channel and unlock/)
  4. social_filters.txt Line 1203

    youtube.com##.share-panel-services, .ytp-button.ytp-share-button
photonzoo commented 4 years ago

Just to clarify the situation, let me re-iterate: only disabling the "Advanced Blocking extension" in Safari, or removing the "Web Annoyances Ultralist" within AdGuard let the issue disappear. Maybe this helps in tracking down the culprit.

ameshkov commented 3 years ago

Renamed this issue.

The issue is present in AG for Safari only so there must be something wrong with the way we apply ExtCSS rules.

Similar issue with Twitter: https://github.com/AdguardTeam/AdguardFilters/issues/67396#issuecomment-733909520

yourduskquibbles commented 3 years ago

Issue in https://github.com/AdguardTeam/AdguardFilters/issues/67396#issuecomment-729060171 is most likely in floating_filters.txt list as this is where almost all CSS changes get applied throughout the entire list.

floating_filters.txt Lines 8121:8135 in current raw list but line # will likely change when someone is looking at this

twitter.com###header, #message-drawer, .js-topbar.topbar, .rn-1lgpqti.rn-zchlnj.rn-1xcajam.rn-12vffkv.rn-gy4na3.rn-1mdbw0j.rn-9aemit.rn-wk8lta.rn-bcqeeo.rn-ifefl9.rn-11wrixw.rn-p1pxzi.rn-61z16t.rn-1mnahxq.rn-1d2f490.rn-eqz5dr.rn-7vfszb.rn-1pxmb3b.rn-6koalj.rn-deolkf.rn-1p0dtai.rn-gxnn5r.rn-ndvcnb.rn-fnigne.rn-13yce4e.rn-mm0ijv.rn-rull8r.rn-14skgim.rn-1efd50x.rn-aqfbo4.rn-1oszu61, .r-1g40b8q.r-ipm5af.r-gtdqiz.r-qklmqi.r-rull8r.r-1igl3o0.r-kemksi.r-aqfbo4.css-1dbjc4n, .r-1g40b8q.r-ipm5af.r-gtdqiz.r-qklmqi.r-rull8r.r-1ila09b.r-yfoy6g.r-aqfbo4.css-1dbjc4n, .css-1dbjc4n.r-f9dfq4, .css-1dbjc4n.r-1gymjhz:style(position: relative !important;)
twitter.com###page-container, .topbar-spacer, .wrapper-search.AppContent:style(padding-top: 0 !important;)
twitter.com##.AppBar--overSignup.u-cf.AppBar--inverted.AppBar, .CoreLayout-header, .ProfileCanopy-inner, .SearchNavigation, .js-variableHeightTopBar.StreamsTopBar--fixed.StreamsTopBar, .u-bgUserColor.ProfileCanopy-header, .u-boxShadow.ProfileCanopy-navBar, [data-testid="sidebarColumn"] > div > div[style*="top: "]:style(position: relative !important; top: 0 !important;)
twitter.com##.ProfileCanopy-avatar:style(bottom: -87px !important;)
twitter.com##.ProfileCanopy-card, .u-cf.OpenInAppBar, header div[style*="height: calc(53px);"], header > .r-1h3ijdo.css-1dbjc4n
twitter.com##.ProfileCanopy-header.u-bgUserColor, .ProfileCanopy-header:style(margin-top: 0 !important;)
twitter.com##.ProfileCanopy-headerBg > img:style(transform: none !important;)
twitter.com##.global-nav, .topbar:style(border-bottom: 0 !important;)
twitter.com##.is-full.twtr-main.u01, .p01, .LiveEventPage-mediaContainer, .rn-136ojw6.rn-axxi2z.rn-o7ynqc.rn-1i36cah.rn-ipm5af.rn-zchlnj.rn-1xcajam.rn-gy4na3.rn-1mdbw0j.rn-9aemit.rn-wk8lta.rn-bcqeeo.rn-ifefl9.rn-11wrixw.rn-p1pxzi.rn-61z16t.rn-1mnahxq.rn-1d2f490.rn-eqz5dr.rn-7vfszb.rn-1pxmb3b.rn-6koalj.rn-deolkf.rn-gxnn5r.rn-fnigne.rn-13yce4e.rn-mm0ijv.rn-rull8r.rn-14skgim.rn-1efd50x.rn-pntssu.rn-1t0trkx.rn-1lrzzbc.rn-gmrdcm.rn-qklmqi.rn-44z8sh.rn-1oszu61, .rn-136ojw6.rn-axxi2z.rn-o7ynqc.rn-1i36cah.rn-ipm5af.rn-zchlnj.rn-1xcajam.rn-gy4na3.rn-1mdbw0j.rn-9aemit.rn-wk8lta.rn-bcqeeo.rn-ifefl9.rn-11wrixw.rn-p1pxzi.rn-61z16t.rn-1mnahxq.rn-1d2f490.rn-eqz5dr.rn-7vfszb.rn-1pxmb3b.rn-6koalj.rn-deolkf.rn-gxnn5r.rn-ndvcnb.rn-fnigne.rn-13yce4e.rn-mm0ijv.rn-rull8r.rn-14skgim.rn-1efd50x.rn-44z8sh.rn-1oszu61, .rn-1lgpqti.rn-1ovo9ad.rn-1xcajam.rn-gy4na3.rn-1mdbw0j.rn-9aemit.rn-wk8lta.rn-bcqeeo.rn-ifefl9.rn-11wrixw.rn-p1pxzi.rn-61z16t.rn-1mnahxq.rn-eqz5dr.rn-7vfszb.rn-1pxmb3b.rn-6koalj.rn-deolkf.rn-gxnn5r.rn-ndvcnb.rn-fnigne.rn-13yce4e.rn-mm0ijv.rn-rull8r.rn-14skgim.rn-1efd50x.rn-1oszu61, .r-136ojw6.r-1ovo9ad.r-ipm5af.r-1xcajam.r-1vsu8ta.r-15d164r.r-1h3ijdo.r-18u37iz.r-14lw9ot.r-aqfbo4.r-1awozwy.css-1dbjc4n, .r-136ojw6.r-1hycxz.r-ipm5af.r-1xcajam.r-1vsu8ta.r-15d164r.r-1h3ijdo.r-18u37iz.r-kemksi.r-aqfbo4.r-1awozwy.css-1dbjc4nm, .r-136ojw6.r-1hycxz.r-ipm5af.r-1xcajam.r-1vsu8ta.r-15d164r.r-1h3ijdo.r-18u37iz.r-yfoy6g.r-aqfbo4.r-1awozwy.css-1dbjc4n, .css-1dbjc4n.r-aqfbo4.r-1xcajam:style(position: absolute !important;)
twitter.com##.r-1g40b8q.r-ipm5af.r-gtdqiz.r-qklmqi.r-rull8r.r-my5ep6.r-14lw9ot.r-aqfbo4.css-1dbjc4n, .rn-gtdqiz, .r-ipm5af.r-1xcajam.r-1pi2tsx.r-aqfbo4.css-1dbjc4n, .r-1xcajam[style^="bottom"], .r-gtdqiz:style(position: static !important;)
twitter.com##.r-1hycxz.r-zchlnj.r-1xcajam.r-xaggoz.r-1mf7evn.r-117bsoe.r-y3da5r.r-1p0dtai.css-1dbjc4n:style(position: absolute !important; right: unset !important; bottom: 0 !important; width: 100% !important;max-width: 100% !important; margin-bottom: 0 !important; box-shadow: none !important;)
twitter.com##.r-1hycxz.r-zchlnj.r-1xcajam.r-xaggoz.r-1mf7evn.r-y3da5r.r-1p0dtai.css-1dbjc4n:style(position: absolute !important;  margin: 0 !important; width: 100% !important; max-width: unset !important;)
twitter.com##.r-1pi2tsx.css-1dbjc4n > .r-1hycxz.r-ipm5af.r-1xcajam.css-1dbjc4n, .r-136ojw6.r-1hycxz.r-ipm5af.r-1xcajam.r-1vsu8ta.r-15d164r.r-1h3ijdo.r-18u37iz.r-14lw9ot.r-aqfbo4.r-1awozwy.css-1dbjc4n, header > div:not([style^="height: calc(53px);"]):not(.column-title):not(.app-header-inner):style(position: relative !important;)
twitter.com##[data-testid="sidebarColumn"] .r-1xcajam, [data-testid="sidebarColumn"] .r-gtdqiz:style(position: static !important; top: 0 !important; bottom: unset !important)
twitter.com##div.css-1dbjc4n[style^="margin-top:"], html:style(margin-top: 0 !important;)

modal_filters.txt Line 928-929

twitter.com##.privacy-change-notice.Banner--aboveNav.Banner
twitter.com##div[class="r-1d2f490 r-u8s1d r-zchlnj r-ipm5af r-184en5c"][style=""] a[href="/i/flow/signup"][role="button"][data-testid="signup"]:upward(7)
yourduskquibbles commented 3 years ago

Using uBO and Firefox and looking at dev console I'm wondering if this bug in Safari might be caused because the element in question can be told to both apply position: static and also position: absolute at the same time? uBO correctly applies the more specific filter but maybe Adguard just does first it sees and discards the more specific filter?

When position: absolute is set that left navbar is "hidden" on the right side of the page and falls "under" the right navbar. Maybe something to test with just the uncondensed filters at bottom of this post.

twitterfilter_conflict

The two condensed consolidated filter lines from floating_filters.txt that are conflicting to test are below:

  1. twitter.com##.is-full.twtr-main.u01, .p01, .LiveEventPage-mediaContainer, .rn-136ojw6.rn-axxi2z.rn-o7ynqc.rn-1i36cah.rn-ipm5af.rn-zchlnj.rn-1xcajam.rn-gy4na3.rn-1mdbw0j.rn-9aemit.rn-wk8lta.rn-bcqeeo.rn-ifefl9.rn-11wrixw.rn-p1pxzi.rn-61z16t.rn-1mnahxq.rn-1d2f490.rn-eqz5dr.rn-7vfszb.rn-1pxmb3b.rn-6koalj.rn-deolkf.rn-gxnn5r.rn-fnigne.rn-13yce4e.rn-mm0ijv.rn-rull8r.rn-14skgim.rn-1efd50x.rn-pntssu.rn-1t0trkx.rn-1lrzzbc.rn-gmrdcm.rn-qklmqi.rn-44z8sh.rn-1oszu61, .rn-136ojw6.rn-axxi2z.rn-o7ynqc.rn-1i36cah.rn-ipm5af.rn-zchlnj.rn-1xcajam.rn-gy4na3.rn-1mdbw0j.rn-9aemit.rn-wk8lta.rn-bcqeeo.rn-ifefl9.rn-11wrixw.rn-p1pxzi.rn-61z16t.rn-1mnahxq.rn-1d2f490.rn-eqz5dr.rn-7vfszb.rn-1pxmb3b.rn-6koalj.rn-deolkf.rn-gxnn5r.rn-ndvcnb.rn-fnigne.rn-13yce4e.rn-mm0ijv.rn-rull8r.rn-14skgim.rn-1efd50x.rn-44z8sh.rn-1oszu61, .rn-1lgpqti.rn-1ovo9ad.rn-1xcajam.rn-gy4na3.rn-1mdbw0j.rn-9aemit.rn-wk8lta.rn-bcqeeo.rn-ifefl9.rn-11wrixw.rn-p1pxzi.rn-61z16t.rn-1mnahxq.rn-eqz5dr.rn-7vfszb.rn-1pxmb3b.rn-6koalj.rn-deolkf.rn-gxnn5r.rn-ndvcnb.rn-fnigne.rn-13yce4e.rn-mm0ijv.rn-rull8r.rn-14skgim.rn-1efd50x.rn-1oszu61, .r-136ojw6.r-1ovo9ad.r-ipm5af.r-1xcajam.r-1vsu8ta.r-15d164r.r-1h3ijdo.r-18u37iz.r-14lw9ot.r-aqfbo4.r-1awozwy.css-1dbjc4n, .r-136ojw6.r-1hycxz.r-ipm5af.r-1xcajam.r-1vsu8ta.r-15d164r.r-1h3ijdo.r-18u37iz.r-kemksi.r-aqfbo4.r-1awozwy.css-1dbjc4nm, .r-136ojw6.r-1hycxz.r-ipm5af.r-1xcajam.r-1vsu8ta.r-15d164r.r-1h3ijdo.r-18u37iz.r-yfoy6g.r-aqfbo4.r-1awozwy.css-1dbjc4n, .css-1dbjc4n.r-aqfbo4.r-1xcajam:style(position: absolute !important;)

  2. twitter.com##.r-1g40b8q.r-ipm5af.r-gtdqiz.r-qklmqi.r-rull8r.r-my5ep6.r-14lw9ot.r-aqfbo4.css-1dbjc4n, .rn-gtdqiz, .r-ipm5af.r-1xcajam.r-1pi2tsx.r-aqfbo4.css-1dbjc4n, .r-1xcajam[style^="bottom"], .r-gtdqiz:style(position: static !important;)

Uncondensed filters with just these two problem filters you can try to test with stock AdGuard without Web Annoyances Ultralist in Safari are below:

  1. twitter.com##.css-1dbjc4n.r-aqfbo4.r-1xcajam:style(position: absolute !important;)
  2. twitter.com##.r-ipm5af.r-1xcajam.r-1pi2tsx.r-aqfbo4.css-1dbjc4n:style(position: static !important;)

In the two uncondensed filters above 1. should not get applied to the left navbar as 2. is more specific

Atleast this is how I think they are being applied in firefox and chrome.

ameshkov commented 3 years ago

Yeah, this might be the reason indeed.

@tvinzz it seems that in AG for Safari we use ExtCSS to apply simple CSS rules. I guess that's okay, but in order to fix that, we need to make ExtCSS take selectors priority into account, i.e. the more specific selector has higher priority.

Alternatively, we could change the way we work with simple CSS rules and simply inject user-styles for them and just let the browser handle it.

yourduskquibbles commented 3 years ago

@tvinzz @ameshkov Another one to test from a popular site if you'd like. This one for youtube.com reported at https://github.com/AdguardTeam/AdguardFilters/issues/67963#issuecomment-734201662

Two "competing" filters below causing this issue in Safari from floating_filters.txt

  1. youtube.com###header.ytd-c4-tabbed-header-renderer:style(position: absolute !important; left: 0 !important;)
  2. youtube.com###wrapper.app-header-layout > [slot="header"]:style(position: relative !important; left: 0 !important;)

In this case filter 1. should not be applied as filter 2. is more specific in this case since it is targeting the DIV id="wrapper" which is higher in the DOM tree than DIV id="header"

Screenshot from firefox dev console is below: youtube_conflict

ameshkov commented 3 years ago

Yeah, that's the very same issue.

yourduskquibbles commented 3 years ago

Just noticed @adguard-bot marked this as Status: Resolved but I don't see any corresponding commit messages tagged to this issue so wondering if I still need to be on the lookout for reports involving Mac Safari?

tvinzz commented 3 years ago

please, check the latest version v1.8.10, it's fixed there image

ameshkov commented 3 years ago

@tvinzz

The issue is not fully resolved though. I mean we implemented a workaround that works, but the proper solution needs to be implemented in Swift inside the Advanced Blocking extension, and not like it's done now.