Closed Bug 393614 Opened 14 years ago Closed 14 years ago

SeaMonkey does not create prefs.js file if that is missing

Categories

(Core :: Preferences: Backend, defect, P4)

x86
OS/2
defect

Tracking

()

RESOLVED FIXED
mozilla1.9beta2

People

(Reporter: mozilla, Assigned: mozilla)

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.9a8pre) Gecko/2007081904 SeaMonkey/2.0a1pre (self compiled optimized build)
Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.9a8pre) Gecko/2007082401 SeaMonkey/2.0a1pre (self compiled debug build)

When removing the prefs.js file or when starting SeaMonkey without a profile (and without one to migrate), prefs.js does not get created. Changes in preferences are only remembered as long as the session lasts, as no prefs.js gets written they are reset to the defaults on application restart.

In the debug build I see the following output that might be related:
[...]
*** Registering components in: nsWidgetOS2Module
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed: file X:/trunk/mozilla/modules/libpref/src/nsPrefService.cpp, line 369
[...]
WARNING: Key 'key_preferences' of menu item 'Preferences...' could not be found: file X:/trunk/mozilla/layout/xul/base/src/nsMenuFrame.cpp, line 1025
[...]
JavaScript strict warning: chrome://communicator/content/pref/nsPrefWindow.js, line 101: reference to undefined property Components.interfaces.nsIChromeRegistrySea
*** Failed to create prefs object
[...]

After using "touch" to create an empty prefs.js file or after moving back the real prefs.js everything works are expected.
Dave noted in mozilla.dev.ports.os2 that prefs.js isn't migrated by the migration tool, either. I suspect that both problems are related.
Summary: SeaMonkey does not create prefs.js file is that is missing → SeaMonkey does not create prefs.js file if that is missing
OK, after quite a bit of debugging, I think I found where it goes wrong. The call to myDosOpenL() in _PR_MD_OPEN() in os2io.c

http://mxr.mozilla.org/seamonkey/source/nsprpub/pr/src/md/os2/os2io.c#249

fails with rc=110 which is the rather generic ERROR_OPEN_FAILED. Actually, I don't see anything wrong with the call, the parameters are
   myDosOpenL(<full path>\prefs.js,
              <valid pointer>,
              <valid pointer>,
              0,
              FILE_NORMAL,
              0x1 == OPEN_ACTION_CREATE_IF_NEW,
              0x40 == OPEN_SHARE_DENYNONE,
              0)
All variables are valid, including the full path specified, none of the parameters are in high memory when the call fails. Very confusing!
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached file call stack
Just for completeness, this is the call stack to the NSPR call where it goes wrong...
Args, the OS/2 Toolkit docs are faulty in one more place. 0x0001 is OPEN_ACTION_FAIL_IF_NEW (OPEN_ACTION_CREATE_IF_NEW would be 0x0010!). This OPEN_ACTION_FAIL_IF_NEW gets set in _PR_MD_OPEN() in os2io.c because the flags just contain the default (PR_RDONLY) as set in nsFileInputStream::Open(). This can be remedied by adding PR_CREATE_FILE | PR_RDONLY to the NS_NewLocalFileInputStream() call in openPrefFile().

In the end this regression occurred because of the checkin of bug 361102 (from end of November 2006!) because that really checked the return code of openPrefFile().
Keywords: regression
Attached patch fix (obsolete) — Splinter Review
Benjamin, you were the last reviewer of changes in this file, so I hope you can review this, too. :-)
Assignee: prefs → mozilla
Status: NEW → ASSIGNED
Attachment #284875 - Flags: review?(benjamin)
Actually a core bug, happens on Firefox trunk (OS/2), too.
Component: Preferences → Preferences: Backend
Product: Mozilla Application Suite → Core
Comment on attachment 284875 [details] [diff] [review]
fix

I don't think this is correct. We don't want to create the file when we're *reading* it, only when we're *writing* it. Perhaps you just want to eat the error code in this case and return NS_OK?
Attachment #284875 - Flags: review?(benjamin) → review-
Perhaps calling exists() before openPrefFile() as in bug 395618 could work. Have to test that.
Attached patch fix v2Splinter Review
OK, this works, too.

I guess the shared file is always there from the start, otherwise I would have to do the same change in ReadAndOwnSharedUserPrefFile().
Attachment #284875 - Attachment is obsolete: true
Attachment #285410 - Flags: review?(benjamin)
Attachment #285410 - Flags: superreview?(cbiesinger)
Attachment #285410 - Flags: review?(benjamin)
Attachment #285410 - Flags: review+
Seems that I have to ask for blocking1.9 to get this on the map for the sr again. It's certainly critical for OS/2.
Severity: normal → critical
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9+
Priority: -- → P4
Whiteboard: [has patch][has review bsmedberg][needs sr biesi]
Attachment #285410 - Flags: superreview?(cbiesinger) → superreview+
Whiteboard: [has patch][has review bsmedberg][needs sr biesi] → [has patch][has review bsmedberg][has sr biesi][needs landing]
Reed, would you (or somebody else) be so kind to land this? I won't have time to actually watch tinderbox before the freeze.
Keywords: checkin-needed
Checking in modules/libpref/src/nsPrefService.cpp;
/cvsroot/mozilla/modules/libpref/src/nsPrefService.cpp,v  <--  nsPrefService.cpp
new revision: 1.98; previous revision: 1.97
done
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [has patch][has review bsmedberg][has sr biesi][needs landing]
Target Milestone: --- → mozilla1.9 M10
You need to log in before you can comment on or make changes to this bug.