1240798 - Copy default profile files to new profiles since Bug 1234012 stopped doing that for us.

Status: RESOLVED FIXED

(SeaMonkey :: Startup & Profiles, defect)

Description

[Philip Chee]

Situation: Bug 1234012 stopped copying the defaults profile files to new profiles.
Requirement: copy these files to newly created profiles.
Proposed Strategy:
1. In nsToolkitProfileService.cpp -> CreateProfileInternal add a notifyObservers(..., "profile-created", ...);
2. In Suite (nsSuiteGlue.js?) add a observer for "profile-created" and copy default files to new profile.

Comment 1

[Frank-Rainer Grahl (:frg)]

Did I miss something?

defaults->CopyToNative(aParentDir, aLeafName) which is even in the patch in Bug 1238428 already copies at least panels.rdf and bookmarks.html. 

Is there more which we need? I see a missing mimetypes.rdf and the user-css examples but are these still neded?

Comment 2

[Philip Chee]

mimetypes.rdf and bookmarks.html isn't copied during my testing.
panels.rdf isn't copied. But our sidebar code creates one anyway so I think we can remove that.
the chrome/user*example.css files are what I want to copy.

Comment 3

[Philip Chee]

This is the bit that did the copy:
https://hg.mozilla.org/mozilla-central/rev/67f78903bbce#l2.109

Comment 4

[Frank-Rainer Grahl (:frg)]

Hmm. Strange. bookmarks.html and panels.rdf are copied under Windows with the V3 patch. I just applied it clean and fired up the compiler. I accidently purged my local fixes so I am 100% sure nothing else was in the trunk to do it :)

I created the profile using the 'Create Profile Wizard'. If panels.rdf wern't copied the sidebar would be blank. I had this problem when I ripped out some parts when trying to check what was wrong with the previous patch.

Are you sure the user.css samples should be copied? Everyone who wants them would know were to look and 99% will never use them.

FRG

Comment 5

[Philip Chee]

I'll test this again. But note the sidebar code creates/overwrites the panels.rdf file on startup. Look at the contents of the panels.rdf file is it the contents of the defaults/profile.panels.rdf or a bigger file with more content?

> Are you sure the user.css samples should be copied? Everyone who wants them would know
> were to look and 99% will never use them.
?? Everyone who wants them would know to look in their %currentprofile%/chrome/ dir but we aren't copying them to new profiles in any more. That's the problem, no?

Comment 6

[Frank-Rainer Grahl (:frg)]

>> But note the sidebar code creates/overwrites the panels.rdf file on startup. Look at the contents of the panels.rdf file is it the contents of the defaults/profile.panels.rdf or a bigger file with more content?
<<

The build might have had other problems. I didn't check. I was unable to add something there. I thought it was used as a model and if missing nothing could be added.

>> Everyone who wants them would know to look in their %currentprofile%/chrome/ dir but we aren't copying them to new profiles in any more. That's the problem, no?

Wouldn't be for me as long as they are still distributed. Under Windows they are still in the defaults\profile directory under Seamonkey. Maybe a Samples directory should be created and they should be put in there. I am somewhat in line with the Firefox developers here (shock). Why install something which is probably never used.

FRG

Comment 7

[Frank-Rainer Grahl (:frg)]

Attachment: 1240798-copyDefaults.patch

Well to get the ball rolling on this one. Philip is that what you had in mind?

Tested only under Windows.

Comment 8

[Frank-Rainer Grahl (:frg)]

Attachment: xxxxx-profilecreatedObserver.patch

Anyone want to take the tookit part to a new bug? Have not messed yet much with core code and try builds etc... You can give credit to yourself for it :)

Comment 9

[Frank-Rainer Grahl (:frg)]

Is this still a blocker? I would set it to major and document it in the release notes until fully fixed but we are in the same boat as Firefox and currently because of bug 1238428 it seems to work with new profiles minus a few default files.

If we back out out bug 1234012 for a release then bug 1238428 also needs to be backed out.

Comment 10

[Frank-Rainer Grahl (:frg)]

I am dialing down the importance to Major. These red lines do not look good when viewing bugzilla :) If not fixed till next release should be documented in the release notes.

Comment 11

[Philip Chee]

(In reply to Frank-Rainer Grahl from comment #10)
> I am dialing down the importance to Major. These red lines do not look good
> when viewing bugzilla :) If not fixed till next release should be documented
> in the release notes.

Unfortunately the observer in nsSuiteGlue doesn't work if you start SeaMonkey with the Profile Manager and create a profile there.

I think the better way is to create a SeaMonkey specific release branch on mozilla-release and backout Bug 1234012 there.

Comment 12

[Philip Chee]

Comment on attachment 8765298 [details] [diff] [review]
xxxxx-profilecreatedObserver.patch

Let's not touch toolkit.

Comment 13

[Philip Chee]

Comment on attachment 8765297 [details] [diff] [review]
1240798-copyDefaults.patch

You're right. If we can do it without touching /mozilla/ files we should.
The only files we need are the user{Chrome|Content}-example.css files.
mimeTypes.rdf is just a stub and will be (re-)created when necessary.
panels.rdf is now unnecessary as the sidebar startup code recreates it when necessary.

So in nsSuiteGlue.js just forget about "profile-created" and use |case "profile-after-change"| and test for the presence of %profile%/chrome/ before copying...

Comment 14

[Frank-Rainer Grahl (:frg)]

Attachment: 1240798-copyDefaults-V2.patch

I opted for final-ui-startup. Pro is that its not checked during every profile change and so should be a little better performance wise. lmk if ok.

Tested with:

seamonkey --profilemanger
seamonkey switch profile
seammonkey first initial profile creation
exiting profile after deleting chrome dir. 

Should the dir service enclosed in a try/catch and/or put in an addition small method?

Comment 15

[Philip Chee]

Comment on attachment 8778608 [details] [diff] [review]
1240798-copyDefaults-V2.patch

> Should the dir service enclosed in a try/catch and/or put in an addition
> small method?
Additional small method please.

> I opted for final-ui-startup. Pro is that its not checked during every
> profile change and so should be a little better performance wise. lmk if ok

Every profile-after-change is followed by one final-ui-startup and every final-ui-startup is preceded by one profile-after-change.

https://developer.mozilla.org/en/docs/Observer_Notifications#Application_startup

nsSuiteGlue is init-ed by app-startup so our choices are:
  profile-do-change
  profile-after-change
  final-ui-startup.
The earlier the better e.g. profile-after-change

Comment 16

[Frank-Rainer Grahl (:frg)]

Attachment: 1240798-copyDefaults-V3.patch

And the name of version #3 is ... version #3

Comment 17

[Philip Chee]

Comment on attachment 8782695 [details] [diff] [review]
1240798-copyDefaults-V3.patch

This patch works for me but can be optimized a bit:

> +    // chrome dir does not exist.
> +    if (!profileDir.exists()) {
Do a early return here:

if (profileDir.exists())
  return;

> +    let enumerator = aSource.directoryEntries;
This will throw if aSource isn't a directory so in the caller above:

if (defaultProfileDir.exists() && defaultProfileDir.isDirectory()) {

> +        try {
> +          subdir.create(Components.interfaces.nsIFile.DIRECTORY_TYPE,
> +                        FileUtils.PERMS_DIRECTORY);
> +          this._copyDir(file, subdir);
> +        } catch (e) {
> +          Components.utils.reportError(e);
> +        }

Perhaps something like:
try {
  subdir.create(Components.interfaces.nsIFile.DIRECTORY_TYPE,
                FileUtils.PERMS_DIRECTORY);
}
catch (ex) {
  if (ex.result != Components.results.NS_ERROR_FILE_ALREADY_EXISTS)
    throw ex;
}
this._copyDir(file, subdir);

Comment 18

[Frank-Rainer Grahl (:frg)]

Attachment: 1240798-copyDefaults-V4.patch

Feedback incorporated.

Just a note:
>>  if (ex.result != Components.results.NS_ERROR_FILE_ALREADY_EXISTS)
>>    throw ex;

This will never be true in the current context because of checking profileDir.exists() earlier. Buts its a nice little copy function so it could be used for other things in the future if needed.

[Approval Request Comment]
Regression caused by (bug #): 1234012
User impact if declined: example files will not be copied.
Testing completed (on m-c, etc.): c-r to c-c
Risk to taking this patch (and alternatives if risky): None. Tested and no side effects should be possible.
String changes made by this patch: --

Comment 19

[Philip Chee]

Comment on attachment 8790078 [details] [diff] [review]
1240798-copyDefaults-V4.patch

a=me for all branches.

Comment 20

[Frank-Rainer Grahl (:frg)]

Checked in:

https://hg.mozilla.org/comm-central/rev/fbc34e53a071
https://hg.mozilla.org/releases/comm-aurora/rev/1ad1e000411f
https://hg.mozilla.org/releases/comm-beta/rev/b6b1c7404ae9
https://hg.mozilla.org/releases/comm-release/rev/2062c13508a3

For Beta and Release I needed to put a trailing space on this (unchanged) line to apply clean:

>> Services.ww.openWindow(null, "chrome://communicator/content/safeMode.xul",