44348 - Search UI: reset only clears criteria rows, not results.

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect, P4)

Description

[laurel]

Using jun30 m17 commercial build

After finding results with one set of search criteria, clicking the reset button
only clears the criteria rows and not the previous search results.

1.  Go to mail window, open a mail account and login.
2.  Search|Search Mail.  Select a mail folder, launch a simple search which you
know will result in matches --> search results pane opens with the results.
3.  Click the Reset button.

Result:  Only criteria is reset to a fresh state.  The results pane still lists
the previous search results.

Expected:  Results should be cleared (if we follow 4.x behavior, the result pane
should also disappear) as well as the criteria.

Comment 1

[laurel]

cc jglick for opinion on whether we should take the result pane away on Reset.
(not spec'd)

Comment 2

[Alec Flett]

I'm with laurel - I think they should be cleared.

Comment 3

[jglick]

Both criteria and results pane should be cleared.

As for the results pane disappearing when clicking Reset, I don't think that is 
necessary at long as it goes clear.  If the person clicks Reset instead of 
close, its probably because they want to do another search.  So having the 
result pane close just to reopen again (when next search is done) isn't 
necessary (and may be annoying for some folks).

Comment 4

[Alec Flett]

Just a comment - I wasn't planning on ever having the results pane 
collapse/expand as it does in 4.x...

Comment 5

[jglick]

Even better reason for NOT doing it!  ;-)

Comment 6

[laurel]

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

Comment 7

[scottputterman]

Mail triage is marking nsbeta3-

Comment 8

[jglick]

Add mail3 keyword so bug considered for 6.5

Comment 9

[scottputterman]

reassigning to gayatrib

Comment 10

[scottputterman]

marking nsbeta1-

Comment 11

[scottputterman]

reassigning to naving

Comment 12

[Håkan Waara]

I don't think there should be a Clear button at all. 

It's gotta be pretty annoying for the user if it accidently clicks the Clear 
buttons and both all results found *and* the criteria used to find it in the 
first place gets removed.

I vote for invalid.

Comment 13

[timeless]

having 5 fields in a query and needing to clear them individually just to get a 
basic search back really sucks...  otoh i'm not sure there's a need to clear 
the results, the moment you search for something else they're cleared, and 
until then they might be useful to you as you try to build your next query.

Comment 14

[laurel]

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

Comment 15

[laurel]

Seth's comments from the newer bug:
------- Additional Comment #1 From Seth Spitzer 2002-05-22 11:25 -------

saw this during jglick's UE study, so adding [ue] to status whiteboard.

I could of sworn I saw that clear also didn't clear the text area in the first
term, but that could have been my imagination.

Comment 16

[[e-mail] (gone - use [e-mail] instead)]

Attachment: Patch

Comment 17

[[e-mail] (gone - use [e-mail] instead)]

I've tested this and it works in my tree - I borrowed the logic from Seth's
addressbook search code, which get the boxObject, and sets the view property to
null.  Ours is the same, just named differently, 'treeBoxObject'.

In the patch, I also clear out the status feedback text.

Comment 18

[Navin Gupta]

Comment on attachment 97905 [details] [diff] [review]
Patch

>? navin.txt
>Index: SearchDialog.js
>===================================================================
>RCS file: /cvsroot/mozilla/mailnews/base/search/resources/content/SearchDialog.js,v
>retrieving revision 1.60
>diff -u -7 -r1.60 SearchDialog.js
>--- SearchDialog.js	14 Aug 2002 22:12:17 -0000	1.60
>+++ SearchDialog.js	5 Sep 2002 04:34:39 -0000
>@@ -265,17 +265,14 @@
> }
> 
> 
> function onSearchStop() {
>     gSearchSession.interruptSearch();
> }
> 
function onReset() {
>+    var tree = GetThreadTree(); // get the ThreadTree
>+    tree.treeBoxObject.view = null; // assign the TreeBoxObject to null
>+    gStatusFeedback.showStatusString(""); // clear out the status tex
}

> function getFirstItemByTag(root, tag)
> {
>     var node;
>     if (root.localName == tag)
>         return root;
> 
>     if (root.childNodes) {
>Index: searchTermOverlay.js
>===================================================================
>RCS file: /cvsroot/mozilla/mailnews/base/search/resources/content/searchTermOverlay.js,v
>retrieving revision 1.24
>diff -u -7 -r1.24 searchTermOverlay.js
>--- searchTermOverlay.js	3 May 2002 21:58:10 -0000	1.24
>+++ searchTermOverlay.js	5 Sep 2002 04:34:39 -0000
>@@ -446,14 +446,17 @@
> }
> 
> function onReset(event)
> {
>     while (gTotalSearchTerms>0)
>         removeSearchRow(--gTotalSearchTerms);
>     onMore(event);
> }
> 
> function convertPRTimeToString(tm)
> {
>   var time = new Date();
>   // PRTime is in microseconds, Javascript time is in seconds
>   // so divide by 1000 when converting


Note above, I moved those lines to SearchDialog.js.  SearchTermOverlay is used
by ABSearch code also so you cannot put it here. Also that function should be
renamed to OnResetSearch() or something to avoid confusion.

Comment 19

[[e-mail] (gone - use [e-mail] instead)]

Attachment: Patch, addresses Navin's review

Moved the logic into SearchDialog.js, renamed it to onResetSearch(event), and
changed SearchDialog.xul to call the newly renamed method onResetSearch.

Comment 20

[[e-mail] (gone - use [e-mail] instead)]

Attachment: Patch, ready for final reviews. Calls onReset, to avoid duplicate code.

Comment 21

[Navin Gupta]

Comment on attachment 97923 [details] [diff] [review]
Patch, ready for final reviews.  Calls onReset, to avoid duplicate code.

thx, r=naving

Comment 22

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

Comment on attachment 97923 [details] [diff] [review]
Patch, ready for final reviews.  Calls onReset, to avoid duplicate code.

sr=sspitzer

Comment 23

[Asa Dotzler [:asa]]

Comment on attachment 97923 [details] [diff] [review]
Patch, ready for final reviews.  Calls onReset, to avoid duplicate code.

a=asa (on behalf of drivers) for checkin to 1.2a

Comment 24

[[e-mail] (gone - use [e-mail] instead)]

Fixed on trunk.

Comment 25

[[e-mail] (gone - use [e-mail] instead)]

I'm only temporarily reopening this so I can reassign to myself, put a target
milestone on it, etc.

Comment 26

[[e-mail] (gone - use [e-mail] instead)]

reassigning

Comment 27

[[e-mail] (gone - use [e-mail] instead)]

arg, try two. (sorry for spamming you all)

Comment 28

[[e-mail] (gone - use [e-mail] instead)]

This made 1.2alpha's milestone because it landed on the trunk.  Marking fixed.

Comment 29

[laurel]

OK using sep16 commercial trunk build: win98, mac OS 10.1, linux rh6.2