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

VERIFIED FIXED in mozilla0.9.3

Status

VERIFIED FIXED
18 years ago
11 years ago

People

(Reporter: bugzilla, Assigned: bugzilla)

Tracking

Trunk
mozilla0.9.3

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(3 attachments)

(Assignee)

Description

18 years ago
Patch coming.
(Assignee)

Comment 1

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

sr=ben@netscape.com with that. 

Comment 3

18 years ago
Could you provide some detail about the problems you see/were fixed?
(Assignee)

Comment 4

18 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

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

Comment 6

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

Comment 7

18 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.
(Assignee)

Comment 8

18 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.
(Assignee)

Comment 9

18 years ago
Matthew, can you discuss the close button stuff again, or point to a place 
where you have?
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
(Assignee)

Comment 11

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

Comment 12

18 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

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

Comment 14

18 years ago
r=naving
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

18 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
(Assignee)

Comment 18

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

Comment 19

18 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

18 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 
buttons.

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

Comment 26

18 years ago
r=hwaara

Comment 27

18 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.

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

Comment 29

18 years ago
We don't need a= tomorrow when trunk opens for 0.9.3 (unless we want it to the
branch).
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 32

18 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

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

Updated

11 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.