478113 - nsMsgDBView::GetCellValue leaks strings

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[David :Bienvenu]

Attachment: proposed fix

nsMsgDBView::GetCellValue .Assigns the result of nsMsgDBView::GetString() to nsCStrings - afaict, this is a memory leak because GetString returns an allocated buffer, and we should be using .Adopt. GetCellValue is used by screen readers, I believe.

Comment 1

[[e-mail]]

Comment on attachment 361870 [details] [diff] [review]
proposed fix

>+            aValue.Adopt(GetString (isContainerOpen ?
>+                                    NS_LITERAL_STRING("messageExpanded").get()
>+                                    : NS_LITERAL_STRING("messageCollapsed").get()));
Nit: personally I prefer the : at the end of the previous line.

>+      aValue.Adopt(GetString ((flags & nsMsgMessageFlags::Read) ?
>+                    EmptyString().get() : NS_LITERAL_STRING("messageUnread").get()));
I wonder why we bother calling GetString on an empty string. I was thinking along the lines of
if (flags & nsMsgMessageFlags::Read)
  aValue.Truncate();
else
  aValue.Adopt(GetString(NS_LITERAL_STRING("messageUnread").get()));

Comment 2

[David :Bienvenu]

I've made this match GetCellText, i.e., truncate aValue before the switch, and then just do .Adopts if the flag condition is right. Fix checked in.