346679 - NormalizationTest.cpp doesn't need MOZILLA_INTERNAL_API

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Benjamin Smedberg]

The only reason I can see it currently uses MOZILLA_INTERNAL_API is because IS_SURROGATE and friends are declared in nsCharTraits.h instead of somewhere more generic. I've moved them to nscore.h

Comment 1

[Benjamin Smedberg]

Attachment: Fix NormalizationTest.cpp

Ignore the #if 0 change in nscore.h, that's from the firefox --enable-libxul testing.

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

since these are sort of string-specific, wouldn't something like nsReadableUtils.h be a better place?

Comment 3

[Benjamin Smedberg]

nsReadableUtils.h is internal-linkage only... we don't really have a good header for string util macros that are available in both the external and internal linkage, except for nsStringGlue.h, and I didn't want to do a tree-wide search for consumers of these macros.

Comment 4

[Darin Fisher]

Comment on attachment 231412 [details] [diff] [review]
Fix NormalizationTest.cpp

I really don't think we want nscore.h to define all of those macros that do not start with the NS_ namespace.  That doesn't play nicely at all with folks who are using our SDK.

Comment 5

[Simon Montagu :smontagu]

Comment on attachment 231412 [details] [diff] [review]
Fix NormalizationTest.cpp

By the way, it would make my life easier if you could do

#ifdef DEBUG_smontagu
#define MOZILLA_INTERNAL_API
#endif
...
#ifdef DEBUG_smontagu
#include "nsString.h"
#else
#include "nsStringAPI.h"
#endif

Why does CharAt() require MOZILLA_INTERNAL_API anyway?

Comment 6

[Benjamin Smedberg]

smontagu, that's a bad idea; internal linkage for code that doesn't actually live in libxul is going away quickly. I discovered this problem because xulrunner build with tests enabled wouldn't link.

What CharAt() signature do you need? The frozen API has

http://lxr.mozilla.org/mozilla/source/xpcom/glue/nsStringAPI.h#71 and an nsACString equivalent.

Comment 7

[Benjamin Smedberg]

> I really don't think we want nscore.h to define all of those macros that do not
> start with the NS_ namespace.

What do you suggest? 

1) put the existing macros in nsStringGlue.h?
2) Rename the macros with NS_ prefixes (and put them in nscore.h)?
3) Create a new header for them?

Comment 8

[Simon Montagu :smontagu]

(In reply to comment #6)
> What CharAt() signature do you need? The frozen API has
> 
> http://lxr.mozilla.org/mozilla/source/xpcom/glue/nsStringAPI.h#71 and an
> nsACString equivalent.

Yeah, well, you only checked that in yesterday, didn't you?

Is there any reason why we shouldn't just #include nsCharTraits.h explicitly?

Comment 9

[Benjamin Smedberg]

> Yeah, well, you only checked that in yesterday, didn't you?

I guess I did ;-)

> Is there any reason why we shouldn't just #include nsCharTraits.h explicitly?

Good question... it looks like it's linkage-safe... let me try it.

Comment 10

[Darin Fisher]

> 2) Rename the macros with NS_ prefixes (and put them in nscore.h)?

That seems like the best choice to me.

Comment 11

[Benjamin Smedberg]

Attachment: Fix NormalizationTest.cpp, rev. 1.1

Comment 12

[Benjamin Smedberg]

Fixed on trunk.