Opened 16 months ago

Closed 15 months ago

Last modified 14 months ago

#4334 closed change (fixed)

Update ABP bundle to version 1.12.2

Reported by: mario Assignee:
Priority: P2 Milestone: Adblock-Browser-for-iOS-1.5.1
Module: Adblock-Browser-for-iOS Keywords:
Cc: pavelz Blocked By:
Blocking: Platform: Adblock Browser for iOS
Ready: no Confidential: no
Tester: Scheer Verified working: no
Review URL(s):

Description

Background

Since the last dependency update we have released two minor updates of ABP for Chrome/Opera/Safari:

In order to stay up to date, we want to update the ABP bundle in ABB.

What to change

Replace the ABP extension bundle with the provided crx file based on ABP 1.12.2 for Chrome/Opera/Safari.

Remarks

ABP 1.12.1 as well as 1.12.2 introduce changes/fixes which may need to be taken into consideration during implementation and QA.
Specifically the following issues might directly/indirectly affect ABB's behaviour:

  • #4141 - Video ads on YouTube not blocked
  • #1727 - WebSocket connections can't be blocked
  • #4150 - Request blocked in whitelisted subframe
  • #4191 - mail.ru shadowRoot stylesheet circumvention
  • #4298 - Unblockable ad box on Slickdeals.net
  • #4300 - Sitekey whitelisting not working for Safari
  • #4075 - Update adblockpluscore dependency to revision 08d73d05cfcd
  • #4076 - Update abp2blocklist dependency to revision 6576f594f0b9

Attachments (3)

ABP off.png (125.0 KB) - added by pavelz 15 months ago.
ABP on, AA off.png (81.7 KB) - added by pavelz 15 months ago.
ABP on, AA on.png (72.8 KB) - added by pavelz 15 months ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 15 months ago by pavelz

Went through the patches including the #4075 #4076 dependencies, except #4150 which i can't see.
Found one potentially hazardous change in #1727:
https://hg.adblockplus.org/adblockpluschrome/rev/20c6e8db2f5c#l2.22
It looks vaguely similar to the Chrome Ports feature, which Kitt does not implement. On the other hand, it's not a new declaration but an usage of something existing before, so either it was unused so far, or it's a harmless mock/polyfill.
https://hg.adblockplus.org/adblockpluschrome/file/20c6e8db2f5c/lib/messaging.js
suggests it might be the latter. But i would like to hear an authoritative confirmation.

comment:2 Changed 15 months ago by mario

  • Cc kzar added

Looping in kzar, as he's introduced said change.
@kzar, do you need additional background or can you tell right away whether this change is harmless in regard of a dependency update?

comment:3 Changed 15 months ago by kzar

  • Cc pavelz added; kzar removed

I can't tell you if these changes will be a problem for the iOS app or not, I have not worked with the app or the platform at all. You'll need to read through the changes to figure that out, and also test of course.

With issue #1727 we started wrapping the WebSocket function by injecting code into the page using a content script, if the WebSocket function doesn't exist we don't wrap it. In order to test you could update the dependency and try a website that uses WebSockets, for example twitch.tv or IRC Cloud. An example of a website that uses WebSockets to serve advertising is opensubtitles.org, you could try to see if those adverts are blocked.

Good luck.

comment:4 Changed 15 months ago by pavelz

Now went through #4150, it does not introduce any new Chrome API usage as suspected, but is apparently expecting certain behavior and return values of an existing API, so should be definitely retested on iOS target. Our Chrome emulation tends to follow common sense, not the particular glitches of real Chrome.

comment:5 Changed 15 months ago by pavelz

So unfortunately 1.12.2 is broken in ABB exactly due to #4150 (more specifically https://hg.adblockplus.org/adblockpluschrome/rev/9d40252192ec).
The indispensable framesOfTabs mapping is now constructed exclusively from webNavigation.onBeforeNavigate and webRequest.onBeforeRequest is just reading it (where 1.12.0 it was also adding frames to it). Now the issue is that Kitt core is firing onBeforeNavigate only for main frames, hence the call to ext.getFrame in onBeforeRequest returns nothing, hence cancel flag is never set.

A sensible place for onBeforeNavigate for subframe needs to be figured out.

comment:6 Changed 15 months ago by pavelz

QA will be the good old http://simple-adblock.com/faq/testing-your-adblocker/ because only the request blocking is broken. CSS hiding works normally, hence regular web pages appear correct - ads are loaded but hidden.

Changed 15 months ago by pavelz

Changed 15 months ago by pavelz

Changed 15 months ago by pavelz

comment:8 Changed 15 months ago by pavelz

The results are inconclusive. There is an empty rectangle at the described place (emptiness of which may be a different issue unrelated to this one) and it is not presented when AAs are off - that's correct i'd say. But the rectangle is also NOT presented when ABP is COMPLETELY off. What gives? See the attached screenshots.

UPDATE: it does the same on desktop Chrome with ABP 1.12.2.1649. The supposed AA is NOT presented when ABP is whitelisted on the site. Is that a correct behavior then?

comment:9 Changed 15 months ago by mario

@pavelz, I cannot reproduce this problem. Can you point me to an URL where this happens to you? Might be even limited to specific countries. AA on phonearena.com are displayed correctly to me. However, if it happens with ABP for Chrome as well it's an issue which can be handled outside of the scope of this dependency update.

comment:10 Changed 15 months ago by pavelz

  • Resolution set to fixed
  • Status changed from new to closed

@mario i don't know about any phonearena.com test case. I was using the website pointed out in #4150 which is http://at.combotag.com/index8.html. Which, uhhh, seems to be the same site? I don't understand the relation. Anyway, on phonearena.com i don't see any AAs at all. Applies to ABB/iOS as well as ABP/Chrome desktop.

The QA conditions for #4150 are apparently not simply verifiable, so i'm going to close this issue and expect a new specific opened if the next devbuild is found misbehaving

comment:11 Changed 14 months ago by mario

  • Milestone set to Adblock-Browser-for-iOS-1.5.1

Added to Milestone 1.5.1

comment:12 Changed 14 months ago by scheer

  • Tester changed from Unknown to Scheer

#4141 – Video ads on Youtube are blocked correctly.

#1727 – Ads are not displayed any longer on http://www.opensubtitles.org. In addition to this, chat is loaded correctly on Twitch.tv, the header and sidebar work correctly on http://www.pwnwin.com. http://www.websocket.org/echo.html also works correctly.

#4150 – This issue still occurs. Please refer to the ABB iOS ticket for this particular issue #4472.

#4191 – As element hiding is not available in ABB iOS, I was not able to effectively test the issue in this ticket. I have checked mail.ru, however, and all works as intended.

#4298 – The google ad-sense ad no longer shows on the mobile or desktop view of the page.

#4300 – As you are unable to manually add filters in ABB iOS, this was not possible to test.

#4075 – This was checked in line with Google search ads as they are hidden with element hide filters only. No issues were experienced.

#4076 – Whitelisting works as expected.

iPhone 6s Plus
iOS 9.3.2
iOS 8.4
iPad 2

Last edited 14 months ago by scheer (previous) (diff)
Note: See TracTickets for help on using tickets.