Opened on 08/25/2016 at 01:47:35 PM

Closed on 09/19/2016 at 07:19:41 AM

Last modified on 09/22/2016 at 05:46:05 AM

#4361 closed defect (fixed)

ABP calls Event#initEvent without all required parameters, breaking Safari TP r11

Reported by: lewisje Assignee: kzar
Priority: P2 Milestone: Adblock-Plus-1.12.3-for-Safari
Module: Platform Keywords:
Cc: kzar, sebastian, mapx, philll Blocked By:
Blocking: Platform: Safari
Ready: yes Confidential: no
Tester: Unknown Verified working: yes
Review URL(s):

Description (last modified by kzar)


Safari 10 Technology Preview Release 11
Adblock Plus 1.12.2 and current development build

How to reproduce

Install Adblock Plus for Safari

Observed behaviour

The extension fails to run. There is an error about incorrect usage of initEvent() in the JS console.

Expected behaviour

Adblock Plus should run without any errors.


Now that the boilerplate is out of the way, user cdumez on the forums said that the problem is that Safari Technical Preview r11 makes the second and third parameters to Event#initEvent required, in alignment with the latest DOM standard (presumably, they were not required previously and defaulted to false), and that in three places in the code, ABP calls this method with just the event type:

Specifically, the problem happens in these places:

  • ./ext/content.js: beforeLoadEvent.initEvent("beforeload");
  • ./ext/content.js: evt.initEvent(eventName);
  • ./ beforeLoadEvent.initEvent("beforeload");

Curiously, MDN itself says that the method is deprecated (so does the spec, as linked from the forum thread):

It recommends instead using the Event constructor and the dispatchEvent method, and only using initEvent in Internet Explorer, which does not support the use of the Event constructor for triggering events (maybe this fact, and the desire to reuse as much code as possible, is why you haven't already moved to window.dispatchEvent(new Event("beforeload")) or something similar):

Where supported, the Event constructor takes an options object as its second argument, and both the object and all properties within are optional:

Hints for testers

  • Ensure Adblock Plus now works properly, without throwing the above exceptions, on Safari 10. (NOT the Technical Preview release, since Safari 10 is now out properly.)
  • Check that the regression #4141 which broke the blocking of YouTube adverts when using the flash player didn't return.

Attachments (1)

Screenshot_2016-09-13_20-50-26.png (316.6 KB) - added by kzar on 09/13/2016 at 06:55:58 PM.

Download all attachments as: .zip

Change History (20)

comment:1 Changed on 08/25/2016 at 01:50:54 PM by mapx

  • Cc kzar sebastian mapx added

comment:2 follow-up: Changed on 08/26/2016 at 11:53:44 AM by sebastian

  • Component changed from Unknown to Platform
  • Description modified (diff)

Unfortunately, we currently don't have any test machines running Safari 10. (Those have been ordered, but it will take a while.) So can somebody who has the Safari 10 pre-release, please confirm that both/either of the following works there:

var evt = document.createEvent("Event"); 
evt.initEvent('beforeload', false, true);
new Event("beforeload");

Note that the Event constructor isn't supported on Safari <=9 at least.

comment:3 Changed on 09/01/2016 at 04:18:27 PM by kzar

  • Cc philll added

We have a test VM running with Safari 10.0 (12602. now. I've just tried and I can't reproduce the problem at all, I don't see exceptions being thrown from our content scripts with Adblock Plus 1.12.1 and I have no trouble calling initEvent with only the first parameter.

Any ideas if our VM is running a version of Safari 10.0 that's newer or older than "Preview Release 11" Phill? If we're running a newer version perhaps initEvent will no longer be a problem, but if not I guess we need to have WebMate upgrade Safari.

comment:4 Changed on 09/01/2016 at 04:25:39 PM by kzar

Looking at the release notes it seems that this change was made in Release 11:

"Made the parameters mandatory for Event.initEvent()"

So I guess we have an earlier version. Please could you ask WebMate to upgrade us to release >=12 Phill?

comment:5 Changed on 09/02/2016 at 07:01:32 AM by philll

You can download that release version directly and install it:
I think it's a good idea to be able to keep the slightly older version for regression testing for now, so we can manually go back and forth.

comment:6 Changed on 09/13/2016 at 08:17:14 AM by sebastian

I don't see the point of keeping our test machines on an outdated pre-release. Most users will be on the latest version, which eventually turns into stable. So it's important that we make sure that everything is working there, and not on any previous pre-release version that nobody is using anymore.

And it seems that Adblock Plus is currently completely broken on the latest Safari 10 pre-release, which will soon be rolled out to all users. However, we can hardly do anything if we don't have an environment to reproduce the issue.

Installing the macOS pre-release yourself, is anything but trivial, unless you have a physical Mac that you are willing to sacrifice. Running macOS in a VM, in particular on a non-macOS host, generally doesn't work great.

comment:7 Changed on 09/13/2016 at 08:48:45 AM by philll

I didn't suggest to use any local machine but simply to upgrade the Safari version on our test infrastructure by one minor pre-release version manually. Obviously the plan is not to keep an all too old version forever. What I suggested, is simply keeping the version older by one minor for now so one can properly regress where issues come from. If you feel very strong about it, I can tell our provider to update to the latest minor version but going back for regression testing is almost impossible then.

comment:8 Changed on 09/13/2016 at 08:49:02 AM by philll

Last edited on 09/13/2016 at 08:50:45 AM by philll

Changed on 09/13/2016 at 06:55:58 PM by kzar

comment:9 Changed on 09/13/2016 at 07:00:19 PM by kzar

I'm not able to update to the latest pre-release on our Webmate machine, I'm getting this error: "Safari Technology Preview can't be installed on this disk. This volume does not meet the requirements for this update." (screenshot attached). Would you mind just getting them to update it for us Phill?

comment:10 Changed on 09/15/2016 at 05:35:39 PM by kzar

  • Owner set to kzar
  • Priority changed from Unknown to P2
  • Ready set

comment:11 in reply to: ↑ 2 Changed on 09/15/2016 at 05:47:42 PM by kzar

Replying to sebastian:

...can somebody who has the Safari 10 pre-release, please confirm that both/either of the following works there:

var evt = document.createEvent("Event"); 
evt.initEvent('beforeload', false, true);
new Event("beforeload");

Yep, can confirm both work with Safari Technology Preview release 12. (Also that evt.initEvent('beforeload'); no longer works.)

I guess since it's supported with earlier versions of Safari I'll stick with initEvent, even though it's now considered deprecated.

comment:12 Changed on 09/15/2016 at 06:01:42 PM by kzar

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

OK did some very quick testing and the extension seems to work fine on Safari Technology Preview release 12 once we're passing all the initEvent parameters.

(I didn't manage to reproduce this issue with WebMate yet, but realised Safari Technical Preview can also be installed on El Capitan and so I could simply install it on my test device! Whoops!)

comment:13 Changed on 09/19/2016 at 07:13:00 AM by abpbot

A commit referencing this issue has landed:
Issue 4361 - Pass all parameters to initEvent

comment:14 Changed on 09/19/2016 at 07:19:41 AM by kzar

  • Description modified (diff)
  • Milestone set to Adblock-Plus-for-Chrome-Opera-Safari-next
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:15 Changed on 09/21/2016 at 09:33:53 AM by sebastian

  • Milestone changed from Adblock-Plus-for-Chrome-Opera-Safari-next to Adblock-Plus-1.12.3-for-Safari

comment:16 Changed on 09/21/2016 at 09:38:46 AM by abpbot

A commit referencing this issue has landed:
Issue 4361 - Pass all parameters to initEvent

comment:17 Changed on 09/21/2016 at 11:56:28 AM by kzar

  • Description modified (diff)

comment:18 Changed on 09/22/2016 at 05:45:34 AM by rraceanu

No errors encountered during installation and browsing.

ABP 1.12.3
Safari 10
OS 10.11

No issues encountered whilst viewing Youtube with Flash player and having ABP installed (ticket 4141).

comment:19 Changed on 09/22/2016 at 05:46:05 AM by rraceanu

  • Verified working set

Add Comment

Modify Ticket

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