384796 - Call to |nsIStringBundleOverride| always beats the embedding calls.

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Nick Kreeger]

In the |nsIStringBundleOverride| declaration, it declares that the interface is designed for embeders so that they can override gecko chrome strings. However, the call to initialize the override service (in |nsStringBundleService::Init()|) is always called before embedding is initialized. 

The only other class in the 1.8 tree that uses this interface is |nsStringBundleTextOverride|. It is designed to look for a file called "custom-strings.txt", if the class can't find the file, it bails out of the service registration.

See bug 248160 for an example of what Camino needs to do to translate chrome strings on the fly.

Comment 1

[Nick Kreeger]

Attachment: Patch V1

Here is a proposed fix, it adds a function (|InitStringBundleOverrideService()|) to the |nsIStringBundle| idl file. This function will assign its member function the string bundle service override service. This allows embedders to make the call and setup the override service, after they have initialized embedding.

An alternative approach would be to add a utility function that looks for the string override service everytime it wants to give a string translation, but this seemed a bit cleaner.

Comment 2

[Axel Hecht [:Pike]]

Benjamin, can you shed a light on this?

I'm interested in finding out why the string bundle service is starting up too early, or if it is.

If the patch happened to be the right approach, I'm tempted to ask for some check if mOverrideStrings is already set. Anyway, embedding startup situations is something that I'd need Benjamin to look at.

Comment 3

[Nick Kreeger]

Can we get someone to please poke someone about this bug. This patch is beginning to sit around again.

Comment 4

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

From #camino last Wednesday:

[3:15pm] mento: kreeger: for the string overrides, benjamin suggests just re-initting when xpcom-startup is received
[3:16pm] mento: or at some other good time
[3:17pm] kreeger: mento: did he mean re-init embedding or re-init the string override service?
[3:17pm] mento: re-init the override service
[3:17pm] kreeger: (becaues I'm already providing a function to re-init the service in the patch)
[3:17pm] mento: mento: seems to me that rather than adding a new API, we should just reload the override thingie at some point in time (e.g. when xpcom-startup is fired
[3:17pm] mento: right, the timing was his only suggestion
[3:17pm] kreeger: oh ok
[3:18pm] kreeger: ill find a good place to pull the scalpel out at

Comment 5

[Nick Kreeger]

Attachment: Patch V2

Here is another option. xpcom-startup observer notices are being made before nsStringBundle is getting created. So I tried poking around for other things to listen to. 

I found out that my listening to "xpcom-category-entry-added" and then seeing if the data that was being added was "xpcom-autoregistration" worked. This seemed to be OK to me since that is what was really beating out the embedding calls (or the auto-registration calls for the embeddor).

If there is a better category, or something else I should be listening for, please let me know. 

FYI. I did try to listen for just "xpcom-autoregistration" events, but those are never posted.

Comment 6

[Nick Kreeger]

Attachment: Patch V 2.5

I decided that the conversions from a 16-bit string to an 8-bit string just to compare contents was unnecessary. This patch just applies the PRUnichar buffer to a local nsAutoString for comparison.

Comment 7

[Mark Mentovai]

Comment on attachment 277857 [details] [diff] [review]
Patch V 2.5

Add {braces} to the existing conditional clause now that you're adding a {braced} else.  (That blank line in there is bad for readability.)

Comment 8

[Nick Kreeger]

Attachment: Patch V 2.6

Updated patch that meets mento's style request.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+  else if (strcmp("xpcom-category-entry-added", aTopic) == 0)
+  {

put the brace on the same line as the if

+    nsAutoString data;
+    if (data.Equals(NS_LITERAL_STRING("xpcom-autoregistration")))

should you be initializing data, perhaps?

Comment 10

[Nick Kreeger]

Attachment: Patch V 3

Doh, sorry I missed that initializer. This patch initializes the local nsAutoString and fixes the curly style.

Comment 11

[Nick Kreeger]

Attachment: Patch V 5

This patch removes the local string and does it all inline in the if statement. (Comment by biesi)

Comment 12

[Nick Kreeger]

Checked into trunk (1.9), thanks to all who contributed.