83763 - remove close button from search and make it a window, not a dialog.

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Blake Ross]

Patch coming.

Comment 1

[Blake Ross]

Attachment: patch

Comment 2

[Ben Goodger (use ben at mozilla dot org for email)]

so, looks ok, but without a close button, this window should be opened as a 
window rather than as a dialog. 

sr=[e-mail] with that. 

Comment 3

[laurel]

Could you provide some detail about the problems you see/were fixed?

Comment 4

[Blake Ross]

The text of the two radio buttons was messed up, and the buttons shift back and 
forth (aren't horizontally aligned).  Also, the statusbar should be connected 
to the tree and there shouldn't be a Close button.

Comment 5

[laurel]

There is also bug number 82013 for what I gather is the radio button switching
problem.
I'm not sure what the comment about the status bar means.
I don't know why there shouldn't be a close button... spec'd for one.

Comment 6

[Blake Ross]

Cc'ing Matthew for Close button discussion.

Comment 7

[laurel]

If there's no close button, Mac would have problems -- there is no close box to
close the dialog other than the button. More work there.

Comment 8

[Blake Ross]

Ben pointed that out too and I changed the openDialog flags to make it open as 
a window. I'll attach that patch later.

Comment 9

[Blake Ross]

Matthew, can you discuss the close button stuff again, or point to a place 
where you have?

Comment 10

[Matthew T (active 1999-2002)]

Copied from bug 42090:
|
| Dialog                                 Document/utility window
| ------                                 -----------------------
| *   Modal to the parent window         *   Non-modal
| *   Cannot be minimized independently  *   Can be minimized
|     of the parent window
| *   Closed using any one of a row of   *   closed using a close box in the
|     buttons (e.g. `Cancel', `OK')          window chrome
| *   does not have menus (except for    *   may have menus
|     `Edit' on Mac OS)

Currently the Search window is a utility window which thinks its a dialog --
it's not modal, but it has a labelled button to close itself -- `Close'. If you
have a `Close' button, you can be pretty sure there's something wrong with your
user interface.

There's no particular reason for Search Messages to be a dialog: it doesn't need
to block you from doing other stuff in the three-pane window (it would be nice
if folder changes in the three-pane window were reflected in the search window,
but that's a relatively trivial problem), and it won't be confusing if it's left
open when other Mozilla windows are closed. Therefore, Blake is correct in
getting rid of the `Close' button and making it a window.

Comment 11

[Blake Ross]

Navin, can you r= this?

Comment 12

[Håkan Waara]

Hold off on this! I've got a big patch cleaning up the Filters UI that will
break if you touch searchTermOverlay.xul

Please wait and re-test this patch when the patch in bug 82773 is in!

Comment 13

[Håkan Waara]

FYI, I'll check it in before monday if I get approval.

Comment 14

[Navin Gupta]

r=naving

Comment 15

[(not reading, please use [e-mail] instead)]

wait, the spec has a close button.

hold off on this patch until jglick gets a chance to comment.

what about laurel's comment:

"If there's no close button, Mac would have problems -- there is no close box to
close the dialog other than the button. More work there."

"Ben pointed that out too and I changed the openDialog flags to make it open as 
a window. I'll attach that patch later."

where's that patch?  without that patch, mac would be hosed right?

Comment 16

[jglick]

No Close button is ok by me. 

Comment 17

[(not reading, please use [e-mail] instead)]

thanks for the follow up, jglick.  can you update the spec?

blake, please provide a new patch with the openDialog change.

Comment 18

[Blake Ross]

To make it a window:

RCS file: 
/cvsroot/mozilla/mailnews/base/resources/content/mailWindowOverlay.js,v
retrieving revision 1.74
diff -u -r1.74 mailWindowOverlay.js
--- mailWindowOverlay.js	2001/06/16 07:20:04	1.74
+++ mailWindowOverlay.js	2001/06/20 06:02:52
@@ -858,7 +858,7 @@
 function MsgSearchMessages()
 {
     var preselectedFolder = GetFirstSelectedMsgFolder();
-    window.openDialog("chrome://messenger/content/SearchDialog.xul", 
"SearchMail", "chrome,resizable,centerscreen", { folder: preselectedFolder });
+    window.openDialog("chrome://messenger/content/SearchDialog.xul", 
"SearchMail", "chrome,resizable,centerscreen,dialog=no", { folder: 
preselectedFolder });
 }
 
 function MsgFilters()

Comment 19

[Blake Ross]

seth, are you okay with this?

Comment 20

[(not reading, please use [e-mail] instead)]

If jglick is ok with it, I'm ok with it.

or, do you mean am I ok with the patch?  I haven't reviewed / tested it yet.  

Please attach the complete patch for review.

Comment 21

[(not reading, please use [e-mail] instead)]

blake, you're probably overload with other stuff.

feel free to re-assing to me and I can drive this in. (or find someone on the
mailnews team to help)

Comment 22

[Håkan Waara]

Also, can you attach a screenshot?

Comment 23

[(not reading, please use [e-mail] instead)]

I've applied and tested blake's patch. nice work, blake.

it works fine, and he's fix the classic skin problem with the "all / any" radio 
buttons.

I'll re-attach it in complete form, and a screen shot.

Comment 24

[(not reading, please use [e-mail] instead)]

Attachment: blake's complete fix.

Comment 25

[(not reading, please use [e-mail] instead)]

Attachment: screen shot (classic skin)

Comment 26

[Håkan Waara]

r=hwaara

Comment 27

[Håkan Waara]

*** Bug 86197 has been marked as a duplicate of this bug. ***

Comment 28

[(not reading, please use [e-mail] instead)]

another nice aspect of removing the close button is that blake removed this 
line of XUL:

<button label="&closeButton.label;" oncommand="onSearchStop(); window.close
();"/>

we had to add the extra call to onSearchStop to work around another bug.  (I'm 
always glad to remove work arounds.)

4.x did not have a close button, so adding 4xp.

sr=sspitzer

Comment 29

[Håkan Waara]

We don't need a= tomorrow when trunk opens for 0.9.3 (unless we want it to the
branch).

Comment 30

[(not reading, please use [e-mail] instead)]

I don't have plans to land this on the brach.

I'll land it tomorrow on the trunk when the tree opens.

Comment 31

[(not reading, please use [e-mail] instead)]

fixed landed on the trunk.

Comment 32

[laurel]

Adding vrtunk keyword for someone else to verify (trunk only).
Fix is trunk only, I'm not going to be looking at trunk builds until after
Netscape's commercial RTM.

Comment 33

[laurel]

OK using aug10 commercial trunk build: win98, mac OS 9.0 and linux rh6.2