435881 - Move mailnews/compose to frozen linkage

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[Mark Banner (:standard8)]

Another step in the move to frozen linkage. Patches coming soon.

Comment 1

[Mark Banner (:standard8)]

Attachment: [checked in] Simple Changes

These are simple changes (mainly includes) to files that don't need (m)any other changes.

Comment 2

[David :Bienvenu]

Attachment: [checked in]fix callers of nsMsgCompFields::SplitRecipients

I think this bug introduced a regression - nsMsgCompFields::SplitRecipients crashes if a null recipient str is passed in, though first there is an assertion to fix the callers. This patch attempts to do that. The crash is in the nsDependentString(recipientsStr) piece of code.

STR - have an empty To: address, and switch the from identity a couple times...

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 322944 [details] [diff] [review]
[checked in]fix callers of nsMsgCompFields::SplitRecipients

I'm giving a provisional r+. I think we should fix the SplitRecipients as well to do the right thing as it is an idl method and extensions can call it, so we shouldn't crash if they do something wrong.

Comment 4

[David :Bienvenu]

I agree - that was what I did at first, but then I read the assertion and went and fixed the callers. I'll attach my patch to fix SplitRecipients as well.

Comment 5

[David :Bienvenu]

Attachment: bulletproof SplitRecipients

trivial check

Comment 6

[[e-mail]]

Comment on attachment 322661 [details] [diff] [review]
[checked in] Simple Changes

>@@ -668,7 +668,7 @@ nsresult nsMsgCompFields::SplitRecipients
>-  CopyUTF16toUTF8(recipients, recipientsStr);
>+  CopyUTF16toUTF8(nsDependentString(recipients), recipientsStr);
>   rv = parser->ParseHeaderAddresses("UTF-8", recipientsStr.get(), &names,
>                                     &addresses, &numAddresses);
Why not make recipients an AString or AUTF8String?

Comment 8

[David :Bienvenu]

> Why not make recipients an AString or AUTF8String?
> 

sorry, didn't realize that was for me. I can do that, AString, I think, would be a more straightforward change

Comment 9

[[e-mail]]

Comment on attachment 322949 [details] [diff] [review]
bulletproof SplitRecipients

As per comment #6.

Comment 10

[David :Bienvenu]

Attachment: [checked in]patch addressing comments

this also contains an unrelated change in my tree, to use const with an nsACString that someone suggested in a drive by comment, iirc.

Comment 11

[Mark Banner (:standard8)]

Although the patch just checked in fixes the crash it doesn't fix the bug, we've got more work to do on this yet, reopening.

Comment 12

[Robert Kaiser]

Serge, why are you changing around dependencies here like this would have been fixed when it hasn't? That doesn't block building with a libxul variant as we already do this, but we're still not building with frozen linkage and it's not at all clear that doing that works in any way, AFAIK.

Comment 13

[Serge Gautherie (:sgautherie)]

(In reply to comment #12)

I did that because bug 377319 is 'fixed' and bug 432162 (which is block by bug 377319) is (still) 'open'.
But maybe some bug statuses are wrong (now), iiuc your comment :-|

Comment 14

[[e-mail]]

I believe this bug should be reresolved, unless anyone else knows different?

Comment 15

[Wayne Mery (:wsmwk)]

(In reply to [e-mail] from comment #14)
> I believe this bug should be reresolved, unless anyone else knows different?

Fixed according to Neil