centralize stream handling in search/filters

RESOLVED FIXED in Thunderbird 8.0

Status

RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Bienvenu, Assigned: Bienvenu)

Tracking

Trunk
Thunderbird 8.0
x86
Windows 7

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

8 years ago
Created attachment 543322 [details] [diff] [review]
proposed fix

This is fallout from the pluggable store work, but can be extracted out to reduce the size of that patch. The idea is to centralize the input stream handling in nsMsgSearchTerm, and get rid of unneeded params. (with pluggable stores, we'll get an input stream for the msg hdr using the pluggable store interface). This passes all unit tests (for me) so if it does break something, we'll probably need more unit tests.
Attachment #543322 - Flags: review?(kent)
(Assignee)

Comment 1

8 years ago
Comment on attachment 543322 [details] [diff] [review]
proposed fix

I guess since this changes an interface, it needs sr.
Attachment #543322 - Flags: superreview?(neil)

Updated

8 years ago
Attachment #543322 - Flags: superreview?(neil) → superreview+

Comment 2

8 years ago
Comment on attachment 543322 [details] [diff] [review]
proposed fix

r=me with one small nit:

+  nsCOMPtr<nsISeekableStream> seekableStream(do_QueryInterface(m_inputStream));
+  rv = seekableStream->Seek(PR_SEEK_SET, offset);

You need a test to confirm non-null seekableStream. Yes I understand that with the current design, that is always true, but it would be very easy in the future to add the ability to set the inputStream, assuming that you start that stream from the beginning, and then this is a hidden trap. I would be happy if you just did not do the seek in that case, perhaps with a warning for now.
Attachment #543322 - Flags: review?(kent) → review+
(Assignee)

Comment 3

8 years ago
fixed on trunk, tfv tb 8, but there's no version for that yet.
http://hg.mozilla.org/comm-central/rev/cf80c93a7d8d
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(In reply to comment #3)
> fixed on trunk, tfv tb 8, but there's no version for that yet.
> http://hg.mozilla.org/comm-central/rev/cf80c93a7d8d

Did you mean target milestone?
Target Milestone: --- → Thunderbird 8.0
(Assignee)

Comment 5

8 years ago
yeah, target milestone. Perhaps I needed to refresh the bug...thx for setting the milestone.
You need to log in before you can comment on or make changes to this bug.