Closed Bug 517200 Opened 10 years ago Closed 10 years ago

[faceted search] Can't remove quick filter after switching from an active traditional Quicksearch filter to "Search all messages" (can cause blank message list although no filter set)

Categories

(Thunderbird :: Search, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0rc1

People

(Reporter: bugzilla2007, Assigned: bugzilla2007)

Details

Attachments

(1 file, 3 obsolete files)

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.4pre) Gecko/20090916 Shredder/3.0pre

You must believe me, I'm not looking for these things, they are looking for me... ;) So here we go, something special (perhaps rare):

Steps
1) pick traditional quicksearch (e.g. "Sender" - that's after bug 517187 :))
2a) enter search term that doesn't have matches in msg list (e.g. "hello"), press Enter -> find nothing (ok)
2b) alternatively, enter search term that has few matches in msg list 
3) think "let my try this faceted thingy" on my search term: click magnifier for quicksearch dropdown, pick "search everwhere" option, -> be happy search term is carried over and facy thingy has found something!
(don't think: keyboard navigation and tooltips are missing)
4) think "let me try this again", close faceted search tab

Actual result
- be confused because Quick Search filter is now empty, but message list is also empty (wrong)
- in case of 2b), be confused because Quick Search filter is now empty, but the former filter is still active so you only see those few messages that match the invisible traditional quicksearch filter word(s)

Expected result
- with no quicksearch filter set, message list should always show all messages
For bonus points, also munge the opposite bug where hopping folders with a traditional quick search filter set will keep the filter term but show ALL messages. :/  jaja, I'll post that as a new bug so that it can be munged orderly.
similar Bug 517202 - Quick Search: Search term sticks across folders (nice), but it doesn't filter anything
I'd appreciate confirmation for this bug. 

This bug is easy to get in and seemingly difficult to get out, because:
Actual behaviour ctd:
- clicking on the blue "remove filter" button in quicksearch will NOT remove the filter, although it is still applied.

Not sure if blocking, but user experience is bad.
Suppose this might be fixed rather easily:

Expected behaviour:

- Whenever user switches from a traditional qs mode to faceted search (fs) mode
- and there is a traditional qs filter applied:
-> do not remove filter words from qs inputbox (currently ok), but
-> remove the actual qs filter that is currently applied to the view before proceeding with fs (that's the fix)
Flags: wanted-thunderbird3?
This bug really has potential for a lot of confusion.
It's easy to get in and difficult to get out of.
Basically, this bug WILL happen to anyone who
- has a traditional quicksearch like "Subject, From, or Recipient" set as default (potentially a lot of former tb2 users, to avoid clumsy "Search all messages" with two extra tabs)
- and uses "Search all messages" for more advanced searches in cases where just filtering current view doesn't find desired matches.
Severity: normal → major
Flags: blocking-thunderbird3?
Summary: [faceted search] Switching between traditional Quicksearch and Search everywhere sometimes causes blank message list although no filter set → [faceted search] Can't remove quick filter after switching from an active traditional Quicksearch filter to "Search all messages" (can cause blank message list although no filter set)
not nice, but not major since it is easily worked around, and additionally an edge case.

if it's similar to other bugs, it's not a filter causing the lack of messages in thread pane, is that the view is blown.  Sorry, not finding any examples.
Severity: major → minor
(In reply to comment #5)
> not nice, but not major
Agreed, but "not major" doesn't make it "minor". Definition of "normal": "It's a bug that should be fixed".

> Since it is easily worked around
If you consider switching to another folder an "easy" workaround for daily operations, then yes. I am a power user and I do get very nervous when I see just a few messages or even no messages in my inbox although there is no filter set (quicksearch box is blank!). So I assume for non-power users this is not something they'll take lightly.

> and additionally an edge case.

I was trying to make the point that it isn't. Unless actively using both ways of searching (quick filters and "search all message") is now considered an edge case.

> if it's similar to other bugs, it's not a filter causing the lack of messages
> in thread pane, is that the view is blown.  Sorry, not finding any examples.

I don't know what you mean, but this is definitely caused by a filter that doesn't get removed when it should. It's not related to a blown-up view like we currently have in the trunk after using "search all messages".

Let's not waste more time in discussing the severity of this. It's probably just a single line (perhaps 3) to fix it, as implied in comment #3.
Severity: minor → normal
I think it's a real bug, which I'd love to fix.  I won't have time to take it on for a while, so if someone else wants to take it on, please do.  I am happy to provide help about that code.
(In reply to comment #6)
> (In reply to comment #5)
> > if it's similar to other bugs, it's not a filter causing the lack of messages
> > in thread pane, is that the view is blown.  Sorry, not finding any examples.
> 
> I don't know what you mean, but this is definitely caused by a filter that
> doesn't get removed when it should. It's not related to a blown-up view like we
> currently have in the trunk after using "search all messages".

poor wording on my part. what I mean by not caused by filter, is there is not a filter active that is causing no messages to be listed.

FWIW, minor isn't a strict judgment of "worth" i.e. whether a bug should be fixed. If that were the case, no bugs would be minor.  Nor is it precisely a measure of inconvenience (though in practice that's why most bugs stay normal). minor == "minor loss of function, or other problem where easy workaround is present" https://bugzilla.mozilla.org/page.cgi?id=fields.html#importance  But I'm here to educate, not argue semantics.  

I use search all the search forms extensively, and haven't hit this bug iirc.  Hence, my suggestion of edge case.

keep up the good work :)
Thanks Wayne!

Wayne, would this be the right place too look at?
http://mxr.mozilla.org/comm-central/source/mail/base/content/search.xml#584
(In reply to my comment #9)
> http://mxr.mozilla.org/comm-central/source/mail/base/content/search.xml#584

Yea! That was the place to start patching from. Easy!
Here's a patch, tested working on my running install for the basic cases.

We're coming from changeMode method (see mxr reference above), which calls doSearch method. In doSearch method, we if-branch between 'global' and other search Modes/filters. In the if-'global'-branch, we need to remove the active traditional filter from search box AND gFolderDisplay.view of the original tab (because user changed to "search all messages" which opens in a new tab; in the original tab, searchbox now has "search all messages" which we blank out for the next search).

>                this.value = ''; // clear our value, to avoid persistence
This was already there, but not enough. I think we are were just clearing the value of the searchbox here, but apparently that wasn't enough to actually remove the filter from view. So I added this:

>                if (gFolderDisplay.view) {
>                  // if we started out from a gFolderDisplay.view,

Above I mean to check if we actually have a gFolderDisplay.view on the original tab (I saw that check further down in another section of this code, so I suppose this is needed to be on the safe side). Please check this.

>                  // remove existing (non-global) filter from view, to avoid
> persistence
>                  gFolderDisplay.view.search.userTerms = null;

This is the actual patch.

>                }
>                tabmail.openTab("glodaFacet", {

After clearing the filter on the original tab, continue with opening the gloda search in the new tab (unchanged).

All working nicely on my end.

Who should/is able to review this?
Attachment #407777 - Flags: review?
I can, but am traveling for a while.  I'd suggest standard8 or bienvenu.
Comment on attachment 407777 [details] [diff] [review]
Patch: remove viewfilter when we change to "global" searchmode

(In reply to David A's comment #11)
> I can, but am traveling for a while.  I'd suggest standard8 or bienvenu.

standard8, could you review this?
Attachment #407777 - Flags: review? → review?(bugzilla)
Attached patch With corrected name? (obsolete) — Splinter Review
Thomas, does this patch have the correct way of spelling your surname? I noticed your original patch had what was essentially an unreadable character (and missed the e?) so based on your email I tried correcting it.

If you look in other files, you'll see we should be able to support some characters at least: http://mxr.mozilla.org/comm-central/source/mail/base/content/mailWindowOverlay.xul#24

Anyway, the patch looks good, r+a=Standard8 with the surname fixed.
Attachment #407777 - Attachment is obsolete: true
Attachment #408353 - Flags: review+
Attachment #408353 - Flags: approval-thunderbird3+
Attachment #407777 - Flags: review?(bugzilla)
Assignee: nobody → bugzilla2007
(In reply to comment #13)
> Created an attachment (id=408353) [details]
> With corrected name?
> Thomas, does this patch have the correct way of spelling your surname?

Mark, thanks for being attentive to the umlaut issue with my name. The correct spelling is "Düllmann" as in the attached patch. Hope it works now.

This patch has been reviewed and approved per Mark's comment #13:
> Anyway, the patch looks good, r+a=Standard8 with the surname fixed.
So someone please set the approval-thunderbird3+ flag again.
Attachment #408353 - Attachment is obsolete: true
Attachment #408419 - Flags: review+
Attachment #408419 - Flags: approval-thunderbird3?
Comment on attachment 408419 [details] [diff] [review]
Patch [correct name, but still unreadable u-umlaut (ü)]: remove viewfilter when we change to "global" searchmode

Hmm, what looks like a perfect u-umlaut on my side always comes out unreadable after uploading the patch. What do I need to change format-wise (characterset etc.) when saving my patches so as to preserve the u-umlaut (ü)?
Attachment #408419 - Flags: review+
Attachment #408419 - Flags: approval-thunderbird3?
Attachment #408419 - Attachment description: Patch [with corrected name]: remove viewfilter when we change to "global" searchmode → Patch [correct name, but still unreadable u-umlaut (ü)]: remove viewfilter when we change to "global" searchmode
Try again to fix the u-umlaut issue.
I am now using utf-8 format for the patch, is that the right format?

Please approve as per previous comments.
Attachment #408419 - Attachment is obsolete: true
Attachment #408421 - Flags: review+
Attachment #408421 - Flags: approval-thunderbird3?
Attachment #408421 - Flags: approval-thunderbird3? → approval-thunderbird3+
Checked in:
http://hg.mozilla.org/comm-central/rev/887e7a38f130
http://hg.mozilla.org/releases/comm-1.9.1/rev/747357755053

Thanks Thomas (note the file wasn't quite right, because of the utf-8 char at the beginning of it, but I made it work anyway).
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: wanted-thunderbird3?
Flags: blocking-thunderbird3?
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.0rc1
You need to log in before you can comment on or make changes to this bug.