Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.9b2pre) Gecko/2007111218 SeaMonkey/2.0a1pre This is a regression from bug 402460. Saving images like http://www.seamonkey-project.org/images/template/header-logo.png fails (Context menu -> Save Image As...) fails because downloads.rdf cannot be created. nsLocalFile::CreateUnique which calls nsLocalFileOS2::Create expects the specific error state NS_ERROR_FILE_ALREADY_EXISTS to go on but translation through NS_ErrorAccordingToNSPR() does not provide this any more.
Yes, because we set the flag OPEN_ACTION_FAIL_IF_EXISTS for DosOpen() in _PR_MD_OPEN in os2io.c we get ERROR_OPEN_FAILED in return instead of ERROR_FILE_EXISTS. Now, the flags used to call PR_Open in OS/2's nsLocalFile::Create are weird anyway: PR_RDONLY | PR_CREATE_FILE | PR_APPEND | PR_EXCL. Not sure how PR_RDONLY and PR_APPEND go together. So one could argue that - either _PR_MD_OPEN should react to PR_APPEND by setting OPEN_ACTION_OPEN_IF_EXISTS and so patch NSPR (with unknown results without _lots_ of testing) - or we just back out the OS/2 part of bug 402460 again. I think this would be less risky because it has worked well for a long time. mkaply, I would like to hear your advice...
Created attachment 288934 [details] [diff] [review] version 1: backout
I made the change to attempt to keep parity with Windows because of bug 394713. All other OSes correctly return NS_ERROR_FILE_ALREADY_EXISTS.
Neil, yes, I know you were trying to do us on OS/2 a favor, but next time you do that, could you CC me? That could save me hours of debugging (or at least I could do the debugging session before the patch messes up branch ;-) ).
Given what the NSPR API docs at http://developer.mozilla.org/en/docs/PR_Open say * PR_EXCL. Used with PR_CREATE_FILE; if the file does not exist, the file is created. If the file already exists, no action and NULL is returned I think the previous behavior was correct. NULL was returned with PR_CREATE | PR_EXCL and so it could be assumed that the file already existed. Given the OS/2 API I don't think we can do anything else for this combination of PR_ flags than we do already, and from that we will probably never get anything else back than PR_IO_ERROR, even when file permissions/right access is a problem. After reading the docs I convinced myself even more that PR_APPEND doesn't make sense here, so we shouldn't use it in _PR_MD_OPEN as I originally suggested.
So is the right fix to backout the change or can we fix NSPR?
Comment on attachment 288934 [details] [diff] [review] version 1: backout No, I see no other way than backing it out.
Actually, this problem also seems to mess up profile handling in other ways (doesn't write stuff to bookmarks or prefs) and hence can cause dataloss, so this is really a blocker. Either we still get it into CVS for the 188.8.131.52 release or apply the fix by hand for that Gecko release on OS/2.
Comment on attachment 288934 [details] [diff] [review] version 1: backout r=mkaply
OK, thanks. Attachment 288934 [details] [diff] checked in to trunk.
Comment on attachment 288934 [details] [diff] [review] version 1: backout I'm not going to check in on the _RELBRANCH without asking this time... But this is a blocker for OS/2.
Sorry to put you to all this trouble. All the other platforms seem be able to produce reasonable return codes and I had no idea that OS/2 couldn't. Are OS/2 users likely to be unable to write to the installation directory? We may need to relnote that SeaMonkey 1.1.x will never support this under OS/2.
There are some solutions to implement access restrictions on OS/2 but in the core it's a single user system. I never saw anybody complain about this as a problem for Mozilla apps.
Comment on attachment 288934 [details] [diff] [review] version 1: backout approved for 184.108.40.206, a=dveditz It's OK to land this OS/2-only backout on the RELBRANCH (and also on the 1.8 branch, of course).
Great, thanks. Checked in attachment 288934 [details] [diff] [review] on both MOZILLA_1_8_BRANCH and GECKO181_20071115_RELBRANCH.
Hmm, I did not reset the release tags like SEAMONKEY_1_1_7_RELEASE or FIREFOX_2_0_0_10_RELEASE. I had somehow assumed that the release team would do that. Should I still do that? Any other tags would I have to set?
OK, I used bonsai's graph feature to find out that only those two tags were set (I don't care about the RC1 tag) and reset them from revision 220.127.116.11 to the new revision 18.104.22.168.2.1 for this one file. In the meantime Mike Luther verified that his branch problems are gone with the SeaMonkey 1.1.7 candidate version that I built (and that now becomes the release) and the same is true for the 1.1.8pre nightly. I have since last week verified that trunk now works again as it should.