If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Should have a category for converting between Gecko and Cocoa strings

NEW
Unassigned

Status

()

Core
Widget: Cocoa
11 years ago
a month ago

People

(Reporter: hsivonen, Unassigned)

Tracking

Trunk
PowerPC
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

11 years ago
Currently, embedding/browser/cocoa/src/NSString+Utils.h is available to Camino, but is not in the right place for use in the Cocoa widget code. 

For the Cocoa widget code, it would be nice to have an NSString category that allowed NSString to be initialized with various Gecko string flavors and that allowed NSString to be converted to various Gecko string flavors.

Comment 1

11 years ago
I agree.
(Reporter)

Comment 2

11 years ago
To self.
Assignee: joshmoz → hsivonen
(Reporter)

Updated

11 years ago
Status: NEW → ASSIGNED
(Reporter)

Comment 3

11 years ago
Where should the conversion category be placed in the tree? Should the methods that are not strictly about string conversion be left in embedding/browser/cocoa/src/NSString+Utils.h?

Comment 4

11 years ago
I suggest xpcom/glue/nsMacString.{h,mm} other utility classes/functions can be moved if mento thinks appropriate.

Comment 5

11 years ago
See also bug 334670, which is about the Core Foundation type (which is the C struct that the objc-only NSString type uses as its core).
(Reporter)

Comment 6

11 years ago
Created attachment 235726 [details]
Unpolished header
(Reporter)

Comment 7

11 years ago
Created attachment 235727 [details]
Unpolished implementation
(Reporter)

Comment 8

11 years ago
Created attachment 239020 [details] [diff] [review]
Diff for existing files

(Will upload new files separately. cvs diff -uN isn't working for me.)
(Reporter)

Comment 9

11 years ago
Created attachment 239022 [details]
mozilla/xpcom/string/public/NSString+Moz.h
Attachment #235726 - Attachment is obsolete: true
(Reporter)

Comment 10

11 years ago
Created attachment 239025 [details]
mozilla/xpcom/string/src/NSString+Moz.mm

hwaara, are these changes adequate for your purposes? Would you like to take the bug to add whatever changes you need?
Attachment #235727 - Attachment is obsolete: true
(Reporter)

Updated

11 years ago
Blocks: 332912

Comment 11

11 years ago
Comment on attachment 239022 [details]
mozilla/xpcom/string/public/NSString+Moz.h

I don't think I need any other changes, so I'd be happy to help out reviewing this, if you plan to drive the patch.

>+ (id)ellipsisString;

This is not needed; IIRC 10.3 offers ways to do this. The reason that there was a category method, was that Camino used to support 10.1/10.2

>+ (id)stringWithPRUnichars:(const PRUnichar*)inString;
>+ (id)stringWith_nsAString:(const nsAString&)inString;
>+ (id)stringWithUTF8_nsACString:(const nsACString&)inString;

I think this should be named simply stringWith_nsACString to match what the rest of XPCOM string classes are named.

>+ (id)stringWith_nsISupportsString:(nsISupportsString*)inString;

I think nsISupportsString is deprecated, and thus should not be endorsed by new APIs. That goes for all the nsISupports helper methods.

>- (NSString *)stringByRemovingCharactersInSet:(NSCharacterSet*)characterSet;
>- (NSString *)stringByReplacingCharactersInSet:(NSCharacterSet*)characterSet withString:(NSString*)string;
>- (NSString *)stringByTruncatingTo:(unsigned int)maxCharacters at:(ETruncationType)truncationType;

Are these strictly needed? There should be ways to do this with the existing NSString APIs, and they're not really Mozilla-specific.

>// allocate a new unicode buffer with the contents of the current string. Caller
>// is responsible for freeing the buffer.
>- (PRUnichar*)createNewUnicodeBuffer;

Also not sure what this has to do with NSString at all.

Comment 12

11 years ago
Comment on attachment 239020 [details] [diff] [review]
Diff for existing files

>Index: mozilla/configure.in
>===================================================================
>RCS file: /cvsroot/mozilla/configure.in,v
>retrieving revision 1.1704
>diff -u -r1.1704 configure.in
>--- mozilla/configure.in	31 Jul 2006 18:11:36 -0000	1.1704
>+++ mozilla/configure.in	18 Sep 2006 14:26:30 -0000
>@@ -4537,7 +4537,7 @@
> 	AC_DEFINE(MOZ_WIDGET_PHOTON)
>     ;;
> mac|cocoa)
>-    TK_LIBS='-framework Carbon'
>+    TK_LIBS='-framework Carbon -framework Cocoa'

I bet linking to -framework Foundation is enough.

Comment 13

11 years ago
Comment on attachment 239025 [details]
mozilla/xpcom/string/src/NSString+Moz.mm

The nsA[C]String utilities can be simplified.

Have a look at http://landfill.mozilla.org/mxr-test/mozilla/source/camino/src/extensions/NSString+Utils.mm

(Note that all files in embedding/cocoa are >3 years out out of sync with their camino counterparts, so that code should really not be copied.)
(Reporter)

Comment 14

11 years ago
The use case I had for this code was not checked in. Should this be WONTFIXed?
Assignee: hsivonen → joshmoz
Status: ASSIGNED → NEW
Someone was asking about (wanting) string conversion stuff the other day, so presumably there's still some need for either this bug or Håkan's bug 334670....
This and bug 334670 are probably dupes of each other. CFString and NSString are toll-free bridged. You can just cast the NSString to a CFString, or vice versa.

Updated

9 years ago
Assignee: joshmoz → nobody
You need to log in before you can comment on or make changes to this bug.