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

VERIFIED FIXED in mozilla0.9.3


MailNews: Message Display
17 years ago
10 years ago


(Reporter: Blake Ross, Assigned: Blake Ross)



Firefox Tracking Flags

(Not tracked)


(Whiteboard: r=hwaara, sr=sspitzer, URL)


(3 attachments)



17 years ago
Patch coming.

Comment 1

17 years ago
Created attachment 36893 [details] [diff] [review]
so, looks ok, but without a close button, this window should be opened as a 
window rather than as a dialog. with that. 

Comment 3

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

Comment 4

17 years ago
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

17 years ago
There is also bug number 82013 for what I gather is the radio button switching
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

17 years ago
Cc'ing Matthew for Close button discussion.

Comment 7

17 years ago
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

17 years ago
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

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

Comment 10

17 years ago
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.
OS: Windows 2000 → All
Hardware: PC → All

Comment 11

17 years ago
Navin, can you r= this?
Whiteboard: fix in hand
Target Milestone: --- → mozilla0.9.2

Comment 12

17 years ago
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

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

Comment 14

17 years ago
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?

Summary: Search has some visual problems → remove close button from search and make it a window, not a dialog.
Whiteboard: fix in hand → waiting for jglick to comment

Comment 16

17 years ago
No Close button is ok by me. 
thanks for the follow up, jglick.  can you update the spec?

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

Whiteboard: waiting for jglick to comment

Comment 18

17 years ago
To make it a window:

RCS file: 
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

17 years ago
seth, are you okay with this?
Target Milestone: mozilla0.9.2 → mozilla0.9.3
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.
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

17 years ago
Also, can you attach a screenshot?
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 

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

Comment 26

17 years ago

Comment 27

17 years ago
*** Bug 86197 has been marked as a duplicate of this bug. ***
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.

Keywords: 4xp
Whiteboard: r=hwaara, sr=sspitzer, a=?

Comment 29

17 years ago
We don't need a= tomorrow when trunk opens for 0.9.3 (unless we want it to the
I don't have plans to land this on the brach.

I'll land it tomorrow on the trunk when the tree opens.
Whiteboard: r=hwaara, sr=sspitzer, a=? → r=hwaara, sr=sspitzer
fixed landed on the trunk.
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 32

17 years ago
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.
Keywords: vtrunk

Comment 33

17 years ago
OK using aug10 commercial trunk build: win98, mac OS 9.0 and linux rh6.2
Product: Browser → Seamonkey


10 years ago
Component: MailNews: Search → MailNews: Message Display
QA Contact: laurel → search
You need to log in before you can comment on or make changes to this bug.