If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Pref overlays should use insertafter/insertbefore instead of position

RESOLVED FIXED

Status

SeaMonkey
Preferences
--
minor
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

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

Comment 1

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

Comment 2

11 years ago
Created attachment 266112 [details] [diff] [review]
Possible patch
Assignee: prefs → bugzilla
Status: NEW → ASSIGNED
(Assignee)

Comment 3

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

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

Comment 5

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

Comment 6

11 years ago
Created attachment 266177 [details] [diff] [review]
The fix

This patch works properly.
Attachment #266112 - Attachment is obsolete: true
Attachment #266177 - Flags: review?(iann_bugzilla)

Comment 7

11 years ago
Comment on attachment 266177 [details] [diff] [review]
The fix

r=me with non-top level ones to follow in another patch (permissions and wallet)
Attachment #266177 - Flags: review?(iann_bugzilla) → review+
(Assignee)

Comment 8

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

Comment 9

11 years ago
Comment on attachment 266177 [details] [diff] [review]
The fix

Sorry, you can't use insertbefore and insertafter on the same element.
Attachment #266177 - Flags: superreview?(neil) → superreview-

Comment 10

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

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

Comment 12

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

Comment 13

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

Comment 14

11 years ago
Created attachment 266354 [details] [diff] [review]
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.
Attachment #266354 - Flags: review?(iann_bugzilla)

Updated

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

Updated

11 years ago
Attachment #266354 - Flags: superreview?(neil)
(Assignee)

Comment 15

11 years ago
Created attachment 266371 [details] [diff] [review]
contents.rdf fix

So as not to break xpfe builds for the little time they have remaining...
Attachment #266371 - Flags: superreview?(neil)
Attachment #266371 - Flags: review?(iann_bugzilla)

Updated

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

Updated

11 years ago
Attachment #266354 - Flags: superreview?(neil) → superreview+

Updated

11 years ago
Attachment #266371 - Flags: superreview?(neil) → superreview+
(Assignee)

Comment 16

11 years ago
Last two patches checked in -> fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.