426280 - Thunderbird should not rely on toolkit/obsolete

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Joey Minta]

Attachment: patch v1

http://mxr.mozilla.org/seamonkey/source/toolkit/obsolete/content/

In particular, there's still some consumers of strres.js, globalOverlay.xul, and dialogOverlay.xul.  The dialogOverlay users are in mailnews/ and I want to look at them more closely to see which ones are in TB, so I can make sure I can test when I remove them.

This patch removes the other users.  Note that globalOverlay.xul included the non-obsolete globalOverlay.js, so I've inserted that script in a few places.

Comment 1

[[e-mail]]

Comment on attachment 312829 [details] [diff] [review]
patch v1

I can't really review the globalOverlay changes, as they're mail/-only.

>+        var sbs = Components.classes["@mozilla.org/intl/stringbundle;1"]
>+                            . getService(Components.interfaces.nsIStringBundleService)
>+        var bundle = sbs.createBundle("chrome://messenger/locale/messenger.properties");
Any particular reason for the extra var? (Avoiding wrapping counts, but sadly I can't easily determine that in a Bugzilla edit patch as comment field.)

>+             Components.classes["@mozilla.org/intl/stringbundle;1"].
>+                 getService(Components.interfaces.nsIStringBundleService)
>+                 createBundle("chrome://messenger/locale/search.properties");
This just goes to show why I like the .-prefix form of wrapping (the missing . isn't even a syntax error thanks to the wonders of JS...) so I'd say wrap it in the same way you did above. Additionally it's a field, so I'd drop the ; too:
Components.classes["@mozilla.org/intl/stringbundle;1"]
          .getService(Components.interfaces.nsIStringBundleService)
          .createBundle("chrome://messenger/locale/search.properties")

Comment 2

[Joey Minta]

Attachment: patch v2

Patch updated to neil's review comments.

Comment 3

[Phil Ringnalda (:philor)]

Comment on attachment 313516 [details] [diff] [review]
patch v2

There's no chance I wouldn't r+ killing strres, so r=me, with yet another nitting about the XPCOM whitespace:

>diff -r facfd42d2fd8 mailnews/base/resources/content/mailWidgets.xml
>     <implementation>
>       <field name="stringBundle">
>           <![CDATA[
>-           srGetStrBundle("chrome://messenger/locale/search-attributes.properties");
>+             Components.classes["@mozilla.org/intl/stringbundle;1"]
>+                 .getService(Components.interfaces.nsIStringBundleService)
>+                 .createBundle("chrome://messenger/locale/search-attributes.properties")
>           ]]>
>       </field>

Admittedly, the style of the file is "complete and total ass," but going from 2, 4, 1 space indent to 2, 4, 3 is... less improvement than I'd like to see. Please suck the CDATA markers back to 2 spaces, use two spaces within them, and then line up the dots (since you aren't going to manage to avoid going over 80 chars anyway).

Comment 4

[Joey Minta]

Patch checked in.  As far as things to look for for regressions, this is a really hard one.  Basically, any dialog touched by the patch could see regressions.  These will almost certainly take the form of a "foo is not defined" type of message in the Error Console.

Checking in mail/base/content/ABSearchDialog.xul;
/cvsroot/mozilla/mail/base/content/ABSearchDialog.xul,v  <--  ABSearchDialog.xul
new revision: 1.19; previous revision: 1.18
done
Checking in mail/base/content/SearchDialog.xul;
/cvsroot/mozilla/mail/base/content/SearchDialog.xul,v  <--  SearchDialog.xul
new revision: 1.25; previous revision: 1.24
done
Checking in mail/base/content/hiddenWindow.xul;
/cvsroot/mozilla/mail/base/content/hiddenWindow.xul,v  <--  hiddenWindow.xul
new revision: 1.11; previous revision: 1.10
done
Checking in mail/base/content/messageWindow.xul;
/cvsroot/mozilla/mail/base/content/messageWindow.xul,v  <--  messageWindow.xul
new revision: 1.39; previous revision: 1.38
done
Checking in mail/base/content/messenger.xul;
/cvsroot/mozilla/mail/base/content/messenger.xul,v  <--  messenger.xul
new revision: 1.89; previous revision: 1.88
done
Checking in mail/base/content/msgPrintEngine.xul;
/cvsroot/mozilla/mail/base/content/msgPrintEngine.xul,v  <--  msgPrintEngine.xul
new revision: 1.8; previous revision: 1.7
done
Checking in mail/base/content/msgSelectOffline.xul;
/cvsroot/mozilla/mail/base/content/msgSelectOffline.xul,v  <--  msgSelectOffline.xul
new revision: 1.8; previous revision: 1.7
done
Checking in mail/base/content/utilityOverlay.xul;
/cvsroot/mozilla/mail/base/content/utilityOverlay.xul,v  <--  utilityOverlay.xul
new revision: 1.13; previous revision: 1.12
done
Checking in mailnews/base/resources/content/mailWidgets.xml;
/cvsroot/mozilla/mailnews/base/resources/content/mailWidgets.xml,v  <--  mailWidgets.xml
new revision: 1.126; previous revision: 1.125
done
Checking in mailnews/base/resources/content/msgPrintEngine.xul;
/cvsroot/mozilla/mailnews/base/resources/content/msgPrintEngine.xul,v  <--  msgPrintEngine.xul
new revision: 1.20; previous revision: 1.19
done
Checking in mailnews/base/resources/content/msgSelectOffline.xul;
/cvsroot/mozilla/mailnews/base/resources/content/msgSelectOffline.xul,v  <--  msgSelectOffline.xul
new revision: 1.30; previous revision: 1.29
done
Checking in mailnews/base/resources/content/msgSynchronize.xul;
/cvsroot/mozilla/mailnews/base/resources/content/msgSynchronize.xul,v  <--  msgSynchronize.xul
new revision: 1.15; previous revision: 1.14
done
Checking in mailnews/base/resources/content/virtualFolderProperties.xul;
/cvsroot/mozilla/mailnews/base/resources/content/virtualFolderProperties.xul,v  <--  virtualFolderProperties.xul
new revision: 1.6; previous revision: 1.5
done
Checking in mailnews/base/search/resources/content/ABSearchDialog.xul;
/cvsroot/mozilla/mailnews/base/search/resources/content/ABSearchDialog.xul,v  <--  ABSearchDialog.xul
new revision: 1.20; previous revision: 1.19
done
Checking in mailnews/base/search/resources/content/FilterEditor.xul;
/cvsroot/mozilla/mailnews/base/search/resources/content/FilterEditor.xul,v  <--  FilterEditor.xul
new revision: 1.88; previous revision: 1.87
done
Checking in mailnews/base/search/resources/content/SearchDialog.xul;
/cvsroot/mozilla/mailnews/base/search/resources/content/SearchDialog.xul,v  <--  SearchDialog.xul
new revision: 1.89; previous revision: 1.88
done
Checking in mailnews/extensions/mailviews/resources/content/mailViewSetup.xul;
/cvsroot/mozilla/mailnews/extensions/mailviews/resources/content/mailViewSetup.xul,v  <--  mailViewSetup.xul
new revision: 1.10; previous revision: 1.9
done