<page>s and <dialog>s don't need dialogOverlay

RESOLVED FIXED in seamonkey2.1b3

Status

SeaMonkey
Composer
--
minor
RESOLVED FIXED
15 years ago
6 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: ewong)

Tracking

(Blocks: 1 bug)

Trunk
seamonkey2.1b3

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

15 years ago
I noticed that the following files use dialogOverlay - this seems superfluous.
pref-publish.xul
pref-applications-edit.xul
turboDialog.xul
Product: Browser → Seamonkey
Assignee: bugs → prefs
QA Contact: bugzilla
(Filter "spam" on 'prefs-nobody-20080612'.)
Assignee: prefs → nobody
QA Contact: prefs
(Assignee)

Comment 2

6 years ago
Is this bug still valid on trunk?
(Reporter)

Comment 3

6 years ago
Certainly those files either no longer exist or no longer use dialogOverlay, however there are still a number of files left that do use dialogOverlay and probably shouldn't.

Comment 4

6 years ago
Two remaining uses of dialogOverlay.xul in /editor/ui
http://mxr.mozilla.org/comm-central/source/editor/ui/dialogs/content/EdInsertTOC.xul
http://mxr.mozilla.org/comm-central/source/editor/ui/dialogs/content/EdInsertTOC.xul
Blocks: 197315
(Assignee)

Comment 5

6 years ago
Taking this for a spin.
Assignee: nobody → ewong
Status: NEW → ASSIGNED
(Assignee)

Comment 6

6 years ago
Created attachment 519398 [details] [diff] [review]
Removed instance of dialogOverlay.xul
Attachment #519398 - Flags: review?(iann_bugzilla)
(Assignee)

Updated

6 years ago
Attachment #519398 - Flags: review?(iann_bugzilla)
(Assignee)

Comment 7

6 years ago
Created attachment 519400 [details] [diff] [review]
Removed instance of dialogOverlay.xul (v2)
Attachment #519398 - Attachment is obsolete: true
Attachment #519400 - Flags: review?(iann_bugzilla)
FTR: Note that editor/ is under TB control, i.e. you'll need an sr from bienvenu or Standard8 in the end and whoever checks this in will need to check whether the TB tree is open.
This bug was filed when only SeaMonkey existed.
Edmund, would you care to fix the other c-c (and maybe m-c) occurrences, after fixing SeaMonkey?
Severity: normal → minor
Component: Preferences → Composer
OS: Windows 95 → All
QA Contact: preferences → composer
Hardware: x86 → All

Comment 10

6 years ago
(In reply to comment #8)
> FTR: Note that editor/ is under TB control, i.e. you'll need an sr from
> bienvenu or Standard8 in the end and whoever checks this in will need to check
> whether the TB tree is open.

Actually those bits that are shared are under TB control, see http://mxr.mozilla.org/comm-central/source/editor/ui/jar.mn#82 onwards for what is only used by SM.
Severity: minor → normal
Component: Composer → Preferences
Hardware: All → x86

Updated

6 years ago
Hardware: x86 → All
(In reply to comment #10)
> http://mxr.mozilla.org/comm-central/source/editor/ui/jar.mn#82 onwards for what

(I don't know either...)

Then:
EdDialogTemplate.* isn't referenced anywhere. Maybe it should be used or deleted?
EdInsertTOC.xul is shared.
Severity: normal → minor
Component: Preferences → Composer
Comment on attachment 519400 [details] [diff] [review]
Removed instance of dialogOverlay.xul (v2)

I assume dialogOverlay.js needs to be removed too...

Comment 13

6 years ago
(In reply to comment #11)
> (In reply to comment #10)
> > http://mxr.mozilla.org/comm-central/source/editor/ui/jar.mn#82 onwards for what
> 
> (I don't know either...)
> 
> Then:
> EdDialogTemplate.* isn't referenced anywhere. Maybe it should be used or
> deleted?
> EdInsertTOC.xul is shared.

If you look at the EdDialogTemplate.xul file itself, lines 45 and 56 are particularly interesting. Probably need to check with kaze on whether the EdDialogTemplate.* files can be removed but that is outside the scope of this bug.

Comment 14

6 years ago
(In reply to comment #12)
> Comment on attachment 519400 [details] [diff] [review]
> Removed instance of dialogOverlay.xul (v2)
> 
> I assume dialogOverlay.js needs to be removed too...

Correct for this patch.

Comment 15

6 years ago
Comment on attachment 519400 [details] [diff] [review]
Removed instance of dialogOverlay.xul (v2)

EdDialogTemplate.* is used as a template for any EdDialog*.* files, so probably best to keep it updated until kaze says otherwise.

As Serge said, need to remove the dialogOverlay.js lines in both files too i.e.
<script type="application/javascript" src="chrome://global/content/dialogOverlay.js"/>

r=me with that fixed.

You will need an additional r or sr from bienvenu or standard8
Attachment #519400 - Flags: review?(iann_bugzilla) → review+

Comment 16

6 years ago
(In reply to comment #9)
> This bug was filed when only SeaMonkey existed.
> Edmund, would you care to fix the other c-c (and maybe m-c) occurrences, after
> fixing SeaMonkey?

Looking through:
the two in suite/mailnews (one is still a window with class="dialog") probably need a new bug dependent on bug 197315
the one in suite/browser again is a window with class="dialog" could be included in suite/mailnews bug or a separate one
the ones in calendar probably need their own bug.
the ones in mailnews should be covered in bug 226956
the ones in toolkit (some of which have a window with class="dialog") should have a new bug.
(Assignee)

Comment 17

6 years ago
Created attachment 519603 [details] [diff] [review]
Removed instances of dialogOverlay.(xul|js) [Checkin: comment 23]
Attachment #519400 - Attachment is obsolete: true
Attachment #519603 - Flags: review?(iann_bugzilla)

Comment 18

6 years ago
> the one in suite/browser again is a window with class="dialog" could be
> included in suite/mailnews bug or a separate one
metadata.xul is blocked by the need for a way to have a dialog with no dialog buttons. See Bug 250609 Comment 0 and Bug 250609 Comment 17
Attachment #519603 - Flags: review?(bugzilla)

Comment 19

6 years ago
Comment on attachment 519603 [details] [diff] [review]
Removed instances of dialogOverlay.(xul|js) [Checkin: comment 23]

r=me
Attachment #519603 - Flags: review?(iann_bugzilla) → review+
Comment on attachment 519603 [details] [diff] [review]
Removed instances of dialogOverlay.(xul|js) [Checkin: comment 23]

Note that it looks like EdDialogTemplate.xul is unused:

http://mxr.mozilla.org/comm-central/search?string=EdDialogTemplate&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central
Attachment #519603 - Flags: review?(bugzilla) → review+

Comment 21

6 years ago
(In reply to comment #18)
> > the one in suite/browser again is a window with class="dialog" could be
> > included in suite/mailnews bug or a separate one
> metadata.xul is blocked by the need for a way to have a dialog with no dialog
> buttons. See Bug 250609 Comment 0 and Bug 250609 Comment 17

Seems that got fixed somewhere, so now works with buttons="none" attribute. Nothing stopping all those remaining windows being converted to dialogs it seems.

Comment 22

6 years ago
> Note that it looks like EdDialogTemplate.xul is unused:
See Comment 13
(Assignee)

Updated

6 years ago
Attachment #519603 - Flags: superreview?(bienvenu)

Updated

6 years ago
Attachment #519603 - Flags: superreview?(bienvenu) → superreview+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
Comment on attachment 519603 [details] [diff] [review]
Removed instances of dialogOverlay.(xul|js) [Checkin: comment 23]

http://hg.mozilla.org/comm-central/rev/c5424d36a8b1
Attachment #519603 - Attachment description: Removed instances of dialogOverlay.(xul|js) → Removed instances of dialogOverlay.(xul|js) [Checkin: comment 23]
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b3
You need to log in before you can comment on or make changes to this bug.