307558 - customized toolbars are reset to default because of corrupt localstore.rdf after search plugin update check

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Teune van Steeg]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4

Sometimes when I restart Firefox, after extension installation or software
update (for nightly 1.8 Branch), the costumization I did to the toolbars
(removing the go button, adding buttons from extensions) is not remembered, and
all is set back to what the toolbar looks like with a fresh profile. 

When I open the Javascript Console the following message is displayed several
times: 

Error: duplicate attribute
Source File: file:///<profile location>/localstore.rdf
Line: 244, Column: 20
Source Code:
                   NS1:LastPingDate="1126176449" />-------------------^

Where <profile location> is the path to my profile folder.
I have set the following prefs to true, but I don't know if the error message
appears when they are set to false:
javascript.options.showInConsole
javascript.options.strict

The entry in localstore.rdf which this error messages is talking about looks
like this:

  <RDF:Description RDF:about="engine://<program
folder>%5Csearchplugins%5Cgoogle.src"
                   NS1:LastPingDate="1126011710"
                   NS1:LastPingDate="1126176449" />

where <program folder> is the path to the Firefox program directory.

When I close Firefox and remove the first LastPingDate attribute and value, and
then restart Firefox the toolbars are back again to how I costumized them.

Reproducible: Sometimes

Steps to Reproduce:
1. Costumize the toolbars, drag toolbar buttons from the costumization window to
any of the toolbars.
2. Restart Firefox (I don't know if that's needed; I had costumized the toolbars
several weeks before this bug hit me)
3. Install an extension or do a software update
4. Restart Firefox

Actual Results:  
The toolbars are layed out just as when you install firefox for the first time
with a fresh profile, the icon in the search box shows a magnifying glass, and
no other icons/search engines are listed in the drop down menu.
The toolbar buttons I had put on the toolbar in a previous session are now all
present in the Costumize Toolbar dialog, along with the other icons I had left
there. The buttons/icons added by extensions are also present there.
Costumizing the toolbar again helps only during the session, after a restart all
is back to default iconset

Expected Results:  
Display the toolbars just as they were before Firefox was restarted.

I have experienced this bug three times now: Two times after installing an
extension, and just now after a software update. All three times, the same entry
in localstore.rdf (about the google search plugin) contained a duplicate
LastPingDate attribute. Removing one of those attributes solved this issue every
time.

So I guess a combination of trying to find an update for the google search
plugin and installing an extension/update or a combination of finding an update
for the google search plugin and restarting Firefox causes the addition of a
LastPingDate attribute, where it should be replaced, in localstore.rdf.

Comment 1

[Ria Klaassen (not reading all bugmail)]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20050908
Firefox/1.6a1 ID:2005090821

WFM in trunk and branch. With Automatically check for updates to Search Engines
enabled I installed some extensions, did several times a software update,
restarted every time; the toolbars did not reset to default. Additional steps to
reproduce?

Comment 2

[Teune van Steeg]

When I gave a closer look at the data in localstore.rdf I found that the
namespace NS1 is defined like this:
         xmlns:NS1="http://click2tab.mozdev.org/rdf/all/window/closedtabs/session#"

I have the click2tab (http://click2tab.mozdev.org) extension installed, which
isn't working, maybe because of this.

In Firefox 1.0.6 the namespace used for the LastPingDate is defined as
"http://home.netscape.com/WEB-rdf#" 

I don't know what's happened, but I find it strange that the LastPingDate
attribute is written to localstore with a wrong namespace.

I now manually removed all references to click2tab from localstore.rdf along
with the namespace defenitions concerning click2tab, and I changed the NS1
namespace definition from
"http://click2tab.mozdev.org/rdf/all/window/closedtabs/session#" to
"http://home.netscape.com/WEB-rdf#", and I'll see if the bug will appear again.

The click2tab extension at least works now as it's supposed to...

Comment 3

[IU]

For me, I have found that the following extensions trigger this bug (disabling
these extensions restores customization):

1. Web Developer 0.9.3
2. Header Monitor 0.3.2
3. IDN Info 0.6.4

Comment 4

[Rob Miller]

This also happens to me. Disabling the Web Developer extension brings back my
customised toolbars - even after they have been reset on restart - but enabling
it resets them on each restart after that.

It seems the customised settings are still being remembered, but are being
temporarily overwritten at each startup.

Comment 5

[Ria Klaassen (not reading all bugmail)]

(In reply to comment #3)
> For me, I have found that the following extensions trigger this bug (disabling
> these extensions restores customization):
> 
> 1. Web Developer 0.9.3
> 2. Header Monitor 0.3.2
> 3. IDN Info 0.6.4
>

This sounds more like Bug 306243. 

Comment 6

[Teune van Steeg]

Today the bug hit me again, with another profile. This time I didn't install an
extension or did a software update.

It seems like LastPingDate is associated with NS1 namespace name in
localstore.rdf no matter to what namespace that name is defined.

I think to reproduce this issue you'll have to define NS1 namespace manually in
localstore.rdf before it is defined by a search plugin update, e.g. in a fresh
profile:
 
<RDF:RDF xmlns:NS1="http://some.bogus.url/rdf#"
         xmlns:NC="http://home.netscape.com/NC-rdf#"
         xmlns:RDF="http://www.w3.org/1999/02/22-rdf-syntax-ns#">

Then you'll have to wait until a search plugin tries to update, because I don't
think you can manually do that. When that search plugin has updated a new entry
in localstore.rdf should be present with LastPingDate assigned to that NS1
namespace. 

The next time the searchplugin updates it apparantly can't overwrite the first
LastPingDate because of the wrong namespace and adds a new one. Which makes it
that Firefox won't read from localstore.rdf anymore.

This makes it a bug, because when an extension adds a self defined namespace to
localstore.rdf (like click2tab does) that namespace is assigned to a namespace
name ranging from NS1 to whatever is available (NS2, NS5, etc). While
LastPingDate seems to be stuck to NS1.

In my old 1.0.x profile, BTW, LastPingDate was assigned to NS4 namespace name,
which stands for the (correct) namespace "http://home.netscape.com/WEB-rdf#",
while namespaces from extensions were assigned to NS8, NS9 and NS10.

Comment 7

[Teune van Steeg]

Attachment: Add "http://home.netscape.com/WEB-rdf#" as a default namespace, to prevent LastPingDate to be assigned to a wrong namespace

this patch is only for the en-US locale, but it should probably also be applied to every other locale available for Firefox. (I can't create a patch for the other locales)

Comment 8

[Ryan Flint [:rflint] (ping via IRC for reviews)]

*** Bug 316921 has been marked as a duplicate of this bug. ***

Comment 9

[Ryan Flint [:rflint] (ping via IRC for reviews)]

*** Bug 316934 has been marked as a duplicate of this bug. ***

Comment 10

[Ryan Flint [:rflint] (ping via IRC for reviews)]

*** Bug 317075 has been marked as a duplicate of this bug. ***

Comment 11

[xiutian]

This bug hits me every single time I close and reopen Firefox

Comment 12

[:Gavin Sharp [email: [e-mail]]]

*** Bug 317044 has been marked as a duplicate of this bug. ***

Comment 13

[Anbo Motohiko]

(In reply to comment #7)
> Created an attachment (id=203123) [edit]
> Add "http://home.netscape.com/WEB-rdf#" as a default namespace, to prevent
> LastPingDate to be assigned to a wrong namespace
I think you also modify nsLocalStore.cpp.
http://lxr.mozilla.org/mozilla/source/rdf/datasource/src/nsLocalStore.cpp#428

Comment 14

[Teune van Steeg]

Attachment: updated patch with update to nsLocalStore.cpp, with respect to comment #13

Comment 15

[Mike Connor [:mconnor]]

Comment on attachment 204113 [details] [diff] [review]
updated patch with update to nsLocalStore.cpp, with respect to comment #13

I'm quite probably the wrong person to review RDF changes, bumping to bsmedberg for review (do we need the same fix for all locales now?)

Comment 16

[Benjamin Smedberg]

Comment on attachment 204113 [details] [diff] [review]
updated patch with update to nsLocalStore.cpp, with respect to comment #13

This is an interesting hack but is the wrong fix. We should instead fix the RDF serializer bug.

Comment 17

[Axel Hecht]

The serializer actually doesn't check if the prefix NS1 is already taken by a 
different prefix-namespace combo, so once it serializes with an unknown and
get yet another one, it breaks.

Comment 18

[:Gavin Sharp [email: [e-mail]]]

*** Bug 311528 has been marked as a duplicate of this bug. ***

Comment 19

[Axel Hecht]

Attachment: check the existing prefixes before adding a new one

I added code that checks if NS? is actually already existing. The code is 
factored so that it is used by both the new namespaces made up by the serializer 
as well as by AddNamespace without prefix. I did not guard AddNamespace against 
the possible corruptions by overwriting an existing namespace, as that's not a 
regression, AFAICT.

I intend to land this on the 1.8.0 branch, as new users get corrupted localstore
very easily.

Comment 20

[Benjamin Smedberg]

Comment on attachment 207630 [details] [diff] [review]
check the existing prefixes before adding a new one

>Index: nsRDFXMLSerializer.cpp

>+already_AddRefed<nsIAtom>
>+nsRDFXMLSerializer::EnsureNewPrefix()

>+    nsIAtom* outPrefix = nsnull;
>+    prefix.swap(outPrefix);
>+    return outPrefix;

It would be nice to have an already_AddRefed<T> nsCOMPtr<T>::forget() signature... want to file a bug on me?

>Index: nsRDFXMLSerializer.h

>+    already_AddRefed<nsIAtom>
>+    EnsureNewPrefix();

Oof, this style sucks. Oh well.

Comment 21

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 207630 [details] [diff] [review]
check the existing prefixes before adding a new one

>+        prefix = do_GetAtom(qname);

If we run out of memory, could this fail? This would mean that you'd need to check for NULL further down the call chain too.

Other than that, this looks good, so r=mrbkap.

Comment 22

[Axel Hecht]

The out-of-memory problems are not new to this patch, I'd like to solve that
in a separate patch, if we decide to stabilize RDF further.
This patch is kept small for easier approval.

Comment 23

[Axel Hecht]

checked in on trunk, let the baking begin.

Comment 24

[Ria Klaassen (not reading all bugmail)]

*** Bug 322679 has been marked as a duplicate of this bug. ***

Comment 25

[Axel Hecht]

Comment on attachment 207630 [details] [diff] [review]
check the existing prefixes before adding a new one

I'd like to land this on the branches.

Risk is low, and new users are affected by corrupted localstore.rdfs.

Comment 26

[Daniel Veditz [:dveditz]]

Comment on attachment 207630 [details] [diff] [review]
check the existing prefixes before adding a new one

awesome, thanks! But drivers would like more trunk testing before accepting and it's too late in the 1.8.0.1 cycle. moving request to 1.8.0.2

Comment 27

[Ria Klaassen (not reading all bugmail)]

*** Bug 321871 has been marked as a duplicate of this bug. ***

Comment 28

[:Gavin Sharp [email: [e-mail]]]

*** Bug 319196 has been marked as a duplicate of this bug. ***

Comment 29

[Steve England [:stevee]]

*** Bug 309137 has been marked as a duplicate of this bug. ***

Comment 30

[Pardal Freudenthal (:ShareBird)]

Just my two cents on this issue. 
If we don't get it fixed for 1.8.0.1, please considere changing the default preference for search engines update.

Comment 31

[Daniel Veditz [:dveditz]]

Comment on attachment 207630 [details] [diff] [review]
check the existing prefixes before adding a new one

approved for 1.8.0 branch, a=dveditz for drivers

Comment 32

[Axel Hecht [:Pike]]

fix landed on the branches

Comment 33

[Jason Barnabe (np)]

So will this problem be fixed for upgrading from 1.5.0.1 to 1.5.0.2, or will it only take effect on the upgrade from 1.5.0.2 to the next one?

Comment 34

[Dave Liebreich [:davel]]

Marking [tcn-dl] - could someone please add an explicit testcase that reliably demonstrates this problem, plus guidelines for testing the fix?  Thanks.

Comment 35

[Teune van Steeg]

(In reply to comment #34)
> Marking [tcn-dl] - could someone please add an explicit testcase that reliably
> demonstrates this problem, plus guidelines for testing the fix?  Thanks.
> 

Read comment #6, or install click2tab (http://click2tab.mozdev.org) extension.

Use a build from the following folder which will show you the bug: ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/1.5-candidates

With a build from the following folder the bug is fixed (i.e. a new namespace name for http://home.netscape.com/WEB-rdf# is created when NS1 is taken): ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla1.8

Create a new profile for each build.

When firefox runs, close it immediately and add the bogus namespace url to localstore.rdf and open firefox again.
 
or 

When firefox runs install click2tab extension and restart, then open and close a few tabs and restart.

Because there is no way to manually update the search engines you'll just have to browse for a while before a search engine updates. (I was lucky: the google search engine looked for an update within the hour). You'll have to look at the localstore.rdf file occasionally to see if an entry with LastPingDate is added.

To see the effects of the bug in the browser, you'll have to wait for the search engine to check for an update for a second time. But this could take days and maybe weeks.

Comment 36

[Dave Townsend [:mossop]]

*** Bug 326068 has been marked as a duplicate of this bug. ***

Comment 37

[Dave Townsend [:mossop]]

*** Bug 325634 has been marked as a duplicate of this bug. ***

Comment 38

[Dave Townsend [:mossop]]

*** Bug 340506 has been marked as a duplicate of this bug. ***

Comment 39

[Majken Connor [:Kensie]]

I am unduping a few of these and duping them to bug 319196

The case where bookmarks were also missing is still happening in upates to 1.5.0.4 while I've seen *no one* come in with just localstore.rdf missing anymore. There must be another issue.

Comment 40

[Majken Connor [:Kensie]]

*** Bug 330303 has been marked as a duplicate of this bug. ***

Comment 41

[Majken Connor [:Kensie]]

*** Bug 342305 has been marked as a duplicate of this bug. ***

Comment 42

[:Gavin Sharp [email: [e-mail]]]

*** Bug 326669 has been marked as a duplicate of this bug. ***

Comment 43

[Anders Conbere]

*** Bug 345714 has been marked as a duplicate of this bug. ***

Comment 44

[Max]

I downloaded the theme "glowyred", and this bug happened to me, even though I have switched the theme several different times.I do not know how to fix it yet.

Comment 45

[Max]

I don't know if this has to do with the bug, but I think it does: Mozilla changed the theme back to the default theme (though my theme and extension settings have been saved, it changed my homepage to the original one, it deleted my bookmarks, it shows error messages that I turned off (You are going on to an encrypted page, etc.), and sometimes it has an error message on (the one "unknown problem generating errrors, must create error logs"). Has anyone had this yet?

Comment 46

[Majken Connor [:Kensie]]

Max - you are experiencing a different issue, please seek support from #firefox in irc.mozilla.org or from forums.mozillazine.org

Comment 47

[Anders Conbere]

*** Bug 360597 has been marked as a duplicate of this bug. ***

Comment 48

[Anders Conbere]

*** Bug 360732 has been marked as a duplicate of this bug. ***