Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#3050 closed defect (fixed)

Whitelisted ad on google.com gets broken link with ABP for Safari iOS

Reported by: philll Assignee: kzar
Priority: P1 Milestone: Adblock-Plus-for-iOS-next
Module: Adblock-Plus-for-iOS/macOS Keywords:
Cc: kzar, matze, trev, till, sebastian Blocked By:
Blocking: Platform: iOS
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29327585/
https://codereview.adblockplus.org/29348923/

Description (last modified by kzar)

Environment

Iphone 6 Plus
iOS 9.0 (13A4325c)
Adblock Plus for Safari iOS 1.0
Acceptable ads enabled

How to reproduce

  1. Using Safari, open http://google.com
  2. Enter "hotels cologne" into the search field
  3. Press the search button
  4. Tap the booking.com ad on top of the search results

Observed behaviour

An empty, blank tab without any URL shown is opened.

Expected behaviour

The booking.com website should open.

Notes

The problem is being caused by this content blocking rule:

{
  "trigger": {
    "url-filter": "^https?://.*&adurl=",
    "resource-type": [
      "image",
      "style-sheet",
      "script",
      "font",
      "media",
      "raw",
      "document"
    ]
  },
  "action": {
    "type": "block"
  }
}

Attachments (1)

Screen Shot 2016-08-01 at 10.30.54.png (50.8 KB) - added by kzar 3 years ago.
New problem Scott noticed.

Download all attachments as: .zip

Change History (22)

comment:1 Changed 4 years ago by fhd

  • Owner set to fhd

comment:2 Changed 4 years ago by fhd

  • Review URL(s) modified (diff)

Added a temporary workaround for this, but not changing the status to codereview - we really have to fix this properly.

comment:3 Changed 4 years ago by fhd

  • Priority changed from P1 to P2

Workaround pushed: https://hg.adblockplus.org/adblockplussafariios/rev/b1a2a1e7b9c4

Reducing priority to P2 but otherwise keeping this open - now we can fix it properly.

comment:4 Changed 4 years ago by fhd

  • Milestone set to Adblock-Plus-for-Safari-for-iOS-1.0.0
  • Priority changed from P2 to P1
  • Resolution set to fixed
  • Status changed from new to closed

Actually, it makes more sense to close this one and create a follow-up. Did that: #3063.

comment:5 Changed 4 years ago by fhd

  • Resolution fixed deleted
  • Status changed from closed to reopened

That fix actually broke the whole list... Reverted it: https://hg.adblockplus.org/adblockplussafariios/rev/ec90a26b6f83

comment:6 Changed 4 years ago by philll

  • Platform changed from Unknown / Cross platform to iOS

Added newly available platform value.

comment:7 Changed 4 years ago by fhd

  • Milestone Adblock-Plus-for-iOS-1.0.0 deleted

comment:8 Changed 4 years ago by fhd

  • Owner fhd deleted

comment:9 Changed 3 years ago by kzar

  • Cc kzar matze trev till sebastian added
  • Review URL(s) modified (diff)
  • Status changed from reopened to reviewing

Realised I didn't need a device with iOS to test this, instead I made a simple Safari extension to test with the content blocker list that is used by the iOS app.

Was able to reproduce the problem as described and tried my fix, which then fixed the problem. Was able to then click text ads and have them work. (Also tested a couple of websites with adverts to make sure they're still getting blocked.)

Matze / Wladimir: Any idea how we can get this patch applied to https://easylist-downloads.adblockplus.org/easylist+exceptionrules_content_blocker.json quickly? (If it's more convenient the diff is also on GitHub.

Last edited 3 years ago by kzar (previous) (diff)

comment:10 Changed 3 years ago by kzar

  • Description modified (diff)

comment:11 Changed 3 years ago by kzar

  • Description modified (diff)

comment:12 Changed 3 years ago by kzar

  • Description modified (diff)
  • Owner set to kzar

(As Sebastian points out this quick-fix is not a long term solution. I've added some steps for fixing this more properly and will take a look at that next week.)

comment:13 Changed 3 years ago by abpbot

A commit referencing this issue has landed:
Issue 3050 - Ensure Google text ads work with AA

comment:14 Changed 3 years ago by Ross

I've now tried this on a 5S and adverts on search results pages are displayed/click-able/work as expected. Turning AA off/on works as expected. (Assuming the update at 31 July 22:00 was the fix?)

ABP 1.1.0
iPhone 5S / iOS 9.2

Last edited 3 years ago by Ross (previous) (diff)

comment:15 Changed 3 years ago by scheer

I am currently looking into the fixed list. The fix has now resolved the issue where selecting an Ad link would load you into a blank page. Now, selecting Ad links loads you correctly into the advertised page.

During this testing, I did notice two issues from about 40 Ad links selected. These were:

  1. Visit www.google.de and search 'Samsung Galaxy S7'
  2. Select the Ad with the URL 'www.base.de/galaxy-s7'

Observed Behaviour: That the error message - 'Safari cannot open the page
'The Error was. 'The URL can't be shown'. is displayed.

Expected Behaviour: That the Advertised page loads correctly.

(Directly inputting the link loads the page just fine).

  1. Visit www.google.co.uk and search 'buy car'
  2. Select the Ad with the URL 'www.nissan.co.uk/cars'

Observed Behaviour: That the error message - 'Safari cannot open the page
'The Error was. 'The URL can't be shown'. is displayed.

Expected Behaviour: That the Advertised page loads correctly.

(Directly inputting the link loads the page just fine).

comment:16 Changed 3 years ago by kzar

I can't reproduce that using my Safari for Mac OS X extension, sounds like it's more likely a problem with Safari for iOS. Whatever the cause this is a separate issue IMO, certainly not a regression caused by whitelisting the googleadservices.com domains.

comment:17 Changed 3 years ago by kzar

Ah, I mispoke :( I just managed to reproduce it.

"Safari can't open the page "https://www.google.co.uk/aclick?..." because the page's address isn't valid".

This isn't a regression with the recent change, but I'm now undecided if this is the same or separate issue.

Changed 3 years ago by kzar

New problem Scott noticed.

comment:18 Changed 3 years ago by kzar

This could even be a Safari bug, I'm not sure. I can reproduce consistently now:

  1. Browse to this URL: https://www.google.co.uk/aclk?sa=L&ai=DChcSEwjC6rqt5p_OAhVGuBsKHUoeAG8YABAA&sig=AOD64_2kAoVInCmwfi5kot9P5h-jb1GQ7Q&q=&ved=0ahUKEwjA-bet5p_OAhVlBcAKHVU4CDMQ0QwIIQ&adurl=

When my content blocking extension is enabled Safari considers the URL invalid. When content blocking extension is disabled Safari handles the URL properly and redirect to the advert page successfully.

I tested before and after the recent hotfix and the issue happened either way. I also tested with a nearly empty blocking list but then the problem went away.

Please could you open a new issue for this Scott? Good catch anyway!

comment:19 Changed 3 years ago by scheer

@kzar - issue #4280.

comment:20 Changed 3 years ago by kzar

  • Description modified (diff)
  • Resolution set to fixed
  • Status changed from reviewing to closed

I have just tested generating a fresh content blocking rule file using the latest easylist.txt, exceptionrules.txt and abp2blocklist tool and this issue no longer occurred. (However I did have to manually whitelist google.co.uk in order for the text ads to be displayed in the first place...)

I think no further action on this issue is necessary, but we will have to be extremely careful before updating the easylist+exceptionrules_content_blocker.json in the future. Content blocking rules are fundamentally different from Adblock Plus filters and do not translate perfectly. I think thorough QA is our only option going forward, at least for now.

(Also if anyone is interested I have identified the rule that caused the problem, I've added it to the issue description.)

comment:21 Changed 3 years ago by mario

  • Milestone set to Adblock-Plus-for-iOS-next

Batch-adjusted milestone for upcoming release of ABP for iOS.

Note: See TracTickets for help on using tickets.