Bug 3876 - (int-93205) some modifications in about:config reset to default when browser restarted
(int-93205)
: some modifications in about:config reset to default when browser restarted
Status: RESOLVED FIXED
Product: Browser
MicroB engine
: 4.1.2 (4.2008.36-5)
: N800 Maemo
: Low normal (vote)
: 5.0 (1.2009.41-10)
Assigned To: unassigned
: microb-bugs
:
: community-diablo, patch
:
:
  Show dependency tree
 
Reported: 2008-11-20 14:29 UTC by Zhihai Wang
Modified: 2009-10-22 07:57 UTC (History)
3 users (show)

See Also:


Attachments
patch for this issue (669 bytes, patch)
2008-12-08 04:08 UTC, Zhihai Wang
Details


Note

You need to log in before you can comment on or make changes to this bug.


Description Zhihai Wang (reporter) 2008-11-20 14:29:49 UTC
STEPS TO REPRODUCE THE PROBLEM:
1. type about:config in browser's address bar
2. scroll down and we could see that advanced.mailftp is set to false
3. set this entry to true by input advanced.mailftp into the Name and true into
Value on the top of this page and then press the button Set Preference
4. scroll down again and we could see that this value has been changed to true
5. close browser
6. open browser again and type "about:config", we could see that
"advanced.mailftp" is reset to false
7. generally speaking, modifications to existing entries or newly added
key-value pairs won't retain after browser is closed


REPRODUCIBILITY: always
(always/sometimes/once)
Comment 1 timeless 2008-11-20 15:15:52 UTC
i just tried this w/ advanced.mailftp and it persisted.

there are quite a few prefs which are destroyed at startup (not technically
shutdown), however the one you picked isn't among them:

http://mxr.maemo.org/garage/search?string=advanced.mailftp&find=microb

generally, the ones destroyed are visible in this list:
http://mxr.maemo.org/garage/search?string=common_set_pref&find=microb.*eal

specifically this block:
http://mxr.maemo.org/garage/source/browser/mozilla/trunk/microb-eal/src/gmozillaweb.c?mark=1688-1689,1698-1871#1686

there are a number of bugs in this block, and in most cases there's a bug in
this system about them.
Comment 2 Andre Klapper maemo.org 2008-11-20 20:41:33 UTC
...plus for example changing general.useragent.vendor (see bug 3868).
Ran into this myself yesterday. :-/
Comment 3 timeless 2008-11-20 20:57:02 UTC
no.

1751         gtk_moz_embed_common_set_pref(G_TYPE_STRING,
G_MOZILLA_PREF_EAL_NAME, MOZILLA_DEFAULT_VENDOR_NAME);

vendor is one of the ones that are explicitly replaced

the one you should be setting is:
general.useragent.override

instead of using vendor+friends.
Comment 4 Zhihai Wang (reporter) 2008-11-21 04:33:35 UTC
Hi, timeless:
I just get confused. 

It seems that all the entries in "about:config" are listed in
/usr/lib/microb-engine/greprefs/all.js, and they are identical. 

According to your comment, values in this file will be destoryed(be reset to my
understanding), so all the entries will restore from all.js when browser
restarted. If this is correct, this matches what I saw and can explain why
modifications don't retain.

But you said your testing persisted, and I found that "advanced.mailftp" does
exist in all.js, this conflicts you mentioned "however the one you picked isn't
among them".

Anything I have done wrong?
Comment 5 timeless 2008-11-21 15:01:34 UTC
i'm not sure which link gave you the impression that i said that all.js entries
would be destroyed.

all.js has default values.
if you try to set a value to something that doesn't match all.js,
    it should be saved.
if you try to set a custom value and it matches all.js,
    the fact that it's a custom value will be lost (this is only interesting if
the default value later changes).

however. there is code in microb...eal (gmozillaweb.c) which is fairly stupid
(understatement) and it will just stomp over certain preferences that it thinks
need to be set.
    if your pref isn't listed in that file, it should persist....
Comment 6 Zhihai Wang (reporter) 2008-11-21 17:50:36 UTC
Sorry I misunderstood you. 

What I thought(and what I see in my N800 so far) is that all entries in
"about:config" are restored with the default values in all.js if they are
modified, and newly added entries will simply be removed.

But you said that these values should be saved, unless be explicitly set by
microb...eal.

I just don't know where the problem is. My system is clean, I didn't install
any package after I updated the firmware.
Comment 7 Andre Klapper maemo.org 2008-11-21 17:55:22 UTC
(In reply to comment #6)
> and newly added entries will simply be removed.

Does not happen here. Added "general.useragent.override" yesterday and it
remained after restarting.
Comment 8 Zhihai Wang (reporter) 2008-11-21 18:37:24 UTC
(In reply to comment #7)
As to newly added entries, I just entered some arbitrary key-value pairs, say
with Name equals to "a" and Value equals to "b". Could this be a reason why
it's removed?

What happended to your device if you change the value of "advanced.mailftp"
from false to true and then restart browser? Will this value be saved or just
restored to false?
Comment 9 Andre Klapper maemo.org 2008-11-21 18:40:57 UTC
It will be restored to false.
Comment 10 Zhihai Wang (reporter) 2008-11-21 18:43:34 UTC
Then it should be a bug.

And maybe invalid newly added entries would be removed?
Comment 11 timeless 2008-11-22 18:13:11 UTC
sorry, there's no such thing as an invalid entry. if you create 'a' = 'b', it
should persist.

can you ls -al `find ~ -name prefs.js` from a terminal?

it's possible some random .deb or equiv has changed ownership of prefs, if that
happens, it's not our fault (the bug's invalid, unless you want to use it in a
crusade against whichever package broke your prefs, however, i'd recommend a
new bug)
Comment 12 Zhihai Wang (reporter) 2008-11-24 03:51:34 UTC
-rw------- 1 user users 1942 Nov 23 13:43 /home/user/.mozilla/microb/prefs.js

It shouldn't be the case you mentioned someone else changed the ownership,
since I do the test as soon as I updated the firmware, not any other packages
installed:(

I would be glad to provide more information if you want me to do.
Comment 13 Zhihai Wang (reporter) 2008-12-05 14:57:36 UTC
I've found the reason. It is because the preference is not saved.
Adding the following code to gmozillaengine.c:load_finished_cb could fix this
bug, but I'm not sure if this is a clean and elegant way or not.

     //ULOG_DEBUG_F("last_net_status: %i", self->last_net_status);
     self->is_loading = FALSE;

+    // save preference here
+    gchar * location = gtk_moz_embed_get_location(embed);
+    const gchar *prefix = "about:config?prefname=";
+    if (location && g_ascii_strncasecmp(location, prefix, strlen(prefix))==0)
{
+        gtk_moz_embed_common_save_prefs();
+    }
+
     GWebEngineEncoding encoding = g_mozilla_engine_get_encoding (embed);
Comment 14 timeless 2008-12-05 15:07:36 UTC
that's terribly broken. something somewhere in the code needs to handle saving
prefs when they change. preferably vaguely lazily and probably if dirty at
shutdown.
Comment 15 Zhihai Wang (reporter) 2008-12-05 15:14:37 UTC
(In reply to comment #14)
> that's terribly broken. something somewhere in the code needs to handle saving
> prefs when they change. preferably vaguely lazily and probably if dirty at
> shutdown.
> 

So it's a ugly fix:(
I haven't studied the source code very carefully yet and I don't know how to do
just like you recommended ...

Waiting for better solution
Comment 16 Zhihai Wang (reporter) 2008-12-08 04:08:55 UTC
Created an attachment (id=1066) [details]
patch for this issue

I couldn't have provided this patch without timeless's help, thanks!
Comment 17 Andre Klapper maemo.org 2009-04-23 14:40:00 UTC
Unfortunately this is a WONTFIX for Diablo (not enough priority).
Now wondering whether this will be still valid for Fremantle...
Comment 18 Andre Klapper maemo.org 2009-06-04 18:26:57 UTC
Changing "general.useragent.vendor" seems to work for me on Fremantle.
Even after a browser restart my own value is there.
Same with changing "advanced.mailftp".

Any other testcase or can I close this as WORKSFORME/FIXED in Fremantle?
Comment 19 timeless 2009-06-04 19:05:50 UTC
zhihai: sorry i didn't get back to you, but indeed you found the right place.

the patch can be shortened to this:
   ps[pmap[type]](name, fmap[type](value));
+  ps.savePrefFile(null);

Fremantle uses a different about:config than the one that shipped in Diablo.
the general bits in microb which stomp on preferences are still present. but
indeed the problem that zhihai's patch addresses is not a problem in Fremantle.

I have no idea how/why that line got lost when I wrote the html version :(.
Comment 20 Lucas Maneos 2009-10-22 07:57:37 UTC
Marking patches of interest to Diablo (Maemo4) community updates, please excuse
the noise.