Opened 5 years ago

Closed 5 years ago

#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 5 years ago.
Safari bug testcase

Download all attachments as: .zip

Change History (15)

comment:1 Changed 5 years ago 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 5 years ago 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 5 years ago 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 5 years ago 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 5 years ago by fhd

  • Cc fhd added

comment:6 Changed 5 years ago by trev

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

comment:7 Changed 5 years ago 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 5 years ago by philll

  • Verified working set

comment:9 Changed 5 years ago by trev

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

comment:10 Changed 5 years ago 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 5 years ago by trev

Safari bug testcase

comment:11 Changed 5 years ago 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 5 years ago by sebastian

comment:13 Changed 5 years ago 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 5 years ago 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
Note: See TracTickets for help on using tickets.