Bug 64926 – REGRESSION (r90991-r91108): Some extensions broken (LocalStorage corrupts or is not read)

• Status: RESOLVED
• Resolution: INVALID
• Component: New Bugs

[Kevin M. Dean]

The Ninjakit extension no longer runs greasemonkey scripts and doesn't open it's panel when clicked.

User CSS extension opens it's panel but it's missing all previously created settings and doesn't take new settings.

Better Facebook doesn't appear to see it's settings and thinks it's a fresh install.

Could be others, but clearly something is broken with the extensions in Safari 5.1 Lion running Webkit 91108+.

Reverting to 90991 or straight Safari 5.1 resolves the issues.

We need a 10.7 option in the OS select box on the bug reports.

[Alexey Proskuryakov]

<rdar://problem/9816583>

[Kevin M. Dean]

Still broken with latest nightly.

I've been around here for a while but I don't remember asking, how (if possible) is one supposed to view a rdar link, or is that limited to a certian level of developer?

[Kevin M. Dean]

Anyone looking into the change that's broken extensions?

Here's some links to acquire the extensions in question.

User CSS
http://code.grid.in.th/

Ninjakit
http://mac.softpedia.com/progDownload/NinjaKit-for-Safari-Download-79296.html

Better Facebook
http://betterfacebook.net/

[Kevin M. Dean]

Extensions still broken for me with r93849.

Last usable release was from July 14th.

It's worst with the Better Facebook extension. Whatever's wrong wipes the preferences completely and even if you revert to the previously working Safari you have to reset all the options again.

[Kevin M. Dean]

Seems that the problem could lie with an extenstions interfacing with LocalStorage. Specifically with Better Facebook's corruption of it's LocalStorage preferences.

in:

~/Library/Safari/LocalStorage/http_www.facebook.com_0.localstorage

I only see a slight change to the header of the file between the working version and the one Better Facebook considers corrupted.

Since User CSS and Ninjakit also use LocalStorage I wouldn't be surprised if there's some similar problem there as well.

[Jessie Berlin]

(In reply to comment #5)
> Seems that the problem could lie with an extenstions interfacing with LocalStorage. Specifically with Better Facebook's corruption of it's LocalStorage preferences.
> 
> in:
> 
> ~/Library/Safari/LocalStorage/http_www.facebook.com_0.localstorage
> 
> I only see a slight change to the header of the file between the working version and the one Better Facebook considers corrupted.
> 
> Since User CSS and Ninjakit also use LocalStorage I wouldn't be surprised if there's some similar problem there as well.

Are you using Private Browsing either when using these Extensions or installing them?

[Jessie Berlin]

(In reply to comment #5)
> Seems that the problem could lie with an extenstions interfacing with LocalStorage. Specifically with Better Facebook's corruption of it's LocalStorage preferences.
> 
> in:
> 
> ~/Library/Safari/LocalStorage/http_www.facebook.com_0.localstorage
> 
> I only see a slight change to the header of the file between the working version and the one Better Facebook considers corrupted.
> 
> Since User CSS and Ninjakit also use LocalStorage I wouldn't be surprised if there's some similar problem there as well.

Also, if Better Facebook is using local storage within an injected script, then they are editing the local storage belonging to Facebook.com. It is likely that Facebook.com is also using local storage and could be messing with those values stored by Better Facebook.

The other two Extensions might not be the same issue.

[Kevin M. Dean]

Not using private browsing.

The extensions work fine with Safari and r90991 and earlier, but haven't worked correctly since, so I doubt it's a Facebook specific issue.

You can see with the filename I previously provided that it his writing to a facebook.com local storage file. Even if I reset options with that extension, when I come back later with a current nightly, it doesn't read in the previous settings that were created. It acts as if it was newly installed again.

[Jessie Berlin]

If he is writing to Facebook.com’s Local Storage (which it looks like he is), then it is very possible that Facebook.com is breaking him, not Safari. He should not be using Facebook.com’s Local Storage at all. Instead, he should be sending the preferences up to the Application Layer and storing it there.

I think the issue with the other two Extensions is not with that specific WebKit revision range, but with the move from WebKit1 to WebKit2 which happened with 5.1.

In WebKit2, we use a multi-process model, but Local Storage does not work when accessed from multiple processes (see https://bugs.webkit.org/show_bug.cgi?id=67817). What results is a race condition between the UI Process and the Web Process in terms of writing local storage to disk. Those other two Extensions write to Local Storage both in the Application Layer (UI Process) and Content Layer (Web Process).

This race condition is present in 5.1 as well as in the nighties, which explains why me attempting to reproduce with the exact steps you described is failing. You are probably falling victim to it.

The developers for those Extensions *should not* be accessing local storage in both the Application Layer and the Content Layer. They should be accessing it only the in Application Layer or only in the Content Layer and messaging the data back and for as described in the Safari Extensions Development Guide.

[Kevin M. Dean]

(In reply to comment #9)
> If he is writing to Facebook.com’s Local Storage (which it looks like he is), then it is very possible that Facebook.com is breaking him, not Safari. He should not be using Facebook.com’s Local Storage at all. Instead, he should be sending the preferences up to the Application Layer and storing it there.
> 
> I think the issue with the other two Extensions is not with that specific WebKit revision range, but with the move from WebKit1 to WebKit2 which happened with 5.1.

I can't really speak to where extension writers should be writing their content, but you mention it being a 5.1 problem, yet the extension problems I have do not occur with 5.1 nor with r90991 and earlier running in 5.1 under OS X Lion. I use r90991 every day so I know. All 3 extensions do break within that range when I test. Things don't seem to be adding up the way you describe. I'm not seeing the issue you say is due to a race condition on anything but the latest webkit nightlies after the range I specified.

I'm not sure this should be considered invalid yet.

[Kevin M. Dean]

The first time I run A Better Facebook on my facebook page under r91108 it displays errors that state:

"Your stored preferences could not be read because they have become corrupt. Sorry, but your options been reset! This shouldn't happen again! The error was:
SyntaxError: Unable to parse JSON string
"

Perhaps JSON is where the issue lies in regard to accessing the stored content.

[Jessie Berlin]

(In reply to comment #11)
> The first time I run A Better Facebook on my facebook page under r91108 it displays errors that state:
> 
> "Your stored preferences could not be read because they have become corrupt. Sorry, but your options been reset! This shouldn't happen again! The error was:
> SyntaxError: Unable to parse JSON string
> "
> 
> Perhaps JSON is where the issue lies in regard to accessing the stored content.

Both me and another person I asked to test cannot reproduce these issues when installing and configuring these Extensions in 5.1 or 90991 and then running 91108 (and subsequent nighties).

Perhaps some JS on your machine is being loaded/run slightly faster or slower in 91108 and therefore accessing the Local Storage in the Application Layer / Content Layer at a different time (due to the race condition).

I am sorry you are still having these issues. I don’t know what to tell you other than to watch the status of the local storage bug (https://bugs.webkit.org/show_bug.cgi?id=67817) and consider contacting the authors of these Extensions.

[Alexey Proskuryakov]

I also cannot reproduce any regression between r90991 and r91108. I tried User CSS, and it worked just fine for me in r94831 nightly.

Being a race condition, the issue can happen at unpredictable moments.

[Kevin M. Dean]

(In reply to comment #13)
> I also cannot reproduce any regression between r90991 and r91108. I tried User CSS, and it worked just fine for me in r94831 nightly.
> 

Here's what I'm seeing regarding User CSS. If I go into r91108 or r94831 using the localstorage that I've used on all prior versions, I'll get the issue where opening the User CSS tab will no longer have my saved settings and if I create a new one, it appears to be saved, but closing / re-opening the User CSS tab results in empty settings.

Now if I manually remove "safari-extension_com.gridth.usercss-v892bvzc73_0.localstorage" and run r90991, make some settings and then open it in r91108 or later, it works correctly as you suggest.

It seems that it's having problems reading an older version of the SQLite file. I'm not sure how to interpret the difference but if I examine each file in Hexedit I'm seeing a difference in the header where the stored content starts at a lower line than the old file.

Old file: 00000540

New file: 00000D40

Screenshots: http://development.rhubarbproductions.com/webkit/extensions/

Has the storage format changed or the read/write to SQLite changed at some point?

[Kevin M. Dean]

OK, If I do the same things to Better Facebook that I just did to UserCSS, I appear to get a working Better Facebook in recent builds. When comparing the 2 localstorage files in Hex Edit I'm seeing a similar but not exactly the same difference in the start row where the newer file is further down.

That's appearing to be the crux of it. If you have an older localstorage, it's causing problems, but if you install new, it so far appears to work correctly. I guess I can manually make note of my settings and recreate them, but it still sounds like a potential upgrade issue for people unless I somehow have a rare special case which I doubt.

[Kevin M. Dean]

Someone else just downloaded a Nightly and had a similar problem with his Adblock extension.

http://www.finetunedmac.com/forums/ubbthreads.php?ubb=showflat&Number=17827

I still think this may end up being a problem for people in a future Safari release if it keeps on causing their extension settings to go bad.

[Kevin M. Dean]

Now, my first non-extension issue.

I noticed that Twitter pages weren't rendering and the console was showing a JSON token error. I removed Twitter's LocalStorage and twitter pages started working again.