Closed Bug 244325 Opened 20 years ago Closed 20 years ago

Add a modifier box to the quick search bar

Categories

(Thunderbird :: General, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird0.8

People

(Reporter: mscott, Assigned: mscott)

References

Details

Attachments

(7 files, 2 obsolete files)

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.
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. 
Status: NEW → ASSIGNED
Target Milestone: --- → Thunderbird0.8
Blocks: 244326
*** Bug 182073 has been marked as a duplicate of this bug. ***
*** Bug 129528 has been marked as a duplicate of this bug. ***
bonus points if the Find In message mode for this box highlights the search
terms in the message body.
Attached patch work in progress patch (obsolete) — Splinter Review
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 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?
Attachment #151465 - Flags: superreview?(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= ...
Attachment #151465 - Flags: superreview?(bienvenu) → superreview+
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.

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.
Attached patch updated patch for the search box (obsolete) — Splinter Review
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.
Attachment #151375 - Attachment is obsolete: true
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.
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)
Attachment #151550 - Attachment is obsolete: true
These patches have been checked into the branch and the trunk.

Still unresolved: handling the search message bodies issue for imap and news.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Blocks: 249127
#search-button needs a name change to fix the regression noted in bug 249127.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Bug # 249297 is linked in with this. 
FYI, this bug also caused the following regression which I just fixed:

Bug #249337
re-closing. I checked in the ID collision patch. 
Status: REOPENED → RESOLVED
Closed: 20 years ago20 years ago
Resolution: --- → FIXED
nsIMimeMiscStatus.idl:45: Case mismatch between `nsIMsgMailNewsUrl'
nsIMimeMiscStatus.idl:80: and `nsImsgMailNewsUrl'
nsIMimeMiscStatus.idl:80: (Identifiers should be case-consistent after initial
declaration)
Comment on attachment 152546 [details] [diff] [review]
Fix the case mismatch idl warning

/me looks at the lack of approval flags
Attachment #152546 - Flags: superreview?(bienvenu)
Attachment #152546 - Flags: review?(bienvenu)
Attachment #152546 - Flags: superreview?(bienvenu)
Attachment #152546 - Flags: superreview+
Attachment #152546 - Flags: review?(bienvenu)
Attachment #152546 - Flags: review+
thunderbird bugs don't get approval flags...
I checked in Stephen's patch for the case mismatch. branch and trunk.
*** Bug 230384 has been marked as a duplicate of this bug. ***
Attached image 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.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: