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

RESOLVED FIXED in seamonkey2.48

Status

--
major
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: philip.chee, Assigned: frg)

Tracking

SeaMonkey 2.43 Branch
seamonkey2.48

SeaMonkey Tracking Flags

(seamonkey2.43 wontfix, seamonkey2.44 wontfix, seamonkey2.45 fixed, seamonkey2.46 fixed, seamonkey2.47 fixed, seamonkey2.48 fixed)

Details

User Story

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.

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

3 years ago
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.
(Reporter)

Updated

3 years ago
User Story: (updated)
(Assignee)

Comment 1

3 years ago
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?
(Reporter)

Comment 2

3 years ago
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.
(Reporter)

Comment 3

3 years ago
This is the bit that did the copy:
https://hg.mozilla.org/mozilla-central/rev/67f78903bbce#l2.109
(Assignee)

Comment 4

3 years ago
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
(Reporter)

Comment 5

3 years ago
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?
(Assignee)

Comment 6

3 years ago
>> 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
(Reporter)

Updated

2 years ago
Summary: Copy default profile files to new profiles since Bug 1234012 stopped doing that for us. → Copy default profile files to new profiles since Bug 1234012 stopped doing that for us. Or backout Bug 1234012 on a SeaMonkey Release branch
(Assignee)

Comment 7

2 years ago
Created attachment 8765297 [details] [diff] [review]
1240798-copyDefaults.patch

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

Tested only under Windows.
Assignee: nobody → frgrahl
Status: NEW → ASSIGNED
Attachment #8765297 - Flags: feedback?(philip.chee)
(Assignee)

Comment 8

2 years ago
Created attachment 8765298 [details] [diff] [review]
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 :)
Attachment #8765298 - Flags: feedback?(philip.chee)
Attachment #8765298 - Flags: feedback?(iann_bugzilla)
(Assignee)

Updated

2 years ago
status-seamonkey2.43: --- → wontfix
status-seamonkey2.44: --- → affected
status-seamonkey2.45: --- → affected
status-seamonkey2.46: --- → affected
status-seamonkey2.47: --- → affected
(Assignee)

Comment 9

2 years ago
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.
(Assignee)

Comment 10

2 years ago
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.
Severity: blocker → major
(Reporter)

Comment 11

2 years ago
(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.
(Reporter)

Comment 12

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

Let's not touch toolkit.
Attachment #8765298 - Flags: feedback?(philip.chee)
Attachment #8765298 - Flags: feedback?(iann_bugzilla)
Attachment #8765298 - Flags: feedback-
(Reporter)

Comment 13

2 years ago
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...
Attachment #8765297 - Flags: feedback?(philip.chee)
(Assignee)

Comment 14

2 years ago
Created attachment 8778608 [details] [diff] [review]
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?
Attachment #8765297 - Attachment is obsolete: true
Attachment #8765298 - Attachment is obsolete: true
Attachment #8778608 - Flags: review?(philip.chee)
(Assignee)

Updated

2 years ago
Summary: Copy default profile files to new profiles since Bug 1234012 stopped doing that for us. Or backout Bug 1234012 on a SeaMonkey Release branch → Copy default profile files to new profiles since Bug 1234012 stopped doing that for us.

Updated

2 years ago
Attachment #8778608 - Flags: review?(iann_bugzilla)
(Reporter)

Comment 15

2 years ago
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
Attachment #8778608 - Flags: review?(philip.chee) → review-
(Assignee)

Comment 16

2 years ago
Created attachment 8782695 [details] [diff] [review]
1240798-copyDefaults-V3.patch

And the name of version #3 is ... version #3
Attachment #8778608 - Attachment is obsolete: true
Attachment #8778608 - Flags: review?(iann_bugzilla)
Attachment #8782695 - Flags: review?(philip.chee)
(Assignee)

Updated

2 years ago
status-seamonkey2.44: affected → wontfix
status-seamonkey2.48: --- → affected
(Reporter)

Comment 17

2 years ago
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);
Attachment #8782695 - Flags: review?(philip.chee)
Attachment #8782695 - Flags: review-
Attachment #8782695 - Flags: feedback+
(Assignee)

Comment 18

2 years ago
Created attachment 8790078 [details] [diff] [review]
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: --
Attachment #8782695 - Attachment is obsolete: true
Attachment #8790078 - Flags: review?(philip.chee)
Attachment #8790078 - Flags: approval-comm-release?
Attachment #8790078 - Flags: approval-comm-beta?
Attachment #8790078 - Flags: approval-comm-aurora?
(Reporter)

Comment 19

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

a=me for all branches.
Attachment #8790078 - Flags: review?(philip.chee)
Attachment #8790078 - Flags: review+
Attachment #8790078 - Flags: approval-comm-release?
Attachment #8790078 - Flags: approval-comm-release+
Attachment #8790078 - Flags: approval-comm-beta?
Attachment #8790078 - Flags: approval-comm-beta+
Attachment #8790078 - Flags: approval-comm-aurora?
Attachment #8790078 - Flags: approval-comm-aurora+
(Assignee)

Comment 20

2 years ago
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",
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-seamonkey2.45: affected → fixed
status-seamonkey2.46: affected → fixed
status-seamonkey2.47: affected → fixed
status-seamonkey2.48: affected → fixed
Resolution: --- → FIXED
(Assignee)

Updated

2 years ago
Target Milestone: --- → seamonkey2.48
(Assignee)

Updated

2 years ago
Version: unspecified → SeaMonkey 2.43 Branch
You need to log in before you can comment on or make changes to this bug.