703175 - nsImportTextAddress::DetermineDelim seems wrong

Status: RESOLVED FIXED

(MailNews Core :: Import, defect)

Description

[Hiroyuki Ikezoe (:hiro)]

As you can see from the fact that some variable names contain 'line' word in nsImportTextAddress::DeteminDelim, The function aims to count tab and comma line by line but actually it fails.

First, NULL-terminated position is always kTextAddressBufferSz - 1, it should be read bytes from stream, in this case it's 'left' variable. (The variable name is not good at all, by the way)

Second, nsIInputStream is used instead of nsILineInputStream.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Attachment: Fix

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Attachment: A test

Comment 3

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #2)
> Created attachment 575079 [details] [diff] [review] [diff] [details] [review]
> A test

This test depend on importHelper.js in bug 700920.

Comment 4

[David :Bienvenu]

Comment on attachment 575078 [details] [diff] [review]
Fix

The patch didn't apply cleanly, but when I fixed it, it passed the tests...

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Attachment: De-rotted patch

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Attachment: De-rotted test

Comment 7

[Ludovic Hirlimann [:Usul]]

So should these be checkin-needed ?

Comment 8

[Hiroyuki Ikezoe (:hiro)]

(In reply to Ludovic Hirlimann [:Usul] from comment #7)
> So should these be checkin-needed ?

Yes. Thanks for the notice. I do sometimes forget the flag.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/c3444a3d3436
https://hg.mozilla.org/comm-central/rev/206610bffbe7