525948 - Quick Search option "Subject, To, or Cc filter" inconsistent after restart

Status: RESOLVED FIXED

(Thunderbird :: Search, defect)

Description

[Daniel Kabs, reporting bugs since 2002]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.4) Gecko/20091016 Firefox/3.5.4 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.5pre) Gecko/20091102 Shredder/3.0pre

Quick Search option  "Subject, To, or Cc filter" is not persistent over restart. 
After restart, selected option does not match displayed text in Quick Search widget.

Reproducible: Always

Steps to Reproduce:
1. Select "Subject, To or CC filter" in quick search drop down.
2. Quick Search widget correctly displays "Subject, To or CC filter"
3. Restart Thunderbird
Actual Results:  
Quick search box shows "Subject or From filter"
If you click on the drop down ("magnifying lens"), "Subject, To or CC filter" option is still selected

-> Inconsistent

Expected Results:  
Quick search box should show "Subject, To or CC filter"

It is not a dupe of bug #477656 as that bug is about the colour of the text (greyed out vs. not greyed out).

Comment 1

[Daniel Kabs, reporting bugs since 2002]

Attachment: Screenshot: inconsistency between text and option

Comment 2

[Thomas D. (:thomas8)]

confirming on

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

wow, that's a very old problem reoccuring... (regression, works in TB2)

Actual behaviour:

- Quicksearchbox's value is "Subject or from filter" (greyed out)
- Real Active filter is "Subject or from filter"
- Selected option in the dropdown is "Subject, To, or CC filter"
(side note: that filter word is really superfluous!)

Expected behaviour:
- we should persist the last filter option chosen by user after restart (as we did in TB2)
- selected filter option in box.value, real view filter, and selected option in dropdown should all be the same.

Comment 3

[David Ascher (:davida)]

Sid0's looking at this code, he may have a fix for this handy.

Comment 4

[Thomas D. (:thomas8)]

culprit code seems to live here:
http://mxr.mozilla.org/comm-central/source/mail/base/content/search.xml#278
http://mxr.mozilla.org/comm-central/source/mail/base/content/search.xml#312

this.inputSearch.searchMode = this.inputSearch.glodaEnabled ?
"global" :
quickSearchModes[QuickSearchConstants.kQuickSearchFromOrSubject].value.toString();

That's where we switch to searchMode kQuickSearchFromOrSubject if the current search is not a global one. Anyone knows where we (used to) save the searchMode for persistence while TB is closed? And where we (used to) read it back in?

We should really persist the last user-selected filter. But even a default filter, if anything, should no longer be "Subject or From", but "Subject, From, or Recipient".

Comment 5

[David Ascher (:davida)]

At least at one point we did persist the searchMode in the tabstate.  That code should get overriden in the tabstate restoration process, but clearly isn't always.  I admit that I get lost in the tab management codepaths.

Comment 6

[Rain [:rain1] (inactive)]

I think this is fixed by bug 518753.

Comment 7

[Rain [:rain1] (inactive)]

Whoops, turns out it isn't.

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

This seems like it would be fairly annoying.  I suspect this wouldn't block, but I'm interested in clarkbw's opinion, so adding him to the CC and setting blocking?.  In any case, I'd certainly consider approving a patch if a suitably low-risk one were to appear.

Comment 9

[Rain [:rain1] (inactive)]

The problem here is that despite what it says about when it should be called, <http://mxr.mozilla.org/comm-central/source/mail/base/content/search.xml#614> is called every time a folder change happens: <http://mxr.mozilla.org/comm-central/source/mail/base/content/folderDisplay.js#906>. At startup, this happens right after tab restore sets up the correct search mode, so one changes but the other doesn't.

Comment 10

[Rain [:rain1] (inactive)]

Attachment: patch

This seems to fix it...

Comment 11

[David :Bienvenu]

Comment on attachment 410923 [details] [diff] [review]
patch

fix works, and mozmill test passes.

Comment 12

[Rain [:rain1] (inactive)]

Attachment: patch v2

Oops, forgot to import window-helpers in my haste.

Comment 13

[David :Bienvenu]

Comment on attachment 410926 [details] [diff] [review]
patch v2

test 2 succeeds now...

Comment 14

[Rain [:rain1] (inactive)]

Comment on attachment 410926 [details] [diff] [review]
patch v2

a=bienvenu over IRC

Comment 15

[Rain [:rain1] (inactive)]

Attachment: tests, not landed yet

Since mozmill is a bit wonky right now, I haven't landed the tests.

http://hg.mozilla.org/comm-central/rev/ecfe5ecbc2cc
http://hg.mozilla.org/releases/comm-1.9.1/rev/ebce9866334b

I guess this bug should be kept open until the tests are landed.

Comment 16

[Dan Mosedale (:dmosedale, :dmose)]

Marking as blocking 3.0, but not targetting at RC1, since we can land tests after code freeze.

Comment 17

[Rain [:rain1] (inactive)]

Tests landed after go-ahead from Standard8.

http://hg.mozilla.org/comm-central/rev/ec712b795a98
http://hg.mozilla.org/releases/comm-1.9.1/rev/7f48ee2226d6