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

[lewisje]

Environment

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

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.

Notes

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: ​https://adblockplus.org/forum/viewtopic.php?t=48231

Specifically, the problem happens in these places:

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

Curiously, MDN itself says that the method is deprecated (so does the spec, as linked from the forum thread): ​https://developer.mozilla.org/en-US/docs/Web/API/Event/initEvent

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): ​https://developer.mozilla.org/en-US/docs/Web/Guide/Events/Creating_and_triggering_events

Where supported, the Event constructor takes an options object as its second argument, and both the object and all properties within are optional: ​https://developer.mozilla.org/en-US/docs/Web/API/Event/Event

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.

[sebastian]

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.

[kzar]

We have a test VM running with Safari 10.0 (12602.1.50.0.5) 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.

[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?

[philll]

You can download that release version directly and install it: ​https://developer.apple.com/safari/download/
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.

[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.

[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.

[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?

[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.

[kzar]

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!)

[abpbot]

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

[abpbot]

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

[rraceanu]

No errors encountered during installation and browsing.

ABP 1.12.3
Safari 10
OS 10.11
Easylist

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