Last Comment Bug 376173 - ProfileManager comes up as XUL error dialog
: ProfileManager comes up as XUL error dialog
Status: RESOLVED FIXED
relnote-seamonkey1.1.3
: fixed-seamonkey1.1.3, regression, relnote
Product: SeaMonkey
Classification: Client Software
Component: General (show other bugs)
: Trunk
: x86 Linux
: -- major (vote)
: ---
Assigned To: Andrew Schultz
:
Mentors:
Depends on:
Blocks: 375102 377529 389136
  Show dependency treegraph
 
Reported: 2007-04-01 12:17 PDT by Andrew Schultz
Modified: 2009-12-20 08:00 PST (History)
13 users (show)
kairo: blocking‑seamonkey1.1.3+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
chrome.rdf generated by regchrome (unhappy) (36.18 KB, text/plain)
2007-04-01 12:23 PDT, Andrew Schultz
no flags Details
chrome.rdf generated by seamonkey-bin (happy) (36.59 KB, text/plain)
2007-04-01 12:26 PDT, Andrew Schultz
no flags Details
explicitly ignore permissions failure (1.12 KB, patch)
2007-04-18 22:53 PDT, Andrew Schultz
neil: review-
Details | Diff | Review
ignore more failures (3.18 KB, patch)
2007-07-02 21:47 PDT, Andrew Schultz
neil: review+
neil: superreview+
kairo: approval‑seamonkey1.1.3+
Details | Diff | Review

Description Andrew Schultz 2007-04-01 12:17:13 PDT
Starting the SeaMonkey profile manager as a user that doesn't own brings up a XUL error dialog: 

XML Parsing Error: undefined entity
Location: chrome://communicator/content/profile/profileSelection.xul
Line Number 53, Column 1:

If I run SeaMonkey once as the user, it works after that, even if I change the directory/file ownership back to root.  Rolling back nsRDFXMLDataSource.cpp to v 1.154 fixes things (v 1.156 didn't help -- it's not the actual permissions).

chrome.rdf seems to be the culprit.  I'll attach working and not-working versions.
Comment 1 Andrew Schultz 2007-04-01 12:23:32 PDT
Created attachment 260285 [details]
chrome.rdf generated by regchrome (unhappy)
Comment 2 Andrew Schultz 2007-04-01 12:26:57 PDT
Created attachment 260286 [details]
chrome.rdf generated by seamonkey-bin (happy)

The difference here is that there are "selectedLocale" lines like:

<c:selectedLocale RDF:resource="urn:mozilla:locale:en-US:communicator"/>

Old builds (before bug 375102) did not have such lines either, but worked OK.
Comment 3 Robert Kaiser (not working on stability any more) 2007-04-01 13:07:29 PDT
The selectedLocale stuff should usually be only in the chrome.rdf of the profile...
Comment 4 Andrew Schultz 2007-04-01 20:52:42 PDT
OK, so the basic problem here is that the API changed a bit.   rdfXMLFlush (which is just an internal method) now returns failure if it can't write (chrome.rdf in the app directory is what it's trying to write to).  This failure gets propogated by RDFXMLDataSourceImpl::Flush (a public API) and things down the line think something is wrong.  The previous v 1.154 implementation ignored failure to open the file.

In SeaMonkey, we seem to be calling into ::Flush from nsChromeRegistry::SetProviderForPackage, and we might update the method to be more selective of what it thinks is an real failure.
Comment 5 neil@parkwaycc.co.uk 2007-04-02 13:21:27 PDT
We actually call ::Flush from six places, one of which is only called for global chrome, one for profile chrome, and the other four may be either.
Comment 6 Andrew Schultz 2007-04-03 16:57:50 PDT
we need to figure this out ASAP so bug 375102 can land on branch without breaking us.
Comment 7 (not reading, please use seth@sspitzer.org instead) 2007-04-11 08:09:11 PDT
andrew / neil:  any update on the seamonkey side?
Comment 8 Andrew Schultz 2007-04-18 22:53:08 PDT
Created attachment 262086 [details] [diff] [review]
explicitly ignore permissions failure

biesi suggested ignoring this specific failure and that does seem to work.  I don't know if it will help with bug 377529, which I suspect is related.

I also noticed that ::rdfXMLFlush actually claims to succeed if !mIsWritable.  It seems reasonable for unwritable files to have !mIsWritable, but the code thinks any file is writable.

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/rdf/base/src/nsRDFXMLDataSource.cpp&rev=1.157&mark=601-606#588

fixing that might be a better fix and/or a way to help with bug 377529.
Comment 9 neil@parkwaycc.co.uk 2007-04-19 05:36:30 PDT
Comment on attachment 262086 [details] [diff] [review]
explicitly ignore permissions failure

This didn't seem to work for me. You probably need to at least look at WriteInfoToDataSource, which is also called during startup. I also think that you should always check for errors when writing to the profile.
Comment 10 Daniel Veditz [:dveditz] 2007-04-26 17:16:50 PDT
Any progress here? People could be very sad when we land bug 375102 and then you can't ship 1.0.9 and 1.1.2
Comment 11 Robert Kaiser (not working on stability any more) 2007-05-01 03:13:25 PDT
bug 375102 has been pushed out to 1.8.0.13/1.8.1.5, so we can let this slip the 1.0.9/1.1.2 releases, but we still need to look into fixing that problem.

The branch patch for the original bug is virtually identical to the trunk patch, so it will probably cause the same problems with old chromereg, and we need to find a way to keep it working on branch (trunk will be fixed by switching to suiterunner eventually, but branch won't without a working patch).
Comment 12 (not reading, please use seth@sspitzer.org instead) 2007-06-22 12:05:16 PDT
andrew / neil / robert:  heads up, bug #375102 is plussed for 1.8.1.5 and 1.8.0.13.

I'd like to land it ASAP, so that we can test those builds as long as possible before we ship.

But there is no patch for this bug, so that will break you.

What can we do here?
Comment 13 Robert Kaiser (not working on stability any more) 2007-06-22 12:22:34 PDT
Someone needs to come up with a patch. I don't understand the code, but I hope someone understands it can come up with a patch, else there will be no 1.8-based releases of SeaMonkey any more.
Comment 14 (not reading, please use seth@sspitzer.org instead) 2007-06-26 16:01:19 PDT
What about using "#ifdef MOZ_SUITE" in nsRDFXMLDataSource.cpp until there is a fix?
Comment 15 Robert Kaiser (not working on stability any more) 2007-06-27 05:15:49 PDT
That surely would make us not block other apps, which is nice, but I fear this will mean that this bug will stay unfixed for the rest of the 1.8.x lifetime.

In any case, please use "#ifndef MOZ_XUL_APP" there if we go that way, as it's really a problem of xpfe vs. toolkit and not suite vs. the rest of the world.
Comment 16 neil@parkwaycc.co.uk 2007-07-02 08:20:50 PDT
Comment on attachment 262086 [details] [diff] [review]
explicitly ignore permissions failure

>     rv = remote->Flush();
>+  // Explicitly ignore permissions failure since we sometimes try to write to
>+  // global chrome when we shouldn't.
>+  if (rv == NS_ERROR_FILE_ACCESS_DENIED)
>+    rv = NS_OK;
I'll give r=me on a patch that does it on all calls to remote->Flush() (which you would of course factor out into a separate (inline?) method).
Comment 17 neil@parkwaycc.co.uk 2007-07-02 09:06:18 PDT
(In reply to comment #16)
>I'll give r=me on a patch that does it on all calls to remote->Flush()
(and installRemote->Flush() too of course)
Comment 18 Andrew Schultz 2007-07-02 21:47:44 PDT
Created attachment 270690 [details] [diff] [review]
ignore more failures
Comment 19 neil@parkwaycc.co.uk 2007-07-04 04:04:38 PDT
Comment on attachment 270690 [details] [diff] [review]
ignore more failures

r+sr=me by code inspection.
Comment 20 (not reading, please use seth@sspitzer.org instead) 2007-07-04 13:41:48 PDT
fyi, on 7/4/07, the patch for bug #375102 landed on the MOZILLA_1_8_BRANCH (in order to be a part of firefox / tbird 2.0.0.5)
Comment 21 Andrew Schultz 2007-07-04 17:32:30 PDT
Comment on attachment 270690 [details] [diff] [review]
ignore more failures

this code is dead on trunk, so going straight for branch approval
Comment 22 Robert Kaiser (not working on stability any more) 2007-07-04 18:01:47 PDT
Comment on attachment 270690 [details] [diff] [review]
ignore more failures

As long as it fixes profile manager on branch, this looks acceptable ;-)
a=me for 1.1.3
Comment 23 Robert Kaiser (not working on stability any more) 2007-07-19 13:42:00 PDT
Since bug 375102 and this landed (I could track it down to having happened on 2007-07-04), SeaMonkey can not be launched directly from the Mac disk image any more, showing XML errors in chrome. This should be relnoted.
Comment 24 neil@parkwaycc.co.uk 2007-07-20 16:46:58 PDT
(In reply to comment #23)
>Since bug 375102 and this landed, SeaMonkey can not be launched directly from
>the Mac disk image any more, showing XML errors in chrome.
What happens if you expand and write protect the disk image (or try to run the expanded image as a user without write permission to it)?
Comment 25 Bjarne D Mathiesen 2007-08-21 10:17:03 PDT
https://www.mozdev.org/bugs/show_bug.cgi?id=17464

I've had this bug recently in connection with MultiZilla since 1.1.3
My workaround was to change the permissions on chrome.rdf to 666
Comment 26 WADA 2007-09-19 00:11:42 PDT
FYI.
When Win version of Seamonkey, following files are found in write-permitted
program directory only(all other files have same timestamp and same file size.)
 A. Seamonkey 1.1.4 (ZIP build)
    chrome\chrome.rdf      (also in chrome in profile) 
    components\compreg.dat (not found in profile)
    components\xpti.dat    (not found in profile)
 B. Seamonkey trunk 2007/9/17 build
    chrome\app-chrome.manifest (not found in profile)
    components\compreg.dat     (found in profile)
    components\xpti.dat        (found in profile)
Trunk still writes compreg.dat & xpti.dat in program directory if
write-permitted, and trunk writes them in profile too.
Comment 27 Robert Kaiser (not working on stability any more) 2007-09-19 04:36:53 PDT
WADA: This comment is useless, as first this is a branch-only probably that is not present in trunk, and second, we know perfectly that .rdf files, probably chrome.rdf, are the problem here. We just don't know where in the code we could ignore or fix an error that is causing this misbehavior.
Comment 28 WADA 2007-09-27 16:20:31 PDT
This bug has keyword of "fixed-seamonkey1.1.3" and OS is set to Linux.
But 389136 reports same problem of Seamonkey 1.1.4 on Mac OS & MS Windows.
Really "fixed-seamonkey1.1.3"?
Does "fixed-seamonkey1.1.3 & OS=Linux" mean following?
 Problem on Linux was resolved with 1.1.3 by this bug.
 But problem on Mac&MS Win was not resolved by this bug, then still remains.
Comment 29 Robert Kaiser (not working on stability any more) 2007-09-27 16:58:28 PDT
The main case, for an all-writeable directory (on all platforms) or a directory from which SeaMonkey never was launched (on Linux and Windows as it seems) was fixed for 1.1.3, but apparently there are still edge cases that see such a problem, like launching directly from a mac .dmg or launching from a non-writable directory that we could already write to in a different run.
Comment 30 WADA 2007-09-27 17:30:30 PDT
(In reply to comment #29)
> The main case, (snip) was fixed for 1.1.3, 
> but apparently there are still edge cases that see such a problem

Thanks, I've understood situation and difference between this bug and Bug 389136.
I think this bug is better to be closed as FIXED(fixed-seamonkey1.1.3 for main case, changing to OS=All) with pointing Bug 389136 as follow-up/successor for remaining edge case, in order to avoid confusion.
Comment 31 Robert Kaiser (not working on stability any more) 2007-09-28 13:25:58 PDT
yes, this might be a good idea. I changed the summary and other fields on that bug accordingly, and am marking this fixed instead.

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