Safari 5.0.3 crashes when reloading the page or removing a series

[matthewcrist]

I created a sample to show this behavior:

http://jsfiddle.net/QXZ7z/4/

It looks like when the destroy methods are called, the browser crashes when either reloading the page or removing a series.

[TorsteinHonsi]

For debugging: http://jsfiddle.net/highcharts/QXZ7z/7/

The crash happens in jQuery's animate when trying to animate the width and height of the clipRect for the remaining series.

[TorsteinHonsi]

It also happen when animation is disabled: http://jsfiddle.net/highcharts/GrHQy/

[TorsteinHonsi]

It happens while setting a width or height to the one remaining clipRect. Insert this code in attr to see it:

// issue #134 if ((prop == 'width' || prop == 'height') && elem.parentNode && elem.parentNode.nodeName == 'clipPath') { alert (prop) }

[matthewcrist]

When I goto the Fiddle, I get prompted for login to dev.highcharts.com

[matthewcrist]

In my testing it looked like the crash was coming from line 1618 of highcharts.src.js:

if (parentNode) { parentNode.removeChild(element); }

[TorsteinHonsi]

I've done more debugging and come up with the following minimal case that you can download and run in Safari: http://highcharts.com/issues/134.svg.

The problem seems to come down to removing a group element with a clip-path applied to it. Removing the group works well, but after removing the group, when trying to set an attribute on the clip-path's defining rectangle, Safari freezes.

Currently I haven't found a way around it. Tried setting the clip-path to 'none' before removing the group, but to no avail. Probably we will have to write a way around the Safari bug in Highcharts.

[trobrock]

I have a non-ideal fix for this safari issue if you want to check it out here: https://github.com/highslide-software/highcharts.com/pull/138

[TorsteinHonsi]

Thanks! It should work, though we can probably narrow it down further to only apply to the problematic clipping path.

But there's another problem. While I have reproduced and found the cause for the crash when removing a series, I still haven't seen the issue when reloading the page. It doesn't happen on the www.highcharts.com/demo page. Does it only happen when you have first removed a series? Can you show me a live page where this is demonstrated?

[matthewcrist]

The crash on reload only happens when series are added to the chart after render.

http://matthewcrist.github.com/highcharts/crash.html

If the series are defined in the config before render, then it seems to work just fine.

http://matthewcrist.github.com/highcharts/no_crash.html

Also, trobrock's suggested fix clears up this issue.

http://matthewcrist.github.com/highcharts/with_fix.html

I also started playing around with the Webkit nightlies and the root SVG issue seems to be fixed in a more recent build, so hopefully this is just a temporary thing.

Thanks for all your help with this!

[TorsteinHonsi]

I fixed part of this issue at https://github.com/highslide-software/highcharts.com/commit/520134b6f3aa2a8d6bdcc633efd3038803f74684. Now Safari shouldn't crash on unloading the page after adding a series. Please confirm.

The other case, where Safari 5 crashes on removing a series, still exists. The work will continue tomorrow.

[matthewcrist]

I'm getting a javascript error on that commit => http://matthewcrist.github.com/highcharts/fix_test.html

[TorsteinHonsi]

Actually I couldn't see this other than in your demo, but I added a check for clipRect that should be performed anyway. See the latest commit.

[matthewcrist]

Much better. I'm also no longer able to reproduce the crash when removing series from the chart.

[TorsteinHonsi]

Now that's interesting. Clicking the Remove button in http://jsfiddle.net/highcharts/GrHQy/ is still very deadly for my Safari. Could it be that Apple has fixed the bug? My version is 5.0.3 (7533.19.4), but I can't see that a newer version exists...

[TorsteinHonsi]

Worked around Safari 5.0 bug that caused the browser to crash when updating the properties of a clipping region after an SVG group (<g>) using that clip was removed. The workaround is to hide the group rather than remove it, remove all its children and remove references to it. As a consequence, there will be a little memory loss for each series that is removed from the chart, but it should be minimal. Closed by 8979cf7b956ef8742a72c5dc87abf8c37c22b27c.

[ngardner]

On iOS 4.3 or older, if the site is added to the Home Screen and ran, the userAgent string isn't populated (since technically its using UIWebView rather than Safari) so the workaround isn't applied and it still crashes. This is also true if you compile your site into an app using something like PhoneGap which also uses UIWebView.

iOS 5.0 doesn't have this bug so this workaround isn't needed at that point.

[TorsteinHonsi]

On my iPhone I added this page http://jsfiddle.net/highcharts/nHsXf/ to my home screen and ran it. It still reports the same userAgent string... Please advice.

[ngardner]

That shows me the same userAgent as well, however http://jsfiddle.net/highcharts/GrHQy/ defiantly crashes if I run it from the home screen using iOS 4.3

[TorsteinHonsi]

Very strange. I tested this with my iPad running 4.3.3 and my iPhone running 4.3.5. Both work well. Here's what I did:

1. Navigate to http://jsfiddle.net/highcharts/GrHQy/show
2. Click the button
3. A series is removed. So far so good.
4. Click the "share" icon and select "Add to home screen"
5. Click on the icon on the home screen
6. Click the button to remove the series
7. A series is removed and all works well

[ngardner]

When I add that URL to my home screen and launch it, it still opens in Safari for some reason - and doesn't crash for me either. However, if you take off the /show so its http://jsfiddle.net/highcharts/GrHQy/ and add that to your home screen, it launches it in fullscreen (non safari) mode and crashes when you remove the series.

[TorsteinHonsi]

Now I can see it. It looks like the bug is fixed with webkit 534, so we can check for 533. I tested this with Safari 5.1.1 on Windows (which doesn't have the bug), iPhone OS 4.3 (which has the bug) and iPhone OS 5 (which doesn't have the bug). Can you please verify that this fixes the issue:

Find this line:

issue134 = /\/5[0-9.]+ (Safari|Mobile)\//.test(userAgent), // todo: update when Safari bug is fixed

Replace it with:

issue134 = /AppleWebKit\/533/.test(userAgent),

[ngardner]

Sorry for the delay - I've made that change and it has fixed the issue on my iPhone running iOS 4.3.5. Thanks!