Closed Bug 175792 Opened 19 years ago Closed 11 years ago

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

Categories

(SeaMonkey :: Composer, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.1b3

People

(Reporter: neil, Assigned: ewong)

References

(Blocks 1 open bug, )

Details

Attachments

(1 file, 2 obsolete files)

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
Is this bug still valid on trunk?
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.
Taking this for a spin.
Assignee: nobody → ewong
Status: NEW → ASSIGNED
Attachment #519398 - Flags: review?(iann_bugzilla)
Attachment #519398 - Flags: review?(iann_bugzilla)
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
(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
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...
(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.
(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 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+
(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.
Attachment #519400 - Attachment is obsolete: true
Attachment #519603 - Flags: review?(iann_bugzilla)
> 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 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+
(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.
> Note that it looks like EdDialogTemplate.xul is unused:
See Comment 13
Attachment #519603 - Flags: superreview?(bienvenu)
Attachment #519603 - Flags: superreview?(bienvenu) → superreview+
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
Closed: 11 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.