382009 - Remove |nsXPIDLString| and |nsXPIDLCString| usage from Camino.

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

[Nick Kreeger]

In the future event of shifting Camino to run on top of an embedded XUL Runner, we need to remove the usage of the |nsXPIDLString| and |nsXPIDLCString| classes. These classes (to my understanding) will not be on the frozen linkage.

The corresponding |nsAString| and |nsACString| classes should be used instead.

Comment 1

[Nick Kreeger]

Attachment: XPIDL-free V1

This patch removes the nsXPIDL* usage and gets rid of some old PL_strcmp functions.

Comment 2

[Mark Mentovai]

Comment on attachment 278069 [details] [diff] [review]
XPIDL-free V1

In CocoaPromptService::GetCommonDialogLocaleString:

>-  if (NS_FAILED(rv)) return returnValue;
>+  NS_ENSURE_SUCCESS(rv, rv);

This method should return an NSString*.  Didn't the compiler warn you?  This should be NS_ENSURE_SUCCESS(rv, returnValue).

In SecurityDialogs::AlertDialog:

>-    rv = prompt->AlertCheck(windowTitle, message, dontShowAgain, &prefValue);
>+    rv = prompt->AlertCheck(windowTitle.get(), message.get(), dontShowAgain.get(), &prefValue);
>   else
>-    rv = prompt->AlertCheck(windowTitle, message, nil, nil);
>+    rv = prompt->AlertCheck(windowTitle.get(), message.get(), nil, nil);

PromiseFlatString?  Also in  -[CHBrowserView lastFindText]

>-      NS_ConvertUTF16toUTF8 utf8Value(term->text);
>+      nsCAutoString utf8Value;
>+      LossyCopyUTF16toASCII(term->text, utf8Value);

Why are you making this lossy?  Is ASCII good enough?

Comment 3

[Nick Kreeger]

Attachment: XPIDL-free V2

Updated to meet comments.

Comment 4

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 278105 [details] [diff] [review]
XPIDL-free V2

rs=pink

Comment 5

[Nick Kreeger]

Comment added to the file (inside the function call).

Checked into the trunk.

Comment 6

[Nick Kreeger]

(In reply to comment #5)
> Comment added to the file (inside the function call).

Sorry, wrong bug, no comment added.

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Do we want this on the branch to keep these files more in sync?

Comment 8

[Eli Friedman]

In case you didn't know, frozen linkage doesn't have PromiseFlatString. See http://developer.mozilla.org/en/docs/Migrating_from_Internal_Linkage_to_Frozen_Linkage.

Comment 9

[Nick Kreeger]

(In reply to comment #8)
> In case you didn't know, frozen linkage doesn't have PromiseFlatString. See
> http://developer.mozilla.org/en/docs/Migrating_from_Internal_Linkage_to_Frozen_Linkage.
> 

Doh!
I filed bug 394161 to remove |PromiseFlatString| usage in Camino since we use it several more times than just in this patch.

Comment 10

[Nick Kreeger]

(In reply to comment #7)
> Do we want this on the branch to keep these files more in sync?
> 

Let's discuss this in the meeting today. I don't think it's an issue, but I'd like to hear what mento or pink think.

Comment 11

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

From the meeting log, we said we wanted to take this on the branch if it worked there.

I just tried to apply this to the branch, and a handful of hunks fail, so I can't see if it builds/works.  Nick, do you want to try to unbitrot this and see if it works on branch?

Comment 12

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

For whatever reason, the PageInfoWindowController hunk never made it in back in 2007 :(

I pushed that hunk tonight, given that it still applied and worked (and had r/sr back then): http://hg.mozilla.org/camino/rev/65f7f62ee685

Of course, since then we've imported FAYT with its 3 nsXPIDL[C]Strings, but oh well; we're not going to clean that mess up.