Skip to content

Loading…

[Safari] µBlock crashes Safari < 8 #593

Closed
asoksevil opened this Issue · 36 comments

7 participants

@asoksevil

The latest version is only compatible with Safari 8+ users but there are a lot of users who are using Safari 5,6 and 7. For those users, an alternative methods should be offered (like older versions).

AFAIK:

Safari 5 works great on 0.8.2: https://googledrive.com/host/0Bx8fnUCX4W2ISjlGTC1HOE0zNFE/uBlock.safariextz
Safari 6 and 7 should be working with this build: https://www.dropbox.com/s/j4tjro1i2b7jq7v/µBlock.safariextz?dl=0

@gorhill

Older Safari has bugs, no extension will fix these, only Safari developers can fix these. Workaround to prevent a crash from manifesting at launch time doesn't imply there won't be crash later on. A proper request is to ask Safari devs to fix their browser, not asking an extension to workaround obscure browser bugs.

@chrisaljoudi

@asoksevil like @gorhill said, it's not that we "dropped" support in newer versions of µBlock as a policy.

Newer versions of µBlock for Safari have had significant improvements — they're more efficient, more effective, and overall just more refined. At the same time, they aren't in any way using "new" features that aren't backward compatible.

We're not trying to obsolete or deprecate older versions of Safari — they just happen to have a bug [or more] that's been fixed in Safari 8. The bug didn't manifest in some older versions of µBlock for Safari.

The point at which µBlock "started" "crashing" older Safari's is arbitrary. Hey — maybe soon, there will be another version of µBlock that just happens not to crash Safari 7! I don't know — we don't know; it wasn't a policy decision.

It's currently unknown what the bug really is, and there's nothing the extension can do to fix the inherent problem.

At this point, all one can do is get Apple to push the fixes to older Safari's just like they did with 8+. If you're able to reproduce the crash/bug, it'd be great (and appreciated) if you can file an issue on bugreport.apple.com.

As a side-note: I spent quite a bit of time trying to get an older version of Safari to run on my Yosemite machine. However, as you may know, there's no real way of running older versions without downgrading OS X.

I would've been happy to modify some of the Safari-specific code if it's a well-defined, simple, and otherwise inconsequential change.


TL;DR: providing 'special' versions of µBlock to toe-tip around bugs in older versions of Safari is like CNN/FOX (or some other TV broadcaster) providing duct tape for broken CRTs.

@asoksevil

@chrisaljoudi

Thank you for your reply.
There's one way where you can test older Safari versions without downgrading your system, that iti's installing a Virtual Machine with Mavericks, Snow Leopard or whatever distribution you want to test. Parallels, VMWare, and Virtual Box are great choices.

From my side, I'll report these bugs to Apple, hoping they can fix this (at least for Safari 7 for Mavericks)

@chrisaljoudi

@asoksevil has the update of Safari that was pushed today by Apple (I believe including to Mavericks and Mountain Lion) resolved this issue?

It'd be great (and much appreciated) if you could test out the latest µBlock with the latest Safari available on your OS X.

@asoksevil

@chrisaljoudi No, it didn't. It keeps crashing on Safari 7.1.3 using 0.8.6

@gorhill

@chrisaljoudi Could you check is using results of String.fromCharCode(0, 0) as an object property name works ok in Safari (write/read)?

@chrisaljoudi

@gorhill seems to work okay (tested over browserstack.com).

@asoksevil

It's fixed for Safari 7 and Mavericks?

@chrisaljoudi

@asoksevil nope, doesn't seem like it yet. I was referencing String.fromCharCode(0, 0) above — µBlock appears to still crash Safari 7.

I'm actively investigating this to see what the crash trigger is (and, if reasonable, avoid it).

At this point, this issue is really about compatibility with the older versions, so I'll change the title.

@chrisaljoudi chrisaljoudi changed the title from [Safari] Provide download links for latest supported versions for older versions of Safari to [Safari] µBlock crashes Safari < 8
@robinadr

Given the requirements listed on the homepage state Safari 8.0+, with a further statement of no plans to support older versions, should this just be closed as wontfix?

@chrisaljoudi

@robinadr maybe. I don't want to alienate a significant percentage of Safari users if it's a simple workaround.

If it's not, this will be closed indeed.

@iHeini

I'm drunken and I'll have to correct my last message. will post again when I'm sober. thx

@chrisaljoudi
Owner

No constructive work to be done until (and if) Apple patches older Safari's.

@asoksevil

Bug's been reported to Apple, I'll update this post once I heard something from them (unlikely to happen).

@meltres
@gitguys

Yeah, my first introduction to ublock was a crash of Safari and not being able to run Safari afterwards. I was able to find a apple discussion group using chrome to remove the extension. I think you guys should pull ublock or put up a HUGE WARNING for Mavericks users because it's caused quite a few issues for me including screwing up other stuff on my Mac including my Finder and another app I use that works with Safari's bookmarks that got hosed during the crash.

No other extension has ever done this to safari including AdBlock and Adblock Plus. And no extension has ever done this to any other browser I've ever used. This is a very SEVERE issue. I just can't see myself trusting ublock even on Yosimite at this point after getting burned this badly by it.

You need to warn people about this issue and pull the ublock for Safari 7.x or people like me are going to continue to run screaming from ublock and tell everyone else to avoid it as well.

@chrisaljoudi
Owner

@gitguys I realize it's quite frustrating, but please know the following:

I understand how much of a pain it is, and I'm sorry, but I hope you don't feel compelled to angrily blame the µBlock team for this.


On a related note, the best way to completely uninstall µBlock if you install it is by removing the safariextz file from ~/Library/Safari/Extensions/.

@gitguys

chrisaljoudi, this is where I got it from:

https://extensions.apple.com/details/?id=net.gorhill.uBlock-96G4BAKDQ9

There's no warning there about it being only compatible with 8.x or up, much less warning of a severe issue with 7.x

Someone needs to pull it off of there ASAP.

This crap just wrecked my day. Also, in the section in your download area here on Github, I would put a MUCH LARGER warning there as well for anyone who still wants to try it (or doesn't know which version of Safari they have) despite it saying it's 8.x or higher.

@chrisaljoudi
Owner

@gitguys

I understand.

Pull it down now.

No, I'm afraid I can't do that. To elaborate, please understand the following:

  • Apple doesn't give me direct access to edit the extension details (not even the description). I have to email them every time and it takes a week to hear back and then a week to have it live on the Gallery.

  • Even if I did have direct access, I wouldn't take down the extension. I would just edit the description to strongly point out not installing on Safari versions prior to 8. I've asked for the edit already; Apple's Extension Gallery team simply hasn't done it/gotten back to me yet.

@gitguys

I'm going to contact the Apple's Extension Gallery team as well. This is really, really bad.

@chrisaljoudi
Owner

@gitguys although it's appreciated you pointed this out, please know it's already very much known and noted — and being worked on as well as I know how.

@chrisaljoudi
Owner

@gitguys I want to re-emphasize this is a Safari bug. Under no circumstances should JavaScript be able to crash your browser. Browsers are designed not to do that, and so is JavaScript/the DOM.

That is not to say µBlock is doing anything wrong. The trigger for the crash is perfectly valid, perfectly fine JavaScript.

The workaround soon to be pushed is just that — a workaround. I can tell you right now one line of code that you can insert into any Safari extension and would cause this crash.

Apple, and only Apple, can fix the real problem.

@gitguys

Thank you for working on it. Apple's Extension Gallery team needs to not drag its feet for another second and either put up a warning or pull it (like right NOW). I mean, when I saw it there without any warning or version caveat, I thought it was safe to install it in Safari (as would most people).

You can imagine my shock when Safari immediately crashed. And then my relative horror when it wouldn't run again. This is bad, bad, bad for your project.

As far as it being Apple's fault, I'll agree with you there, but I'm still not sure why you'd release this when it kills Safari. Seems like a severe lack of proper testing? Or did it work with earlier versions of 7.x and Apple made a change that killed it? Either way, as I said, as long as this extension is up there on that extensions page without any warning, etc. for Maverick's users -- this is going to be destructive for uBlock.

@gorhill

@chrisaljoudi Wouldn't it be possible to create a stub for XMLHttpRequest in the platform-safari code which would just shortcut to an error if the browser is < Safari 8 and the URL has no extension in the path part?

@chrisaljoudi
Owner

@gorhill funny; already got started on that just five minutes ago. Will let you know ASAP.

@chrisaljoudi
Owner

@gorhill hadn't thought of that earlier, though, which is a drag.

@chrisaljoudi
Owner

@gorhill @gitguys 1e4f725 stops crashing on older Safari's; shouldn't affect any other browser.

@gorhill are we good for a release?

@gorhill

You tried it, it works?

@chrisaljoudi
Owner

@gorhill yes. Tested on Safari 7.0.6 on OS X Mavericks (which crashed consistently until now).

@gorhill

There has been a lot of code change since 0.8.9.1 and I find it scary to make an official release out of these. It would be safer to rewind to 0.8.9.1, branch, then integrate your patch, and make an official 0.8.9.2 release just for Safari?

@chrisaljoudi
Owner

@gorhill hmm. Would it be alright with you if we put all the other changes since 0.8.9.1 forward to a release numbered as 0.8.9.3, and 0.8.9.2 can be just the crash fix (with just the one patch applied on top of 0.8.9.1)?

@gorhill

Yes, that's what I was thinking. For the dev build the version number is meaningless so we can set it to whatever is convenient.

@gorhill

I will let you fill in the details.

@gorhill

@chrisaljoudi I think you should summarily explain the Safari bug in the release, looks like many still do not understand that if there is shame to be had, certainly Safari deserves its share: crashing because no . in the URL, how are outside developers suppose to figure that one... Actually, how did you figure it, that's quite a finding.

@chrisaljoudi chrisaljoudi removed the wontfix label
@asoksevil

Thanks for the update Chris, appreciate it!

Working flawlessly on Mavericks (10.9.5)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.