311672 - Remove the obsolete 2nd parameter from |nsIStringBundleService::createBundle(...)| "JS" callers

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[O. Atsushi (Torisugari)]

Since bug 76332 was fixed, nsIStringBundleService::createBundle(...)
needs only one argument, |aURLSpec|.
> 87   nsIStringBundle createBundle(in string aURLSpec);

However, many scripts have the second argument in that function.
And contributors do not stop using that wrong style, for more than 4 years.
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsIStringBundle.idl&branch=&root=/cvsroot&subdir=mozilla/intl/strres/public&command=DIFF_FRAMESET&rev1=1.13&rev2=1.14

lazy scripts list:
http://lxr.mozilla.org/mozilla/source/browser/base/content/search.xml
http://lxr.mozilla.org/mozilla/source/browser/components/bookmarks/content/bookmarks.js
http://lxr.mozilla.org/mozilla/source/toolkit/content/widgets/browser.xml
http://lxr.mozilla.org/mozilla/source/toolkit/content/widgets/dialog.xml
http://lxr.mozilla.org/mozilla/source/toolkit/content/widgets/stringbundle.xml
http://lxr.mozilla.org/mozilla/source/toolkit/content/widgets/wizard.xml
http://lxr.mozilla.org/mozilla/source/toolkit/content/contentAreaUtils.js
http://lxr.mozilla.org/mozilla/source/xpfe/communicator/resources/content/contentAreaUtils.js
http://lxr.mozilla.org/mozilla/source/xpfe/components/bookmarks/resources/bookmarks.js
http://lxr.mozilla.org/mozilla/source/xpfe/global/resources/content/bindings/browser.xml
http://lxr.mozilla.org/mozilla/source/xpfe/global/resources/content/bindings/dialog.xml
http://lxr.mozilla.org/mozilla/source/xpfe/global/resources/content/bindings/stringbundle.xml
http://lxr.mozilla.org/mozilla/source/xpfe/global/resources/content/bindings/wizard.xml

Comment 1

[Nickolay_Ponomarev]

Sounds valid, no duplicates found - confirming.

Comment 2

[O. Atsushi (Torisugari)]

Attachment: patch for xpfe

Comment 3

[O. Atsushi (Torisugari)]

Attachment: patch for browser/.../bookmarks.js + toolkit

I'm not too sure I should delete |appLocale| property in <xul:stringbundle>

http://xulplanet.com/references/elemref/ref_stringbundle.html#prop_appLocale

I guess nobody (among any extension authors) are using that property,
but changing XUL spec is something awful...

Comment 4

[[e-mail]]

Comment on attachment 204112 [details] [diff] [review]
patch for xpfe

>       this._brandShortName = BUNDLESVC.createBundle(brandBundle,     LOCALESVC.getApplicationLocale())
Nit: missed one :-P

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Torisugari, were you still planning on working on this?

Comment 6

[Serge Gautherie (:sgautherie)]

Attachment: (Cv1) </suite/*/*> [Checkin: Comment 16]

</xpfe/> patch, with comment 4 suggestion(s),
and ported to </Suite/>,
with further rewrite.

Comment 7

[Serge Gautherie (:sgautherie)]

Attachment: (Dv1) </browser/*/*>

[Unified diff; no(t) Hg.]

Too bad this function is duplicated...

NB: <bookmarks.js>, from the older patch, doesn't exist anymore.

Comment 8

[Simon Bünzli]

Comment on attachment 328238 [details] [diff] [review]
(Dv1) </browser/*/*>

Thanks, Serge. BTW: You could also just completely remove _getStringBundle from nsSessionStore.js, as it's never been used at all (since the code needing it was moved to nsSessionStartup.js).

Comment 9

[Serge Gautherie (:sgautherie)]

Attachment: (Dv1a) </browser/*/*> [Checkin: Comment 17]

Dv1, with comment 8 suggestion(s),
and related nits.

Comment 10

[Serge Gautherie (:sgautherie)]

(In reply to comment #5)
> Torisugari, were you still planning on working on this?

He emailed me that he is not.

Comment 11

[Serge Gautherie (:sgautherie)]

Attachment: (Ev1) </toolkit/*/*>

Older </toolkit/> patch,
with further rewrite.

2 untested/uncompiled points to double check:
*<nsNavHistoryResult.cpp>: bug 323492 should have removed these lines too, shouldn't it ?
*<stringbundle.xml> removal of |<property name="appLocale">|: see comment 3.

NB: <wizard.xml> use of its |var localeService| was removed by bug 142502.

Comment 12

[Asaf Romano (gone)]

Comment on attachment 328289 [details] [diff] [review]
(Ev1) </toolkit/*/*>

r=mano

Comment 13

[Serge Gautherie (:sgautherie)]

No other cases to fix (in mozilla & mozilla-central) :-)

Comment 14

[O. Atsushi (Torisugari)]

(In reply to comment #11)
> Created an attachment (id=328289) [details]
> (Ev1) </toolkit/*/*>
> 
> Older </toolkit/> patch,
> with further rewrite.
> 
> 2 untested/uncompiled points to double check:
> *<nsNavHistoryResult.cpp>: bug 323492 should have removed these lines too,
> shouldn't it ?
> *<stringbundle.xml> removal of |<property name="appLocale">|: see comment 3.
> 
> NB: <wizard.xml> use of its |var localeService| was removed by bug 142502.
> 

>diff -r b1e8d5739778 -r 0c65c6af8987 toolkit/content/widgets/wizard.xml
>--- a/toolkit/content/widgets/wizard.xml	Mon Jul 07 02:52:56 2008 +0200
>+++ b/toolkit/content/widgets/wizard.xml	Mon Jul 07 04:41:08 2008 +0200
>@@ -175,12 +175,9 @@
>         try {
>           // need to create string bundle manually instead of using <xul:stringbundle/>
>           // see bug 63370 for details
>-          var localeService = Components.classes["@mozilla.org/intl/nslocaleservice;1"]
>-                                .getService(Components.interfaces.nsILocaleService);
>-          var stringBundleService = Components.classes["@mozilla.org/intl/stringbundle;1"]
>-                                      .getService(Components.interfaces.nsIStringBundleService);
>-          var bundleURL = "chrome://global/locale/wizard.properties";
>-          this._bundle = stringBundleService.createBundle(bundleURL);
>+          this._mStrBundle = Components.classes["@mozilla.org/intl/stringbundle;1"]
>+                                       .getService(Components.interfaces.nsIStringBundleService)
>+                                       .createBundle("chrome://global/locale/wizard.properties");
>         } catch (e) {
>           // This fails in remote XUL, which has to provide titles for all pages
>           // see bug 142502
>

Are you sure to change from |this._bundle| to |this._mStrBundle| ?

Comment 15

[Serge Gautherie (:sgautherie)]

Attachment: (Ev1a) </toolkit/*/*> [Checkin: Comment 17]

Ev1, with comment 14 suggestion(s).

(Good catch ! I double checked again...)

Comment 16

[Frank Wein [:mcsmurf]]

Comment on attachment 328182 [details] [diff] [review]
(Cv1) </suite/*/*>
[Checkin: Comment 16]

Checking in bookmarks/bookmarks.js;
/cvsroot/mozilla/suite/common/bookmarks/bookmarks.js,v  <--  bookmarks.js
new revision: 1.145; previous revision: 1.144
done
Checking in contentAreaUtils.js;
/cvsroot/mozilla/suite/common/contentAreaUtils.js,v  <--  contentAreaUtils.js
new revision: 1.154; previous revision: 1.153
done

Comment 17

[Dão Gottwald [:dao]]

http://hg.mozilla.org/index.cgi/mozilla-central/rev/d4c293d33170