Closed Bug 224578 Opened 17 years ago Closed 17 years ago

Separate embedding/GRE default preferences from application preferences

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla1.7alpha

People

(Reporter: benjamin, Assigned: benjamin)

References

Details

Attachments

(2 files, 1 obsolete file)

The backend is now in place with bug 212222 to load default preferences from the
GRE. This bug is the "middle-end" work of separating all.js etc. into
embedding/GRE defaults prefs and prefs that are application-specific.
No longer depends on: 212222
Attached patch separate embedding prefs (obsolete) — Splinter Review
Whew, this is going to bitrot quickly, I'm afraid. To use this patch, apply it
and then copy (or symlink)

mozilla/modules/libpref/src/init/mailnews.js to mozilla/mailnews and
mozilla/modules/libpref/src/init/editor.js to mozilla/editor/ui/composer.js

Actually, I forgot one part of this patch: I updated standlone composer too,
but I'll post that separately for glazou to review.

--BDS
Attached patch Updated, part 1Splinter Review
ok, timeless had some nits picked over IRC... draft two. Follow the same
symlink/copy instructions as above.
Attachment #135850 - Attachment is obsolete: true
Comment on attachment 135983 [details] [diff] [review]
Updated, part 1

There is no appropriate reviewer for this kind of change, so I'm just gonna ask
four relevant people... most of this patch is a cut-n-paste job, though I did
move significant parts of the existing editor.js into all.js because it is
relevant to midas.

--BDS
Attachment #135983 - Flags: superreview?(darin)
Attachment #135983 - Flags: review?(timeless)
Comment on attachment 135984 [details] [diff] [review]
Part 2, standalone composer changes

alec, please take a look at both patches, not just this one
Attachment #135984 - Flags: superreview?(alecf)
Attachment #135984 - Flags: review?(daniel)
Note to the aviary: with this patch or soon after, we should get rid of the
various <aviary-app>/profile/all.js files, or prune them like I have done for
standalone composer. It won't break the build to leave them as they are, it just
makes maintenance rather "traumatic". Also, please take a look at the
xxxbsmedberg comments in part 1 of this patch, with regards to the toolkit... We
should separate the default prefs for the old toolkit and the new toolkit and
ship them as separate files, but I can't do this yet until we figure out whether
the new toolkit is going to ship with the GRE/XRE in some form.

--BDS
Why not doing the mozilla/browser and mozilla/mail part here?
because
1) I don't have a firebird objdir or much more disk space
2) I'm a lazy lout and not doing the *birds in this bug doesn't break anybody
this is for 1.7 alpha, right?
yes, I wouldn't dare land this now ;)
Target Milestone: --- → mozilla1.7alpha
Comment on attachment 135984 [details] [diff] [review]
Part 2, standalone composer changes

cool, it all looks great to me! sr=alecf
Attachment #135984 - Flags: superreview?(alecf) → superreview+
Comment on attachment 135984 [details] [diff] [review]
Part 2, standalone composer changes

>-pref("browser.display.use_document_fonts",  1);  // 0 = never, 1 = quick, 2 = always
>-pref("browser.display.use_document_colors", true);
>-pref("browser.display.use_system_colors",   false);
>-pref("browser.display.foreground_color",    "#000000");
>-pref("browser.display.background_color",    "#FFFFFF");

All above needed.

>-pref("browser.anchor_color",                "#0000EE");
>-pref("browser.active_color",                "#EE0000");
>-pref("browser.visited_color",               "#551A8B");
>-pref("browser.underline_anchors",           true);

All above needed.
Attachment #135984 - Flags: review?(daniel) → review-
Comment on attachment 135984 [details] [diff] [review]
Part 2, standalone composer changes

r=daniel@glazman.org after discussion with bsmedberg over IRC
Attachment #135984 - Flags: review- → review+
Comment on attachment 135983 [details] [diff] [review]
Updated, part 1

> // Use 17 for Ctrl, 18 for Alt, 224 for Meta, 0 for none. Mac settings in macprefs.js
change "in macprefs" to "overriden below" or whatever :)
> pref("ui.key.accelKey", 17);
> pref("ui.key.generalAccessKey", 18);
> pref("ui.key.menuAccessKey", 18);
>-pref("ui.key.menuAccessKeyFocuses", false);
>+
>+pref("ui.key.menuAccessKeyFocuses", false); // overridden below

make sure that the japanese font pref stuff isn't corrupted, I can't check
here.

future tasks (bug #s referenced in this bug would be great):
move midas stuff into its own file, since xpconnect and other apps shouldn't
have to pay for it.

We need a bug for uncommenting the editor.encode_entity pref ...

>+pref("plugin.scan.SunJRE", "1.3");
We probably need a bug to find out what version of the jre we want, i suspect
1.4 something these days

>+//The following pref is internal to Communicator. Please
>+//do *not* place it in the docs...
>+pref("netinst.profile.show_dir_overwrite_msg",  true);
this really could be removed since it wasn't even used in /classic/ as indexed
by lxr
http://landfill.mozilla.org/mxr-test/classic/search?string=show_dir_over  

someday pics really needs to die :), but we have a bug for that...
Attachment #135983 - Flags: review?(timeless) → review+
Priority: -- → P1
Checked in with xpinstall/minimo packaging updated. Thank you timeless for
catching my UTF8 cut-n-paste errors before I checked them in.

Bryner, I checked in a firebird build-bustage that was related to the prefs dir
being missing, sorry I didn't catch it sooner. Can you give ex-post-facto
approval for that checkin to the firebird partition:?

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/browser/app&command=DIFF_FRAMESET&file=Makefile.in&rev1=1.38&rev2=1.39&root=/cvsroot
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Blocks: 231170
Blocks: 231172
Attachment #135983 - Flags: superreview?(darin)
*** Bug 62755 has been marked as a duplicate of this bug. ***
*** Bug 207315 has been marked as a duplicate of this bug. ***
*** Bug 207317 has been marked as a duplicate of this bug. ***
in a current cvs build (Linux) all modifiers that untill recently were
"Ctrl+something" are now "Meta+something"
Ane the key to use is no longer the Ctrl button, but the more uncomfortably
located button with a picture of a wet flag.

Could it have something to do with this checkin?
rkaa: Yes, I noticed that as well. making a clean build fixed it.
As far as I can tell, not fixing Firebird in this patch caused significant
bustage, since Firebird has it's own all.js that's overwriting yours, but it
depended on not overwriting unix.js, etc.

(Also, needing to clobber, as described in the previous comments, is a bug that
should be fixed ASAP.)
(Or does the need to clobber come from the movement of all.js within dist/bin/?
 If so, there's only a need to clobber dist (or even just the one file), not
make a whole clean tree.)
Blocks: 231270
The bad firebird platform overrides are covered in bug 231200.

The need to clobber is due to the changed location of all.js and the fact that
the old one isn't cleaned up properly. Removing dist/bin/defaults/pref/* and
doing "export" should fix seamonkey problems.

I also messed up some MacOSX stuff with a couple #ifdef XP_UNIX sections that
should have excluded XP_MACOSX. Patch in bug 231270.
Blocks: 231200
Depends on: 231373
People, you lost keyboard settings from beos.js in current all.js!
http://bugzilla.mozilla.org/attachment.cgi?id=136834&action=edit

bsmedberg, i hope you will checkin missing parts as soon as possible:
pref("ui.key.accelKey", 18);
pref("ui.key.generalAccessKey", 17);
pref("ui.key.menuAccessKey", 17);
Blocks: 231593
I think this change broke platform pref files for thunderbird. See Bug #231784
where I'm trying to keep track of that. We really need to be careful about large
changes like this that don't get tested\changed on the various products.
C'mon, this broke every app except SeaMonkey -- that's not acceptable now, or
since last April, given the roadmap change.

/be
BeOS is now fixed, I had made my patch here before bug 178910 landed.

Brendan, that's not quite correct: I broke seamonkey too, for a while. As I
mentioned in comment #6, I didn't expect other embedding apps to break from this
checkin, and I did build FB with the patch, I just didn't catch the odd
platform-specific bugs that it caused. I only have tbird cleanup now, I think
(bug 231784).
*** Bug 222105 has been marked as a duplicate of this bug. ***
*** Bug 220529 has been marked as a duplicate of this bug. ***
Blocks: 224840
You need to log in before you can comment on or make changes to this bug.