244325 - Add a modifier box to the quick search bar

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Scott MacGregor]

mail.app has a great feature where their quick search bar has a finder box
inside of it. Clicking the finder box drops down a menu list of options for what
kind of search you want your quick search to be. 

You can make it:
In Message (i.e. body search)
Subject
Sender

When you don't have any text in the quick search bar at all. They have faded
gray text which shows what type of search the quick search bar is configured to do.

This could be a really neat way to make quick search more customizeable and
powerful.

Comment 1

[Scott MacGregor]

Will temporarily put in a triage bucket for 1.0 consideration. But I doubt we
have the time to do it unless a volunteer steps up. 

Comment 2

[Scott MacGregor]

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

Comment 3

[Mike Cowperthwaite]

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

Comment 4

[Scott MacGregor]

bonus points if the Find In message mode for this box highlights the search
terms in the message body.

Comment 5

[Scott MacGregor]

Attachment: screen shot of work in progress

Comment 6

[Scott MacGregor]

Attachment: work in progress patch

still needs polish.

I'm also having problems making the highlighting work when you load a new
message since right now we don't have a notification that fires when layout is
actually done laying out the message. The only notification we have is the one
from libmime saying we are done processing the headers and that's too early for
highlighting to commence.

Comment 7

[Scott MacGregor]

Attachment: broadcast a notification to the UI when a mesage is finished being rendered

Comment 8

[Scott MacGregor]

Comment on attachment 151465 [details] [diff] [review]
broadcast a notification to the UI when a mesage is finished being rendered

David, this is the change we were talking earlier about.

I renamed the current OnEndMsgDownload notification to a more appropriate name,
onEndMsgHeaders and I left alone all the work we do in the front end when this
notification gets called by libmime. 

The most dangerous part of this patch is the change to nsImapProtocol as it
could introduce leaks. It assumes that nsImapProtocol::ReleaseUrlState	gets
called for every url we run through the imap protocol. I still need to verify
that this is true for fetching cached parts   otherwise the cycle won't be
getting broken. 

One more thing. Am i correct in assuming that  nsImapMsgFetchPeek and
nsImapMsgFetch are the only two valid imap actions when fetching a message for
display purposes?

Comment 9

[David :Bienvenu]

+    case nsIMsgMailNewsUrl::eDisplay:
+      *isType = (m_imapAction == nsIImapUrl::nsImapMsgFetch || 
+        m_imapAction == nsIImapUrl::nsImapMsgFetchPeek);

It's true that these are the only types when used to fetch a message for
display; however, actions of these types don't mean we *are* displaying a
message, so as long as we're not assuming that, we should be OK.

removing the clearing of the url is scary, but it might be OK.  Can you run with
it for a while to be sure nothing strange starts happening? I'll go put my sr= ...

Comment 10

[Scott MacGregor]

David, I just spent some time in the debugger convincing myself that the channel
and the url both still get released when we are fetching from the memory cache
instead of going through the imap protocol. They aren't released until you click
on the next message when the document lets go of some state (including the
document url), but that's the way things used to work before this change.

Comment 11

[David :Bienvenu]

that's good. The other thing I was worried about was that the mock channel and
the protocol object might be holding onto different actual urls for longer than
they used to, but that is very unlikely to matter...since before the mock
channel would have a null url, and now it'll just point to a previously run url,
and we mostly care about the url the protocol object has.

Comment 12

[Scott MacGregor]

Attachment: updated patch for the search box

Improved highlighting support when switching between messages
A new search criteria for Message Body searches
Some style improvements
When doing a "search message bodies" quick search we also highlight the
resulting terms as if you were in "Find In Message" mode.

Comment 13

[Scott MacGregor]

Adding the message body quick search mode is a great featue for POP3/Local
Folder and offline IMAP users. However it doesn't work at all for non-offline
IMAP (and news) users because we don't have any message bodies to search. We
need to figure out a graceful way out of this situation. Should we check the
validity of the body search and silently fall back to a subject or sender search
if we are news or online imap just before we do the search?

Oh one more new thing in the last patch. The search criteria is now persisted
across sessions.

Comment 14

[Scott MacGregor]

Attachment: updated patch includes a better mechanism for persistence

This new patch contains:
1) a better way for remembering the state of the search criteria drop down
2) optimizing when we try to remove highlighting from a message (such as when
the criteria changes)

Comment 15

[Scott MacGregor]

Attachment: CSS changes for OS X

Comment 16

[Scott MacGregor]

These patches have been checked into the branch and the trunk.

Still unresolved: handling the search message bodies issue for imap and news.

Comment 17

[Stephen Walker]

#search-button needs a name change to fix the regression noted in bug 249127.

Comment 18

[Tom Graham]

Bug # 249297 is linked in with this. 

Comment 19

[Scott MacGregor]

FYI, this bug also caused the following regression which I just fixed:

Bug #249337

Comment 20

[Scott MacGregor]

Attachment: change the id of the search box to avoid ID collisions

Comment 21

[Scott MacGregor]

re-closing. I checked in the ID collision patch. 

Comment 22

[Stephen Walker]

Attachment: Fix the case mismatch idl warning

nsIMimeMiscStatus.idl:45: Case mismatch between `nsIMsgMailNewsUrl'
nsIMimeMiscStatus.idl:80: and `nsImsgMailNewsUrl'
nsIMimeMiscStatus.idl:80: (Identifiers should be case-consistent after initial
declaration)

Comment 23

[Stephen Walker]

Comment on attachment 152546 [details] [diff] [review]
Fix the case mismatch idl warning

/me looks at the lack of approval flags

Comment 24

[David :Bienvenu]

thunderbird bugs don't get approval flags...

Comment 25

[Scott MacGregor]

I checked in Stephen's patch for the case mismatch. branch and trunk.

Comment 26

[Tom-Erik Duestad]

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

Comment 27

[Tom-Erik Duestad]

Attachment: Proposal UI mock up

A mock up of an alternative UI, this way it very easy to see what your search
criteria were, after the search has completed, and it's easier to see that you
can choose what's searched.

Should be easy to change if others like it :-) This was my idea for layout in
bug 230384.