Once all pref panels have been migrated to the new prefwindow, pref-help.js is not needed anymore and can be removed. For migrated panels, the help topic is stored in the helpTopic attribute of the panel's <treeitem>.

Actually, all /suite files already as such in suite/common/jar.mn can go: # files deprecated by preferences.xul: begin content/communicator/pref/nsPrefWindow.js (pref/nsPrefWindow.js) content/communicator/pref/nsWidgetStateManager.js (/xpfe/global/resources/content/nsWidgetStateManager.js) content/communicator/pref/pref.xul (pref/pref.xul) content/communicator/pref/preftree.xul (pref/preftree.xul) content/communicator/pref/pref-help.js (pref/pref-help.js) # files deprecated by preferences.xul: end

erm, from bug 453715 lets add to the list: suite/security/prefs/PrefOverlay.xul

suite/locales/en-US/chrome/common/pref/PrefsOverlay.dtd can be killed as well.

adding blocking flag as a blocker depends on this fix.

Karsten, can you take this over? I'd like us to have all blockers actually assigned to someone as we enter the freeze period, to know who to talk to for getting that completed. And I think we can prepare a patch already for at least the file (e.g. jar.mn) changes, maybe even the removals themselves.

oh, and please don't forget to remove the "READ THIS!" stuff from the new prefwindow.

This is the core patch in this bug, the other patches may need this one before making sense. All these patches need to go in in one go. - Makes /suite/common/utilityOverlay.js:goPreferences accept just a pref panel id and fixes callers. - Remove various now obsolete files and overlay directives, plus access to the legacy prefwindow. Things left out: - Can /mozilla/security/manager/pki/resources/content/contents.rdf and the overlay used in it be removed? - Same question for /mozilla/extensions/wallet/resources/content/contents.rdf and walletPrefsOverlay.xul. - What about /mozilla/extensions/irc/xul/content/contents.rdf? - What about the stuff under /mozilla/embedding? I did

s/I did//

Comment on attachment 339351 [details] [diff] [review] obsoletes in comm-central, v1 >diff --git a/mailnews/base/resources/content/manifest.rdf b/mailnews/base/resources/content/manifest.rdf I'm not sure about deleting this file, there is pref related parts to it, but there is other stuff in it too. >- <!-- messenger taskbar/tasks menu items --> >- <!-- messenger items for Navigator --> >- <!-- messenger items for Messenger --> >- <!-- messenger items for Mail Compose --> >- <!-- messenger items for Addressbook --> >- <!-- messenger items for Select Addresses dialog --> >- <!-- messenger items for Composer --> >+++ b/mailnews/compose/resources/content/MsgComposeCommands.js > function DoCommandPreferences() > { >- goPreferences('mailnews', 'chrome://messenger/content/messengercompose/pref-composing_messages.xul', 'mailcomposepref'); >+ goPreferences('mailnews_pane'); Shouldn't this be 'composing_messages_pane'? >+++ b/suite/common/pref/preferences.xul You forgot to remove this line: <treeitem label="READ THIS!" prefpane="temp_readthis_pane"/>

(In reply to comment #14) > /mailnews/base/resources/content/manifest.rdf > I'm not sure about deleting this file, there is pref related parts to it, but > there is other stuff in it too. It's unused for years, AFAIK. And mxr doesn't even find a single user. > >+ goPreferences('mailnews_pane'); > Shouldn't this be 'composing_messages_pane'? Oh. Yes. > >+++ b/suite/common/pref/preferences.xul > You forgot to remove this line: > <treeitem label="READ THIS!" prefpane="temp_readthis_pane"/> Ah, the code is too clever, it hides broken entries. ;-) I'll wait for other comments before putting up a revised monster patch.

Comment on attachment 339351 [details] [diff] [review] obsoletes in comm-central, v1 >diff --git a/suite/common/utilityOverlay.js b/suite/common/utilityOverlay.js >-function goPreferences(containerID, paneURL, itemID) >+function goPreferences(paneID) Could we go and split this apart from the main patch so that the main comm-central-only patch can go in right away for alpha1 and the rest can be done later when those other pieces are completely reviewed?

> - Can /mozilla/security/manager/pki/resources/content/contents.rdf and the > overlay used in it be removed? contents.rdf was not used since 1.5a... the overlay is now in suite/security > - Same question for /mozilla/extensions/wallet/resources/content/contents.rdf > and walletPrefsOverlay.xul. Die, wallet, die! > - What about /mozilla/extensions/irc/xul/content/contents.rdf? Needed for backcompat.

(In reply to comment #12) > This is the core patch in this bug, the other patches may need this one before > making sense. All these patches need to go in in one go. Surely the order is: 1) venkman/chatzilla/domi 2) comm-central 3) mozilla-central

Comment on attachment 339351 [details] [diff] [review] obsoletes in comm-central, v1 I didn't spot anything but I'll test it tomorrow and let you know if I find anything else wrong. We need a followup bug/patch to rename preftree.dtd to preferences.dtd (and move the two entities from pref.dtd there too).

(In reply to comment #12) > Things left out: > - Can /mozilla/security/manager/pki/resources/content/contents.rdf and the > overlay used in it be removed? No contents.rdf should be left over in tree that's only used with hg trunk. > - Same question for /mozilla/extensions/wallet/resources/content/contents.rdf > and walletPrefsOverlay.xul. I wouldn't touch wallet, the stuff in there doesn't harm us (overlaying non-existent stuff) and we'll hopefully exclude wallet before the next alpha. > - What about /mozilla/extensions/irc/xul/content/contents.rdf? Still needed for xpfe-based suite. > - What about the stuff under /mozilla/embedding? Ignore it, is about to be removed. (In reply to comment #18) > (In reply to comment #12) > > This is the core patch in this bug, the other patches may need this one before > > making sense. All these patches need to go in in one go. > Surely the order is: > 1) venkman/chatzilla/domi > 2) comm-central > 3) mozilla-central Doesn't have to go that way if we split it like I proposed in comment #16.

Comment on attachment 339344 [details] [diff] [review] obsoletes in Venkman, v1 Pretty sure we need backcompat here, just like in the ChatZilla case. Would like to be able to give stable SeaMonkey users something better than what they currently have, if possible. r- on that.

Comment on attachment 339346 [details] [diff] [review] obsoletes in Chatzilla, v1 >Index: xul/content/commands.js >+ // open Mozilla/SeaMonkey preferences >+ if (goPreferences.arity == 1) >+ { >+ // SeaMonkey 2.x uses a toolkit <prefwindow> derivation >+ goPreferences('navigator_pane'); >+ } >+ else >+ { >+ // Mozilla, SeaMonkey 1.x, etc. >+ goPreferences('navigator', >+ 'chrome://chatzilla/content/prefpanel/pref-irc.xul', >+ 'navigator'); >+ } Please: - stick 'chrome://chatzilla/content/prefpanel/pref-irc.xul' in an intermediate const, - make the second goPreferences call a one-liner also, - just say "SeaMonkey 2.x" and "Mozilla, SeaMonkey 1.x, etc." as comments, -- on the same line as the if/else statements - and eliminate the braces. Makes it much more concise and readable, as far as I'm concerned. r=me with that. Thanks! :-)

(In reply to comment #22) > - stick 'chrome://chatzilla/content/prefpanel/pref-irc.xul' in an intermediate > const, In fact, I just realized this code has bitrotted. You want to remove the "prefpanel" bit, from what I can tell from our current packaging scheme...

CCing sdwilsh so that he actually gets this review request

Comment on attachment 339347 [details] [diff] [review] obsoletes in DOMI, v1 [checked in] rs=sdwilsh

Backwards compatibility for Venkman.

Fixed nits as per comments #22/23.

Changes since v1: - corrected pref target for message composition - removed temporary treeitem - merged pref.dtd into preftree.dtd (but didn't rename preftree.dtd as per comment #19 yet - I filed bug 456301 on this)

> >+function goPreferences(paneID) > > Could we go and split this apart from the main patch so that the main > comm-central-only patch can go in right away for alpha1 and the rest can be > done later when those other pieces are completely reviewed? No, I would still need to touch eg. Venkman and such, else those would try to open the removed pref.xul.

Comment on attachment 339697 [details] [diff] [review] obsoletes in Venkman, v2 [checked in] Brilliant, thanks! r=me

Comment on attachment 339347 [details] [diff] [review] obsoletes in DOMI, v1 [checked in] Landed on trunk as <http://hg.mozilla.org/dom-inspector/rev/215eee579d47>.

Comment on attachment 339697 [details] [diff] [review] obsoletes in Venkman, v2 [checked in] Landed on Venkman trunk aka 1.9.0.

Comment on attachment 339698 [details] [diff] [review] obsoletes in Chatzilla, v2 [checked in] Landed on Chatzilla trunk aka 1.9.0.

Comment on attachment 339343 [details] [diff] [review] obsoletes in mozilla-central/xpfe, v1 [checked in] Landed on mozilla-central trunk as <http://hg.mozilla.org/mozilla-central/rev/d839ce2da8ca>.

Comment on attachment 339699 [details] [diff] [review] obsoletes in comm-central, v2 [checked in] Landed in comm-central as <http://hg.mozilla.org/comm-central/rev/c794f257143c>.

\o/ Please file new bugs for additional/forgotten cleanup.

(In reply to Comment 27 From Karsten Düsterloh) > Created an attachment (id=339698) [details] + if (goPreferences.arity == 1) // SeaMonkey 2.x http://developer.mozilla.org/En/Core_JavaScript_1.5_Reference/Global_Objects/Function/Arity According to this page .arity has been deprecated since JS1.4 and from DevMo, I see that .length works in all platforms that .arity does except for the Nintendo Entertainment System (according to gavin) and I don't think Chatzilla works there either.