306842 - Pref overlays should use insertafter/insertbefore instead of position

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, defect)

Description

[Mark Banner (:standard8)]

From comments on bug 189821:

http://lxr.mozilla.org/seamonkey/source/editor/ui/composer/content/editorPrefsOverlay.xul#29
http://lxr.mozilla.org/seamonkey/source/xpfe/components/prefwindow/resources/content/pref-appearance.xul#65
http://lxr.mozilla.org/seamonkey/source/mailnews/base/prefs/resources/content/mailPrefsOverlay.xul#46
http://lxr.mozilla.org/seamonkey/source/calendar/resources/content/pref/prefOverlay.xul#60

At the locations above, and elsewhere in those files, we use position to
determine the order of the elements in the pref panes. Currently, mailnews &
calender conflict, with them both using position 3. We should be using
insertafter/insertbefore as its more reliable.

Comment 1

[Mark Banner (:standard8)]

(In reply to comment #0)
> From comments on bug 189821:
> 
> http://lxr.mozilla.org/seamonkey/source/editor/ui/composer/content/editorPrefsOverlay.xul#29
> http://lxr.mozilla.org/seamonkey/source/xpfe/components/prefwindow/resources/content/pref-appearance.xul#65
> http://lxr.mozilla.org/seamonkey/source/mailnews/base/prefs/resources/content/mailPrefsOverlay.xul#46
> http://lxr.mozilla.org/seamonkey/source/calendar/resources/content/pref/prefOverlay.xul#60

pref-appearance.xul and calendar's prefOverlay.xul no longer exist.

Comment 2

[Mark Banner (:standard8)]

Attachment: Possible patch

Comment 3

[Mark Banner (:standard8)]

(In reply to comment #2)
> Created an attachment (id=266112) [details]
> Possible patch
> 
Forgot to say: I'm still testing this, but hopefully this will be the one.

Comment 4

[[e-mail]]

(In reply to comment #1)
>(In reply to comment #0)
>>http://lxr.mozilla.org/seamonkey/source/xpfe/components/prefwindow/resources/content/pref-appearance.xul#65
>pref-appearance.xul and calendar's prefOverlay.xul no longer exist.
pref-appearance.xul does exist, but it got moved to suite/common/pref

Comment 5

[Mark Banner (:standard8)]

(In reply to comment #4)
> (In reply to comment #1)
> >(In reply to comment #0)
> >>http://lxr.mozilla.org/seamonkey/source/xpfe/components/prefwindow/resources/content/pref-appearance.xul#65
> >pref-appearance.xul and calendar's prefOverlay.xul no longer exist.
> pref-appearance.xul does exist, but it got moved to suite/common/pref
> 
Sorry, yes your right, however it doesn't seem to have any position attributes in it now.

Comment 6

[Mark Banner (:standard8)]

Attachment: The fix

This patch works properly.

Comment 7

[Ian Neal]

Comment on attachment 266177 [details] [diff] [review]
The fix

r=me with non-top level ones to follow in another patch (permissions and wallet)

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 266177 [details] [diff] [review]
The fix

Not sure if sr is required or not for editor, so requesting it just in case.

Comment 9

[[e-mail]]

Comment on attachment 266177 [details] [diff] [review]
The fix

Sorry, you can't use insertbefore and insertafter on the same element.

Comment 10

[[e-mail]]

(In reply to comment #9)
>(From update of attachment 266177 [details] [diff] [review])
>Sorry, you can't use insertbefore and insertafter on the same element.
To be precise, insertbefore is ignored if insertafter is specified.

Comment 11

[[e-mail]]

Comment on attachment 266177 [details] [diff] [review]
The fix

On second thoughts, I don't think you need those insertbefore attributes, so sr=me if you remove them.

Comment 12

[Mark Banner (:standard8)]

(In reply to comment #11)
> (From update of attachment 266177 [details] [diff] [review])
> On second thoughts, I don't think you need those insertbefore attributes, so
> sr=me if you remove them.
> 
I just checked, it does work without the insertbefore attributes. I think I was getting mixed up when I was originally trying to get the patch working. I'll drop them before checkin.

Comment 13

[Mark Banner (:standard8)]

(In reply to comment #11)
> (From update of attachment 266177 [details] [diff] [review])
> On second thoughts, I don't think you need those insertbefore attributes, so
> sr=me if you remove them.
> 
Checked in with inserbefore attributes removed.

Comment 14

[Mark Banner (:standard8)]

Attachment: Sort out permissionsPrefsOverlay and walletPrefsOverlay

This sorts out the positions attributes in walletPrefsOverlay and permissionsPrefsOverlay. I've also removed permissionsPrefsOverlay.{xul,dtd} as per discussion on irc that said the permissions didn't really need to be a separate overlay any longer.

Comment 15

[Mark Banner (:standard8)]

Attachment: contents.rdf fix

So as not to break xpfe builds for the little time they have remaining...

Comment 16

[Mark Banner (:standard8)]

Last two patches checked in -> fixed.