Opened on 05/02/2014 at 09:39:38 PM

Closed on 05/14/2014 at 06:38:32 AM

#419 closed defect (fixed)

Safari: ABP 1.7.4.1170 blocks almost all the sites

Reported by: mapx Assignee:
Priority: P2 Milestone: Adblock-Plus-1.8.2-for-Chrome-Opera-Safari
Module: Core Keywords:
Cc: trev, sebastian Blocked By:
Blocking: #354 Platform:
Ready: yes Confidential: no
Tester: Verified working: yes
Review URL(s):

http://codereview.adblockplus.org/5825855188107264
http://codereview.adblockplus.org/6589722470121472

Description (last modified by trev)

Most pages are rendered blank by the current Safari build (1.7.4.1170), reportedly version 1.7.4.1169 works fine. It seems that creatives.livejasmin.com##body rule in EasyList that is only supposed to apply on creatives.livejasmin.com applies to other sites as well for some reason, as the result the entire page body is hidden. This appears to be a side-effect of updating the core code, #354 change made it into Safari then.

Attachments (1)

property_persistance.html (469 bytes) - added by trev on 05/13/2014 at 04:10:36 PM.
Safari bug testcase

Download all attachments as: .zip

Change History (15)

comment:1 Changed on 05/04/2014 at 04:22:13 AM by Vavatch

I too can confirm that adblock plus 1.7.4.1170 on Safari 7.0.3 on OS X 10.9.2 is blocking nearly all content on nearly all websites - only sites I can confirm getting through are Google results pages and Wikipedia.

When I uninstalled 1170 and installed 1169, the pages are displaying properly.

Note that I believe 1170 was working fine yesterday, so this MAY be due to the latest update of the EasyList perhaps? It shows as last updated at "Last updated at 21:14:22 PDT today"

comment:2 Changed on 05/04/2014 at 04:29:05 AM by unnamed

I have the same problem here, i'm on Mac OS X 10.9.2, Safari 7.0.3 and Ad Block Plus 1.7.4.1170.

Yesterday the safari was normal, but today 04/03/14 the bug start to working.

comment:3 Changed on 05/05/2014 at 09:38:05 AM by trev

  • Blocking 354 added
  • Cc trev added
  • Component changed from Unknown to Core
  • Description modified (diff)
  • Ready set

Updated the description to properly reflect the results of the investigation.

comment:4 Changed on 05/05/2014 at 09:58:12 AM by john.elgin

I have the same issues (including inability to load adblock sites! ;)) on Safari 6.1.2, running on OS/X 10.8.5. The home page renders - but essentially nothing else. What kind of bug checking was done??

comment:5 Changed on 05/05/2014 at 09:59:17 AM by fhd

  • Cc fhd added

comment:6 Changed on 05/05/2014 at 10:25:19 AM by trev

  • Cc fhd removed
  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:7 Changed on 05/05/2014 at 10:33:21 AM by trev

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

For reference: this looks like a bug in Safari's JavaScript engine, the getter we added to an object sometimes doesn't persist. There is no obvious relation to the changes in #354, it merely seems to have affected browser's memory allocation in an unfortunate way.

Fixed:
https://hg.adblockplus.org/adblockplus/rev/d76e8603bcb9
https://hg.adblockplus.org/adblockpluschrome/rev/dfc94942220c

comment:8 Changed on 05/05/2014 at 11:43:24 AM by philll

  • Verified working set

comment:9 Changed on 05/09/2014 at 06:17:03 AM by trev

  • Milestone set to Adblock-Plus-1.8-for-Chrome-Opera-Safari

comment:10 Changed on 05/13/2014 at 04:09:47 PM by trev

  • Cc sebastian added
  • Priority changed from P1 to P2
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening, turns out our fix merely masked the effect of this Safari bug. Rather than generating a bogus list of selectors we now throw an error the second time the domains getter runs. So this no longer has catastrophic results but it occasionally breaks element hiding.

Changed on 05/13/2014 at 04:10:36 PM by trev

Safari bug testcase

comment:11 Changed on 05/13/2014 at 04:14:29 PM by trev

I attached a reduced testcase - this bug is reproducible on a simple webpage. If the number of objects created is too large (this testcase creates 1000 of them but for me it is reproducible with 9 already) the exception "Attempting to change value of a readonly property" will be thrown - the property getter is executed a second time on the same object even though that getter should never run twice for each particular object (getter result is memoized). This seems to be the result of the JIT compiler generating broken code, the JIT compiler only kicks in if it sees a particular code path executed a number of times.

Two things need to happen here:

  • File a WebKit bug.
  • Implement a workaround - it seems that accessing filter.domains explicitly from ElemHide.getSelectorsForDomain() makes sure the getter runs and its result is persisted.

comment:12 Changed on 05/13/2014 at 05:32:31 PM by sebastian

comment:13 Changed on 05/14/2014 at 06:15:09 AM by sebastian

  • Review URL(s) modified (diff)
  • Status changed from reopened to reviewing

I can confirm that accessing filter.domains in ElemHide.getSelectorsForDomain() fixes it.

comment:14 Changed on 05/14/2014 at 06:38:32 AM by sebastian

  • Milestone changed from Adblock-Plus-1.8-for-Chrome-Opera-Safari to Adblock-Plus-for-Chrome-Opera-Safari-next
  • Resolution set to fixed
  • Status changed from reviewing to closed

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from (none).
 
Note: See TracTickets for help on using tickets.