279405 - Change nsIAbCard.idl from wstring to AString to save code-size

Status: RESOLVED FIXED

(MailNews Core :: Address Book, defect)

Description

[Mikael Parknert]

A proposal to change all wstring to AString in nsIAbCard.idl. 

This removes lots of conversions and getter_Copies and saves 
about 10k on my optimized Suite-build

Comment 1

[Mikael Parknert]

Attachment: proposed changes

Comment 2

[David :Bienvenu]

Comment on attachment 172120 [details] [diff] [review]
proposed changes

this will break the palm sync code, won't it? mailnews/extensions/palmsync. I'm
worried there might be other extensions out there that this will break as well
but at minimum, we'd need to fix the palm sync code.

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

Once you sort out the palmsync question, could you generate a patch with "diff
-w" for reviewing purposes?  That'd make it easier to eyeball the changes and be
sure that nothing is inadvertantly missed.

Comment 4

[Mark Banner (:standard8)]

I know it's not the original problem of this bug, but I noticed this and the
changes you were going to make which will probably duplicate(ish) a set of
changes I need to make.

David & Dan both mention about the palmsync extension - one of the palmsync
classes (nsAbIPCCard) inherits from nsAbMDBCardProperty (see link below), which
I believe is wrong.

http://lxr.mozilla.org/seamonkey/source/mailnews/extensions/palmsync/src/nsAbIPCCard.h#56

From what I understand, if possible, this should really inherit from nsIAbCard
as that is the proper interface. I was going to change this in Bug 131849 as
part of it's subsequent tidy up from the build bustages, such that address book
wouldn't need to export the nsAbCardPropery.h and nsAbMDBCardProperty.h internal
headers.

If I am right,then these changes are obviously going to affect one another
perhaps we could co-ordinate them?



David, can you confirm whether or not I am right about the inheritance structure
for the nsAbIPCCard class and whether you agree that the changes should be made,
or do you know of a possible problem?

Comment 5

[Mark Banner (:standard8)]

Mikael isn't able to do patches for us currently, moving to core address book & reassigning to default owner.

Comment 6

[Mark Banner (:standard8)]

Attachment: Updated patch

I've taken Mikael's original patch and updated it to latest trunk and to cover the windows side of things as well (though I've only compiled on linux).

I think this would nice to get in now while we're doing lots of changes to interfaces etc on trunk, and it should cut down on the amount of string copying we're doing...

Comment 7

[Mark Banner (:standard8)]

Attachment: Updated patch (diff -w)

Opps, almost forgot, here's the -w version as there's some extra whitespace fixups as well.

Comment 8

[David :Bienvenu]

Comment on attachment 261847 [details] [diff] [review]
Updated patch

thx, Mark, this looks good. Does it affect palmsync, I wonder?

Comment 9

[Mark Banner (:standard8)]

(In reply to comment #8)
> (From update of attachment 261847 [details] [diff] [review])
> thx, Mark, this looks good. Does it affect palmsync, I wonder?

David:

Index: mailnews/extensions/palmsync/src/nsAbIPCCard.cpp
===================================================================
RCS file: /cvsroot/mozilla/mailnews/extensions/palmsync/src/nsAbIPCCard.cpp,v

It's in there already. I think those changes are the only ones that are required.

Comment 10

[Scott MacGregor]

Comment on attachment 261847 [details] [diff] [review]
Updated patch

Nice change.

Comment 11

[Mark Banner (:standard8)]

Patch checked in -> fixed.

Comment 12

[Mark Banner (:standard8)]

(oh and depending on which SeaMonkey build you looked at it saved 8480 bytes or 10648 bytes) :-)