Last Comment Bug 363238 - saved searches fail for searches on x-headers
: saved searches fail for searches on x-headers
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Search (show other bugs)
: Trunk
: All All
: -- normal with 2 votes (vote)
: Thunderbird 19.0
Assigned To: Kent James (:rkent)
:
Mentors:
Depends on: 184490 810637
Blocks: qfasfailtracker
  Show dependency treegraph
 
Reported: 2006-12-08 18:32 PST by Asa Dotzler [:asa]
Modified: 2012-12-20 09:25 PST (History)
8 users (show)
rkent: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Always add mailnews.customHeaders to db (19.71 KB, patch)
2011-04-29 14:56 PDT, Kent James (:rkent)
mozilla: review+
Details | Diff | Review
Unbitrotted and modernized patch (18.44 KB, patch)
2012-10-17 09:47 PDT, Kent James (:rkent)
irving: review+
Details | Diff | Review

Description Asa Dotzler [:asa] 2006-12-08 18:32:33 PST
If I add a custom x-header to the search criteria (first dropdown in the advanced search dialog) and then search for an x-header in the advanced search dialog, it works.  If, however, I save that search as a search folder, it fails. 

Tested with IMAP, both searching the server and not. Tested on latest nightly build.
Comment 1 Karsten Düsterloh 2006-12-09 07:27:06 PST
This is technically a dupe of bug 205501.
Comment 2 David :Bienvenu 2006-12-09 08:08:17 PST
we may be able to use the work in bug 356860 to add the headers required for custom views to "mailnews.customDBHeaders", but we'd also need to change the local db search code to know how to find the custom headers from the msg hdr (basically, call getStringProperty with the lower case version of the header name)
Comment 3 Kent James (:rkent) 2009-07-28 13:14:32 PDT
I'm going to address some of the issues from bug 184490 here.
Comment 4 Kent James (:rkent) 2011-04-29 14:56:37 PDT
Created attachment 529213 [details] [diff] [review]
Always add mailnews.customHeaders to db

I have mixed feelings about this patch. I am really trying to address the complaints in bug 184490, but using this current bug as a partial solution.

The downsides of this patch are:

1) It only affects messages that are downloaded after a custom header is added,
2) The size of the message db for every message is increased, as any custom header that is defined will now be added to the database for all folders.
3) It makes a lot of search code for custom headers used only rarely, as the custom headers will get added to the database.

But it largely solves the problem. AFAICT the problem only occurs for folders that do not have offline storage enabled, hence the disabling of offline storage in the test.

What do you think?
Comment 5 David :Bienvenu 2011-05-02 20:54:54 PDT
I'm probably OK with this because it only affects users who define filters that use custom headers...
Comment 6 David :Bienvenu 2011-05-09 16:27:45 PDT
Comment on attachment 529213 [details] [diff] [review]
Always add mailnews.customHeaders to db

copyright on test wants to be 2011, I believe.

double ; here
nsCString customHeaders;;

don't need LL macros here:

+        if (LL_IS_ZERO(m_receivedTime))
Comment 7 Wayne Mery (:wsmwk, NI for questions) 2012-01-23 08:40:18 PST
rkent, are you game to finish this?
Comment 8 Kent James (:rkent) 2012-10-12 08:59:26 PDT
Sorry, somehow this fell off of my radar screen.

I suppose I should fix the nits and ask for a re-review, since it has been over a year.
Comment 9 Kent James (:rkent) 2012-10-17 09:47:19 PDT
Created attachment 672361 [details] [diff] [review]
Unbitrotted and modernized patch

I took the previous patch (which had r+ from bienvenu, but never got landed), and brought it up to date. But since it has been over a year, I figure I should get it re-reviewed.

Feel free Irving to just say land it with the existing review.
Comment 10 :Irving Reid (No longer working on Firefox) 2012-10-21 19:42:30 PDT
Comment on attachment 672361 [details] [diff] [review]
Unbitrotted and modernized patch

Review of attachment 672361 [details] [diff] [review]:
-----------------------------------------------------------------

r=bienvenu,irving with the nits fixed.

::: mailnews/imap/test/unit/test_imapSearch.js
@@ +6,5 @@
> + * Derived from a combination of test_imapPump.js and test_search.js
> + * Original author: Kent James <kent@caspia.com>
> + */
> +
> +// async support 

Trailing space

@@ +249,5 @@
> +                          gIMAPMailbox.uidnext++, []));
> +  gIMAPInbox.updateFolderWithListener(null, asyncUrlListener);
> +  yield false;
> +
> +  do_check_eq(1, gIMAPInbox.getTotalMessages(false));  

Trailing space
Comment 11 Kent James (:rkent) 2012-10-22 11:23:33 PDT
Checked in https://hg.mozilla.org/comm-central/rev/5cca127781c1

Note You need to log in before you can comment on or make changes to this bug.