remove files obsoleted by pref panel migration

RESOLVED FIXED in seamonkey2.0a1

Status

RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: mnyromyr, Assigned: mnyromyr)

Tracking

Trunk
seamonkey2.0a1
Dependency tree / graph
Bug Flags:
blocking-seamonkey2.0a1 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 3 obsolete attachments)

(Assignee)

Description

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

Comment 1

11 years ago
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
Summary: kill pref-help.js → remove files obsoleted by pref panel migration
(Assignee)

Updated

11 years ago
Duplicate of this bug: 453715

Comment 3

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

Comment 4

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

Comment 5

11 years ago
adding blocking flag as a blocker depends on this fix.
Flags: blocking-seamonkey2.0a1+

Comment 6

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

Updated

11 years ago
Assignee: nobody → mnyromyr
Status: NEW → ASSIGNED
(Assignee)

Updated

11 years ago
No longer blocks: 394522
Depends on: 394522

Comment 7

11 years ago
oh, and please don't forget to remove the "READ THIS!" stuff from the new prefwindow.
(Assignee)

Comment 8

11 years ago
Created attachment 339343 [details] [diff] [review]
obsoletes in mozilla-central/xpfe, v1 [checked in]
Attachment #339343 - Flags: superreview?(neil)
Attachment #339343 - Flags: review?(neil)
(Assignee)

Comment 9

11 years ago
Created attachment 339344 [details] [diff] [review]
obsoletes in Venkman, v1
Attachment #339344 - Flags: review?
(Assignee)

Updated

11 years ago
Attachment #339344 - Flags: review? → review?(gijskruitbosch+bugs)
(Assignee)

Comment 10

11 years ago
Created attachment 339346 [details] [diff] [review]
obsoletes in Chatzilla, v1
Attachment #339346 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 11

11 years ago
Created attachment 339347 [details] [diff] [review]
obsoletes in DOMI, v1 [checked in]
Attachment #339347 - Flags: review?(sdwilsh)
(Assignee)

Comment 12

11 years ago
Created attachment 339351 [details] [diff] [review]
obsoletes in comm-central, v1

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
Attachment #339351 - Flags: superreview?(neil)
Attachment #339351 - Flags: review?(iann_bugzilla)
(Assignee)

Comment 13

11 years ago
s/I did//

Comment 14

11 years ago
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"/>
Attachment #339351 - Flags: review?(iann_bugzilla) → review-
(Assignee)

Comment 15

11 years ago
(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 16

11 years ago
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).
Attachment #339351 - Flags: superreview?(neil) → superreview+

Updated

11 years ago
Attachment #339343 - Flags: superreview?(neil)
Attachment #339343 - Flags: superreview+
Attachment #339343 - Flags: review?(neil)
Attachment #339343 - Flags: review+

Comment 20

11 years ago
(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 21

11 years ago
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.
Attachment #339344 - Flags: review?(gijskruitbosch+bugs) → review-

Comment 22

11 years ago
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! :-)
Attachment #339346 - Flags: review?(gijskruitbosch+bugs) → review+

Comment 23

11 years ago
(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...

Comment 24

11 years ago
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
Attachment #339347 - Flags: review?(sdwilsh)
(Assignee)

Comment 26

11 years ago
Created attachment 339697 [details] [diff] [review]
obsoletes in Venkman, v2 [checked in]

Backwards compatibility for Venkman.
Attachment #339344 - Attachment is obsolete: true
Attachment #339697 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 27

11 years ago
Created attachment 339698 [details] [diff] [review]
obsoletes in Chatzilla, v2 [checked in]

Fixed nits as per comments #22/23.
Attachment #339346 - Attachment is obsolete: true
Attachment #339698 - Flags: review+
(Assignee)

Comment 28

11 years ago
Created attachment 339699 [details] [diff] [review]
obsoletes in comm-central, v2 [checked in]

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)
Attachment #339351 - Attachment is obsolete: true
Attachment #339699 - Flags: superreview+
Attachment #339699 - Flags: review?(iann_bugzilla)
(Assignee)

Updated

11 years ago
Blocks: 456301
(Assignee)

Comment 29

11 years ago
> >+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 30

11 years ago
Comment on attachment 339697 [details] [diff] [review]
obsoletes in Venkman, v2 [checked in]

Brilliant, thanks!

r=me
Attachment #339697 - Flags: review?(gijskruitbosch+bugs) → review+

Updated

11 years ago
Attachment #339699 - Flags: review?(iann_bugzilla) → review+
(Assignee)

Comment 31

11 years ago
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>.
Attachment #339347 - Attachment description: obsoletes in DOMI, v1 → obsoletes in DOMI, v1 [checked in]
(Assignee)

Comment 32

11 years ago
Comment on attachment 339697 [details] [diff] [review]
obsoletes in Venkman, v2 [checked in]

Landed on Venkman trunk aka 1.9.0.
Attachment #339697 - Attachment description: obsoletes in Venkman, v2 → obsoletes in Venkman, v2 [checked in]
(Assignee)

Comment 33

11 years ago
Comment on attachment 339698 [details] [diff] [review]
obsoletes in Chatzilla, v2 [checked in]

Landed on Chatzilla trunk aka 1.9.0.
Attachment #339698 - Attachment description: obsoletes in Chatzilla, v2 → obsoletes in Chatzilla, v2 [checked in]
(Assignee)

Updated

11 years ago
Attachment #339343 - Attachment description: obsoletes in mozilla-central/xpfe, v1 → obsoletes in mozilla-central/xpfe, v1 [checked in]
(Assignee)

Comment 34

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

Comment 35

11 years ago
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>.
Attachment #339699 - Attachment description: obsoletes in comm-central, v2 → obsoletes in comm-central, v2 [checked in]
(Assignee)

Comment 36

11 years ago
\o/
Please file new bugs for additional/forgotten cleanup.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Updated

11 years ago
Target Milestone: --- → seamonkey2.0alpha

Comment 37

11 years ago
(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.
You need to log in before you can comment on or make changes to this bug.