Closed
Bug 56680
Opened 24 years ago
Closed 11 years ago
use a xul <stringbundle/> instead of including the strres.js code
Categories
(Core :: XUL, defect)
Core
XUL
Tracking
()
RESOLVED
FIXED
mozilla26
People
(Reporter: jag+mozbugs, Assigned: sgautherie)
References
(Blocks 1 open bug, )
Details
(Keywords: meta)
Attachments
(6 files, 18 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
iannbugzilla
:
review+
neil
:
superreview+
|
Details | Diff | Splinter Review |
<BenGoodger> Ok, basically every time you include a js file you get that js file again since they aren't cached so all over the product we have 65,423 people including strres.js over and over. The binding is cached ;) Plus, strres also has a bunch of other shit that no one ever uses in it. </BenGoodger> Currently there are 83 .xul files "including" strres.js which could benefit from Ben's <stringbundle/>. I suggest we move to <stringbundle/>. The migration is simple: In the xul file: - add a <stringbundle id="foo" src="chrome://path/to/file.properties"/> - remove the strres.js reference In the js file: - replace the stringbundle fetching code with a document.getElementById('foo'); - replace .GetStringFromName(property) with .getString(property) I'll attach a sample patch for the xp filepicker.
Reporter | ||
Comment 1•24 years ago
|
||
Reporter | ||
Comment 3•24 years ago
|
||
In that case I'll spend some time tracking module owners and creating a bunch of patches :-)
Comment 5•24 years ago
|
||
If we did cache and share JS scripts loaded via <script src="..."/> in XUL, then I wonder how big a win this <stringbundle> tag would be over the XPConnect way. I like it for other reasons (language-neutrality, XUL+DOM sufficiency), don't get me wrong. Jag, any idea of the win from this one change, in terms of code and data footprint? /be
Reporter | ||
Comment 6•24 years ago
|
||
I've no clue on the win. How'd I find out? Btw, I believe we're still doing things the XPConnect way (behind the scenes), take a look at stringbundleBindings.xml.
Comment 7•24 years ago
|
||
Use a reliable process monitor, one that tells the truth about data segment size moment by moment. Do A-B tests starting mozilla on the test page, measure size. /be
Reporter | ||
Comment 10•24 years ago
|
||
Comment 12•24 years ago
|
||
you sexy bitch. my personal fetish is for globals to be prefixed with 'g' so that its obvious that the var is global when it's used throughout the file. If you agree with me, you can change it, otherwise, feel free to ignore me. a=ben@netscape.com
Reporter | ||
Comment 13•24 years ago
|
||
Comment 15•24 years ago
|
||
Comment 16•24 years ago
|
||
+ if (window.arguments && window.arguments[0]) { "arguments" in window, no?
Comment 17•24 years ago
|
||
Ok, ignoring that last patch for now, as I'm intending to do mailnews/* at once, due to interdependencies between overlays, etc. Also, jag and I were discussing the conventions to be used, and going forward it will be: .xul -> id="bundle_<identifier>" .js -> g<Identifier>Bundle where <identifier> is the first part of the .properties file referenced e.g. for search.properties we would have id="bundle_search" in xul and gSearchBundle in js
Comment 18•24 years ago
|
||
Comment 19•24 years ago
|
||
Ok, the patch just attached requires jag's patch to 68449 to even stand a chance. Also, the attached patch includes, as an unintended bonus, the patches to 61801 and 61802 which will likely be checked in well ahead of this and thus leave in a later revisions of this patch. That said, there are also instances where I couldn't yet avoid strres.js, especially where it is used by xbl controls... Also, there are a few places that look like code was moved around arbitrarily, but it must be noted that the getElementById() call will fail when the .js file is first loaded, so some things got moved to accommodate that. I tested as much of mailnews as I could, but I didn't cover IMAP and might have missed some other things, so it might be nice to get a mailnews qa, or someone equally familiar with this area, to apply this patch and run through their testsuite before this gets near the tree. enjoy
Comment 20•24 years ago
|
||
Index: mozilla/mailnews/addrbook/resources/content/abCardViewOverlay.js wouldn't a global object containing the z* properties be more appropriate? what are the issues [performance ?] between globals and objects? <q class="ignore" comment="if someone else convinces you to do a rewrite, ..."> at some point we need to fix <script language="javascript" src=...> and <script src=...> to <script type="text/javascript" src=...> It looks like you're continuing tabs, esp AccountManager.js,v @@ -201,7 +201,7 @@ if (!a || a == "") => if (!a) ibid. return(a); => return a; ... if (!(accountCount > 0)) { I know, not your fault. maybe i need a bug against me to do all this. </q> + <!-- XXX: only mailWidgets.xml requires strres.js (<script> isn't valid in XUL yet - see hyatt)--> ?@#? function onNewFilter() 2spaces or 4? there are 2 //dump()s that your patch mentions that you didn't remove. Do we think they're more important? the bundle conventions sound good.
Assignee: disttsc → maolson
Reporter | ||
Comment 21•24 years ago
|
||
Regarding your re-ordering trick, I think in the end you'll be safer off by always initializing var gFooBundle from the onload function instead of depending on load order (which is fragile, meaning you need comments indicating to keep that stuff in that specific order, blah). If for whatever reason you can't edit the onload function (in the case of an overlay where the overlay doesn't provide the onload function for example) you can do something like this in the overlay's js: var gFooBundle window.addListener("load", overlayInit, false); function overlayInit() { gFooBundle = document.getElementById(...); } In the cases you're touching it looks like you won't need that though. I see you've put some <stringbundle/>s in windows which are used by the overlay. What you could do instead is use <stringbundleset id="stringbundleset"/>. Also, the other way around won't work. If you're placing a <stringbundle/> in an overlay, you'll need to have a <stringbundle/> with the same id in the document you're overlaying on. In that case, again it's easier to use <stringbundleset id="stringbundleset"/> and wrap the <stringbundle/> in the overlay in it. For example, put <stringbundleset id="stringbundleset"/> in : /mailnews/addrbook/resources/content/abEditCardDialog.xul /mailnews/addrbook/resources/content/abNewCardDialog.xul Then put this: <stringbundleset id="stringbundleset"> <stringbundle id="bundle_addressBook src="chrome://messenger/locale/addressbook/addressBook.properties"/> </stringbundleset> in /mailnews/addrbook/resources/content/abCardOverlay.xul That way you keep the stringbundle and the code closer together. Same for abCardViewOverlay.* and addressbook.xul. These two files already have abMailListDialog.js /mailnews/addrbook/resources/content/abMailListDialog.xul /mailnews/addrbook/resources/content/abEditListDialog.xul because they're having this file overlayed: /mailnews/addrbook/resources/content/abListOverlay.xul I think you can safely remove the script tags from the first two, or alternatively (and better, I think) you can split the JS up into two files. Anyway, here too you can use a stringbundle set, and it looks like you didn't add the code to init gAddressBookBundle. Also, this change is not needed if you're always calling awClickEmptySpace with a DOM element as |targ|: - if (targ.localName != 'treechildren') + if ("localName" in targ && targ.localName != 'treechildren') /mailnews/addrbook/resources/content/abSelectAddressesDialog.xul and .js It looks like the <stringbundle/> needed by MsgComposeCommands.js is only ever used in messengercompose.xul. For now I think it's safe to not put the composeMsgs <stringbundle/> in abSelectAddressesDialog.xul, though ultimately the code shared should be factored out (less bloat). ---- more to come, but I feel a crash coming up ;-)
Reporter | ||
Comment 22•24 years ago
|
||
About WizardManager, you could move these four scripts into wizardOverlay.xul: <script src="chrome://global/content/wizardOverlay.js"/> <script src="chrome://global/content/wizardHandlerSet.js"/> <script src="chrome://global/content/wizardManager.js"/> <script src="chrome://global/content/widgetStateManager.js"/> And then you can do the <stringbundleset/> thing again :-) Same deal about moving <stringbundle/> from newFolderDialog.xul and renameFolderDialog.xul to msgFolderPickerOverlay.xul Etc. etc... If you want specifics I'll look further and write them in an e-mail :-) You've done great work here though, don't let all these comments make you think otherwise. Since you're touching these lines, you could use |foo.disabled = !enable;| + if (enable) { + checkbox.removeAttribute("disabled"); + numberFld.removeAttribute("disabled"); + } + else { + checkbox.setAttribute("disabled", "true"); + numberFld.setAttribute("disabled", "true"); + }
Comment 23•24 years ago
|
||
Nice work. A few comments: 1. StringBundles are cached insde strres module. Once a bundle is created, it is cached inside nsIStringBundleService. Furhter request of stringBundle creation of the same URI gets the cached bundle. 2. StringBundle creation and loading is a synchronous operation which blocks the UI thread. Attempting to create several bundles at once during startup might be a performance drag. 3. Q: What's the life span of these XUL binding cached bundles and how are they cached? Be careful not to reference a bogus bundle. This probably won't happen if the xul binding cache is a in-memory cache.
Reporter | ||
Comment 24•24 years ago
|
||
> 1. StringBundles are cached insde strres module. Once a bundle is created, > it is cached inside nsIStringBundleService. Furhter request of stringBundle > creation of the same URI gets the cached bundle. I guess what's cached here is the xbl prototype (xbl binding DOM + compiled script), which is a win over having to compile strres.js every time it's used. But you should really ask Ben or hyatt... > 2. StringBundle creation and loading is a synchronous operation which blocks > the UI thread. Attempting to create several bundles at once during startup > might be a performance drag. Yeah, that's why I recently changed <stringbundle/> to lazily create the bundle, i.e. the first time someone does |getString| or |getFormattedString|, instead of at xbl binding time. > 3. Q: What's the life span of these XUL binding cached bundles and how are > they cached? Be careful not to reference a bogus bundle. This probably > won't happen if the xul binding cache is a in-memory cache. I'd expect the per instance data to live as long as the instance, the DOM element. In these cases, that's as long as the window is open.
Comment 25•24 years ago
|
||
Comment 26•24 years ago
|
||
Reporter | ||
Comment 27•24 years ago
|
||
For some of the getFormattedString calls you could make the second param (on the next line) line up with the first. abCardViewOverlay.js: You have a var gAddressBookBundle in addressbook.js which will clash with the one in here. One way around this is to use function local vars. Another is to just expect gAddressBookBundle to already be set. A third is to do something like: At the global level: if (!("gAddressBookBundle" in window)) var gAddressBookBundle; And then in onload: if (!gAddressBookBundle) gAddressBookBundle = document.getElementById("..."); In this case I'd just expect gAddressBookBundle to be set, perhaps add a note that it's set in addressbook.js' onload function. Normally I'd pick the function local var option, though abMailListDialog.js: Perhaps we should replace the <script> tags in abEditListDialog.xul and abMailListDialog.xul with comments referring to abListOverlay.xul for abMailListDialog.js. function awClickEmptySpace(targ, setFocus) { - if (targ.localName != 'treechildren') + if ("localName" in targ && targ.localName != 'treechildren') return; I don't think that change is necessary. msgAccountCentral.js: It looks like the string bundle vars don't need to be global there. In subscribe.js: +var gSubscribeBundle = document.getElementById("bundle_subscribe"); In SearchDialog.js: - if (gCurrentFolder && (searchSubfolders || gCurrentFolder.isServer)) - { + if (gCurrentFolder && (searchSubfolders || gCurrentFolder.isServer)) + { AddSubFolders(gCurrentFolder); } Fix those other two lines too then :-) In SearchDialog.xul: - <script src="chrome://global/content/globalOverlay.js"/> <script src="chrome://messenger/content/SearchDialog.js"/> + <script src="chrome://global/content/globalOverlay.js"/> Tsk! :-) downloadheaders.js: + if (window.arguments && window.arguments[0]) { ... - numberElement = document.getElementById("number"); - numberElement.value = nntpServer.maxArticles; + numberElement = document.getElementById("number"); + numberElement.value = nntpServer.maxArticles; Indentation looks wrong there (switching from 4 to 2). In importDialog.js: - meterText = GetFormattedBundleString('MailProgressMeterText', name); + meterText = gImportMsgsBundle.getFormattedString('MailProgressMeterText', name); You forgot to add the [ ] which were added inside GetFormattedBundleString. What do you think, this change, or just changing the implementation of Get(Formatted)BundleString? importDialog.xul: is this a dead file? In MsgComposeCommands.js: +var gComposeMsgsBundle = document.getElementById("bundle_composeMsgs"); Forgot one ;-) In msgPrintEngine.xul: are you sure you can remove that? utilityOverlay.js seems to need it, and since utilityOverlay.* hasn't been converted yet, I'm not sure that's safe. Then again, it looks like utilityOverlay.xul isn't used there at all. *sigh* In messageWindow.xul: You'll need to remove the <stringbundleset id="stringbundleset"/> a bit lower in that file :-) In mailWindowOverlay.xul: I'm not sure if you can remove the stringbundleset there, I'm using it has hook for the overlays on that overlay. If mWO.xul is first overlayed, and then its overlays are overlayed, your change is fine. If the overlays are first overlayed on mWO.xul, before it is overlayed itself, then there's a problem. I didn't look this up and just kept on the safe side, but this is an interesting question IMO. In am-copies.xul: + <script type="text/avascript" src="chrome://messenger/content/am-copies.js"/> ^^^^^^^^^^^^^^ Oops... The rest of the code looks pretty good. What do you think about adding comments like: // gMessengerBundle needs to be defined and set to use this overlay and // gMessengerBundle can be found in foo.js / foo.xul ?
Comment 28•24 years ago
|
||
Sweet! I can always count on thorough comments from jag, that's for sure... If I don't mention it, it's done. Othewise: - if (targ.localName != 'treechildren') + if ("localName" in targ && targ.localName != 'treechildren') Remnant of a similar strict warning fix - may or may not be necessary, but should be harmless either way. I screwed up the calls to getFormattedString, that doesn't mean we should break the function to fix the programmer. I think last time we talked about this, it was decided to leave getFormattedString alone. Now I think it might be a good idea to assert or at least dump() loudly if we get a string instead of an array. importDialog.xul - I'd like to stay (somewhat) focused on the bundle changes, since an sr is going to be hard enough here... Should we see what brendan can do about an avascript engine for that newly created type there? :( Do I sense some unease with the status quo here? Perhaps some refactoring here jag? There seem to be a couple areas that could benefit from a careful look at the js. I think that's all the comments here. I think we have a few side bugs that might spring out from what we've seen here. Using globals in overlayed js is just asking for conflicts, so maybe we need to grab bundles locally to each function that uses them, although the globals may provide more benefit than trouble... New patch momentarily, after some testing.
Status: NEW → ASSIGNED
Comment 29•24 years ago
|
||
Reporter | ||
Comment 30•24 years ago
|
||
About getFormattedString:
> What do you think, this change, or just changing the implementation of
> Get(Formatted)BundleString?
With that I meant changing the implementation of those functions in
importDialog.js:
function GetFormattedBundleString(strId, formatStr)
{
return gImportMsgsBundle.getFormattedString(strId, [formatStr]);
}
Or something like that...
I think calling nsIStringBundle::formatStringFromName with an incorrect argument
type (string instead of array) will cause xpconnect to throw some exception of
that kind, so I don't think adding special code in |getFormattedString| is
necessary.
r=jag.
Comment 31•24 years ago
|
||
Ok, since the mailnews stuff has jag's highly sought after r=, cc'ing all three of the mailnews sr folks since any or all might be interested. (Or all three might think the others are covering this - let's hope that doesn't happen).
Comment 32•24 years ago
|
||
Comment 35•24 years ago
|
||
Comment 36•24 years ago
|
||
Latest attachment removes strres.js include from mini-nav.xul (it wasn't actually used there) and the embedding jar.mn. stringBundleBindings.xml is already in jar.mn, so embedding can use that if stringbundles are necessary. Removing mailnews folks I cc'd earlier to save them the spam.
Comment 38•24 years ago
|
||
Embedding portion checked in (sr=blizzard on irc) Profile manager diff coming up.
Comment 39•24 years ago
|
||
Comment 40•24 years ago
|
||
function onLoad() strict error. function doesn't always return a value. suggest: return; <script language="javascript" ...></script> should be <script type="application/x-javascript" .../> although until we do the mass change, i'll accept text/javascript. + lString = lString.replace(/%brandShortName%/gi, + gBrandBundle.getString("brandShortName")); breaks the %s convention and using the bundle formatting functions rules.
Comment 41•24 years ago
|
||
timeless: shouldn't you suggest 'return true;' at the end of onLoad, instead of just 'return;'? /be
Comment 43•24 years ago
|
||
Comment 44•24 years ago
|
||
html:iframe => iframe the .replace() stuff is more important. jag want to offer some help?
Comment 45•24 years ago
|
||
timeless, what patch are you reviewing? Surely it isn't one attached to a bug entitled "use a xul <stringbundle/> instead of including the strres.js code", because none of your comments have addressed anything material to the issue at hand. The only reason I touched the <html:iframe> line was to remove a tab. Whatever issues you have with replace(), please take up in another bug as this one is quite large enough of its own accord.
Comment 46•24 years ago
|
||
r=timeless w/ leway to fix stuff i've mentioned. fwiw the replace stuff is now covered by another bug
Comment 48•24 years ago
|
||
Reporter | ||
Comment 49•24 years ago
|
||
mozilla/xpfe/components/prefwindow/resources/content/pref-proxies.js + prefutilitiesBundle = document.getElementById("bundle_prefutilities"); no var? mozilla/xpfe/components/prefwindow/resources/content/pref-proxies.xul @@ -102,7 +110,7 @@ <textfield id="networkProxySOCKS_Port" pref="true" preftype="int" prefstring="network.proxy.socks_port" prefattribute="value" size="5"/> </box> - </row> + </row> <row> Is that re-indent correct? mozilla/xpfe/components/prefwindow/resources/content/pref-smart_browsing.xul: + var regionBundle = document.getElementById("bundle_region"); + if (regionBundle) + var smartBrowsingURL = regionBundle.getString("smartBrowsingURL"); There is no need for that if...
Comment 50•24 years ago
|
||
Comment 54•24 years ago
|
||
Reporter | ||
Comment 55•24 years ago
|
||
Hmmm... I say we create a prefutilities.xul and dump some stuff in there :-) Then you can add a <stringbundleset> to the files that need it and put the actual <stringbundle> in there.
Updated•21 years ago
|
Assignee: maolson → jag
Status: ASSIGNED → NEW
Comment 57•20 years ago
|
||
Also includes a small optimisation for the Add Languages dialog (it doesn't build its own copy of the available languages array any more). Timeless, could you review this or pass it on if you don't want it?
Attachment #173192 -
Flags: review?(timeless)
Comment 58•20 years ago
|
||
Comment 59•20 years ago
|
||
Comment on attachment 173192 [details] [diff] [review] Patch for prefs / languages seems ok, but i'd rather pass :)
Attachment #173192 -
Flags: review?(timeless) → superreview?(neil.parkwaycc.co.uk)
Comment 60•20 years ago
|
||
Comment on attachment 173192 [details] [diff] [review] Patch for prefs / languages Just by reading it two things spring to mind; the first is that the popup dialog only uses one of the string bundles. The second is that we normally list stringbundles near the top after the scripts. However more importantly I don't like the way the Init method is overloaded. What should happen is this: 1. The panel's onload should be the call to parent.initPanel. 2. There should be a Startup() function for the panel. Conveniently parent.initPanel calls this automatically. 3. The popup dialog's startup function can stay in Init. Another point is that we generally store the stringbundle element in the variable and call the XBL functions rather than directly calling the C++ interface; for instance this saves a parameter when calling getFormattedString, although this doesn't help in the case of acceptBundle.
Comment 61•20 years ago
|
||
(In reply to comment #60) Thanks for the input, Neil, really appreciated! > Just by reading it two things spring to mind; the first is that the popup > dialog only uses one of the string bundles. The second is that we normally list > stringbundles near the top after the scripts. Right, no problem. > However more importantly I don't > like the way the Init method is overloaded. What should happen is this: > 1. The panel's onload should be the call to parent.initPanel. > 2. There should be a Startup() function for the panel. Conveniently > parent.initPanel calls this automatically. > 3. The popup dialog's startup function can stay in Init. I didn't knew what initPanel actually does. Now I do, will fix. > Another point is that we generally store the stringbundle element in the > variable and call the XBL functions rather than directly calling the C++ > interface; I wanted to keep the patch small, but again, no problem. Since bug 280693 complains about createElement for XUL I'll also convert that to creatElementNS. New patch coming later today.
Comment 62•20 years ago
|
||
As described above. LXR says I'm the only one using parent.initPanel(this.baseURI). I hope that's ok, I hate repeating long string constants that are available elsewhere. I also fixed a bug caused by the broken array logic in AddAvailableLanguages which could lead to 'undefined' appearing in the listbox. To reproduce: 1. Add Afrikaans 2. Open the Add... dialog again, select Afrikaans and Albanian. 3. Click ok. Result: Afrikaans, undefined, Albanian.
Attachment #173192 -
Attachment is obsolete: true
Attachment #173193 -
Attachment is obsolete: true
Attachment #173249 -
Flags: review?(neil.parkwaycc.co.uk)
Comment 63•20 years ago
|
||
Updated•20 years ago
|
Attachment #173192 -
Flags: superreview?(neil.parkwaycc.co.uk)
Comment 64•20 years ago
|
||
Argh, I believed |this| was a reference to the page element but instead it points to the window, just like in HTML. document.documentURI is better.
Comment 65•20 years ago
|
||
I did wonder about this.baseURI ;-) I can't get your patches to apply: patching pref-languages-add.xul 2 out of 2 hunks FAILED patching pref-languages.js 7 out of 11 hunks FAILED patching pref-languages.xul 2 out of 3 hunks FAILED
Comment 66•20 years ago
|
||
Neil, strange, the diff is against the latest version on the trunk. You don't have my patch from bug 168728 in your tree, do you? That won't work...
Comment 68•20 years ago
|
||
Attachment #173249 -
Attachment is obsolete: true
Attachment #173250 -
Attachment is obsolete: true
Attachment #173254 -
Attachment is obsolete: true
Updated•20 years ago
|
Attachment #173249 -
Flags: review?(neil.parkwaycc.co.uk)
Comment 69•19 years ago
|
||
I guess I'm too late to weigh in much on this one, but here goes anyway: I prefer strres.js. It means I can directly access the string bundle service from JS code instead of using a "hopefully-unique" ID for yet another XUL element. You have a problem with Moz not cacheing JS files, why not fix that instead of creating this 'stringbundle' element? I find it much cleaner to access the stringbundle service purely from within JS code than to have this arbitrary mixing of XUL and JS, with messy grabbing of an element via Yet Another Unique ID. Someone care to explain how this is superior?
Assignee | ||
Updated•16 years ago
|
Attachment #27439 -
Attachment is obsolete: true
Assignee | ||
Updated•16 years ago
|
Attachment #28222 -
Attachment description: patch - prefs v2 → patch - prefs v2
[Checkin: Comment 53]
Assignee | ||
Updated•16 years ago
|
Attachment #26606 -
Attachment description: patch - remove strres.js from embedding → patch - remove strres.js from embedding
[Checkin: Comment 38]
Assignee | ||
Updated•16 years ago
|
Attachment #25423 -
Attachment is obsolete: true
Assignee | ||
Updated•16 years ago
|
Attachment #25870 -
Attachment description: patch - resync w/ tip after bug 68505 checkin → patch - resync w/ tip after bug 68505 checkin (mailnews)
[Checkin: Comment 34]
Assignee | ||
Updated•16 years ago
|
Attachment #25048 -
Attachment is obsolete: true
Assignee | ||
Updated•16 years ago
|
Attachment #26765 -
Attachment is obsolete: true
Assignee | ||
Updated•16 years ago
|
Attachment #26852 -
Attachment description: patch - profile manager v2 → patch - profile manager v2
[Checkin: Comment 47]
Assignee | ||
Updated•16 years ago
|
Attachment #25330 -
Attachment description: update (tip shifted a bit today - no functional changes) → update (tip shifted a bit today - no functional changes) (mailnews)
Attachment #25330 -
Attachment is obsolete: true
Assignee | ||
Updated•16 years ago
|
Attachment #25326 -
Attachment description: patch - latest thinking on the matter → patch - latest thinking on the matter (mailnews)
Attachment #25326 -
Attachment is obsolete: true
Assignee | ||
Updated•16 years ago
|
Assignee | ||
Updated•16 years ago
|
Attachment #25006 -
Attachment description: patch - downloadheaders.* (includes detabbification and agreement with modeline changes) → patch - downloadheaders.* (includes detabbification and agreement with modeline changes) (mailnews/news only)
Attachment #25006 -
Attachment is obsolete: true
Assignee | ||
Comment 70•16 years ago
|
||
Comment on attachment 24026 [details] [diff] [review] [patch] Move navigator over to <stringbundle/> [Checkin: Comment 70] {{ 2001-02-01 01:53 disttsc%bart.nl ... Move over, strres.js, the new, sexy <stringbundle/> is in Browser Town. bug=56680, r=timeless, a=ben }} with comment 12 suggestion(s).
Attachment #24026 -
Attachment description: [patch] Move navigator over to <stringbundle/> → [patch] Move navigator over to <stringbundle/>
[Checkin: Comment 70]
Assignee | ||
Updated•16 years ago
|
Attachment #24935 -
Attachment description: [patch] missed something in navigatorDD.js → [patch] missed something in navigatorDD.js
[Moved to bug 72137]
Attachment #24935 -
Attachment is obsolete: true
Assignee | ||
Updated•16 years ago
|
Attachment #28689 -
Attachment description: patch - convert prefutilities.js → patch - convert prefutilities.js
[Superseded by bug 73355]
Attachment #28689 -
Attachment is obsolete: true
Assignee | ||
Updated•16 years ago
|
Attachment #173254 -
Attachment description: pref-languages.xul → pref-languages.xul (only)
Assignee | ||
Updated•16 years ago
|
Attachment #17153 -
Attachment description: [sample-patch] use <stringbundle/> in xp filepicker → [sample-patch] use <stringbundle/> in xp filepicker
[Superseded by bug 27612]
Attachment #17153 -
Attachment is obsolete: true
Assignee | ||
Comment 71•16 years ago
|
||
Comment on attachment 173633 [details] [diff] [review] pref-languages v4 Only the reminder comment got checked in yet: <http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/suite/common/pref/pref-languages-add.xul&rev=HEAD&mark=1.11> <http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/suite/common/pref/pref-languages.xul&rev=HEAD&mark=1.26> Waiting for answer to bug 444411 question to know how to proceed.
Assignee | ||
Comment 72•16 years ago
|
||
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.0.2pre) Gecko/2008070902 SeaMonkey/2.0a1pre] (nightly) (W2Ksp4) "pref-languages v4", ported to </suite/>, plus: *a few "nits". *removal of the obsoleted |broadcaster| element.
Assignee: jag → sgautherie.bz
Attachment #173633 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #328807 -
Flags: superreview?(neil)
Attachment #328807 -
Flags: review?
Assignee | ||
Updated•16 years ago
|
Attachment #328807 -
Flags: review? → review?(iann_bugzilla)
Comment 73•16 years ago
|
||
(In reply to comment #72) > Created an attachment (id=328807) [details] > (Iv4a) <pref-languages*.*> > > [Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.0.2pre) > Gecko/2008070902 SeaMonkey/2.0a1pre] (nightly) (W2Ksp4) > > "pref-languages v4", ported to </suite/>, > plus: > *a few "nits". > *removal of the obsoleted |broadcaster| element. > I would forget about making the whitespace changes to xul as this will all be changing with your patch on bug 444411
Assignee | ||
Comment 74•16 years ago
|
||
(In reply to comment #73) > I would forget about making the whitespace changes to xul as this will all be Iv4a, with comment 73 suggestion(s). > changing with your patch on bug 444411 Note that I am not working on that bug.
Attachment #328977 -
Flags: review?(iann_bugzilla)
Comment 75•16 years ago
|
||
Comment on attachment 328807 [details] [diff] [review] (Iv4a) <pref-languages*.*> The stringbundle-specific changes look OK but I didn't look at any of the other miscellaneous changes as those probably belong in bug 444411.
Attachment #328807 -
Flags: superreview?(neil) → superreview+
Assignee | ||
Updated•16 years ago
|
Attachment #328807 -
Attachment is obsolete: true
Attachment #328807 -
Flags: review?(iann_bugzilla)
Comment 76•16 years ago
|
||
Comment on attachment 328977 [details] [diff] [review] (Iv4b) <pref-languages*.*> As Neil said, only the strres.js related changes should be in here r=me for just those changes.
Attachment #328977 -
Flags: review?(iann_bugzilla) → review+
Assignee | ||
Comment 77•16 years ago
|
||
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.0.2pre) Gecko/2008071402 SeaMonkey/2.0a1pre] (nightly) (W2Ksp4) Iv4b, with comment 75 & 76 suggestion(s). Like this ? Or even without the "onload" change ?
Attachment #328977 -
Attachment is obsolete: true
Attachment #329561 -
Flags: superreview?(neil)
Attachment #329561 -
Flags: review?(iann_bugzilla)
Attachment #329561 -
Flags: review?(iann_bugzilla) → review+
Comment 78•16 years ago
|
||
Comment on attachment 329561 [details] [diff] [review] (Iv4c) <pref-languages*.*> [Checkin: Comment 79] The new pane will need Startup() so I don't think that's a problem.
Attachment #329561 -
Flags: superreview?(neil) → superreview+
Assignee | ||
Updated•16 years ago
|
Comment 79•16 years ago
|
||
Comment on attachment 329561 [details] [diff] [review] (Iv4c) <pref-languages*.*> [Checkin: Comment 79] Checking in pref-languages-add.xul; /cvsroot/mozilla/suite/common/pref/pref-languages-add.xul,v <-- pref-languages-add.xul new revision: 1.24; previous revision: 1.23 done Checking in pref-languages.js; /cvsroot/mozilla/suite/common/pref/pref-languages.js,v <-- pref-languages.js new revision: 1.36; previous revision: 1.35 done Checking in pref-languages.xul; /cvsroot/mozilla/suite/common/pref/pref-languages.xul,v <-- pref-languages.xul new revision: 1.52; previous revision: 1.51 done
Attachment #329561 -
Attachment description: (Iv4c) <pref-languages*.*> → (Iv4c) <pref-languages*.*> (Checked in)
Updated•16 years ago
|
Keywords: checkin-needed
Whiteboard: [c-n: Iv4c // Leave opened]
Assignee | ||
Updated•16 years ago
|
Attachment #329561 -
Attachment description: (Iv4c) <pref-languages*.*> (Checked in) → (Iv4c) <pref-languages*.*>
[Checkin: Comment 79]
Assignee | ||
Comment 80•16 years ago
|
||
Thanks Erik for all the good work he had done on this "language" patch !
Keywords: meta
Whiteboard: [Waiting for blocking bugs, for now]
Assignee | ||
Comment 81•16 years ago
|
||
(In reply to comment #75) > (From update of attachment 328807 [details] [diff] [review]) > The stringbundle-specific changes look OK but I didn't look at any of the other > miscellaneous changes as those probably belong in bug 444411. I filed bug 446123 about the additional fixes.
![]() |
||
Comment 82•12 years ago
|
||
Can this bug be closed now that Bug 445371 has been fixed? The only remaining use of strres.js that I can find is test_bug292789.html
Flags: needinfo?(sgautherie.bz)
Assignee | ||
Comment 83•11 years ago
|
||
(In reply to Cykesiopka from comment #82) > Can this bug be closed now that Bug 445371 has been fixed? I prefer to leave this bug open until strres.js itself is removed. > The only remaining use of strres.js that I can find is test_bug292789.html Right. I filed (blocking) bug 909107.
Flags: needinfo?(sgautherie.bz)
Assignee | ||
Comment 84•11 years ago
|
||
Bug 909107 is fixed, then strres.js is no longer used :-) I just filed Bug 916235 - Remove obsolete (and now unused) strres.js from the tree
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Whiteboard: [Waiting for blocking bugs, for now]
Updated•11 years ago
|
Target Milestone: --- → mozilla26
Assignee | ||
Comment 85•11 years ago
|
||
> Target Milestone: --- β mozilla26
Ftr, this bug patches themselves were checked in years before that release.
You need to log in
before you can comment on or make changes to this bug.
Description
•