Closed Bug 120970 Opened 20 years ago Closed 9 months ago
Break up and clean charset
Overlay JS, XUL for Editor, Mail/News, Browser
Spun off of bug 120275. Our friend timeless required I separate charsetOverlay.js and charsetOverlay.xul for 0.9.9, as a part of clean-up efforts.
This is mine. Whether I want it or not, a promise is a promise...
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.9
Assignee: yokoyama → jscript
Status: ASSIGNED → NEW
What this patch does is it breaks up the reference to the global charsetOverlay.xul into four separate charsetOverlay.xul files. Each charsetOverlay.xul file will have its own charsetOverlay.js file in the same directory. Opinions requested on whether the files should retain the same filenames.
I'm not sure what will be required to check these files into the CVS tree. In the XUL files, I separated the four XUL portions into four different XUL files, removing from each instance the XUL code from the other three. I also broke up the XUL code such that the rows did not have too many characters. In the JS files, I did several things: (1) I preserved functions the corresponding XUL file permits. (2) I preserved functions which those functions might call, and those which might be called later in the chain, etc., etc. (3) I removed any functions not in the chain. (4) I moved functions which other functions call before the functions that would call them. (5) I moved top-level commands to the end of each script. This is all I can do at this point. I do not have checkin access, nor do I have the equipment to test these changes in the various platforms. Would someone please create the code fragment needed to check in these files and remove the global charsetOverlay JS and XUL files?
Attachment #65813 - Attachment mime type: application/octet-stream → application/zip
Based on bz's advice, here's where the files should go imho: chrome\content\editor\charsetOverlay.xul chrome\content\editor\charsetOverlay.js to: http://lxr.mozilla.org/seamonkey/source/editor/ui/composer/content/ chrome://editor/content/ chrome\content\messenger\charsetOverlay.xul chrome\content\messenger\charsetOverlay.js to: http://lxr.mozilla.org/seamonkey/source/mailnews/base/resources/content/ chrome://messenger/content/ chrome\content\messenger\messengercompose\charsetOverlay.xul chrome\content\messenger\messengercompose\charsetOverlay.js to: http://lxr.mozilla.org/seamonkey/source/mailnews/compose/resources/content/ chrome://messenger/content/messengercompose/ chrome\content\navigator\charsetOverlay.xul chrome\content\navigator\charsetOverlay.js to: http://lxr.mozilla.org/seamonkey/source/xpfe/browser/resources/content/ chrome://navigator/content/
cc'ing ftang and nhotta
This has both old and new files. The new ones have line-ending issues that should be addressed before they get checked in.
I've also been warned about keeping up with changes to the original charsetOverlay.js and charsetOverlay.xul. Far be it for me to be the one responsible for bugs marked with the regression keyword. http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/xpfe/global/resources/content/charsetOverlay.js The XUL file hasn't had any changes from the file I built my approach on.
Could someone explain why this is needed? It is not clear from the original report.
timeless has briefly explained to me this would be desirable in case the user wants to install or execute only a few of the various components. For instance, if the user wants to install only the browser, he should not be saddled with the editor or mail/news code for the charset overlays. Hopefully, timeless will speak up to clarify, if I am wrong.
the cost to load this large overlay has to be at least a bit larger than loading just the bit for navigator, so this change should improve navigator slightly. And as mentioned in comment 10 i noted that users who don't install certain components (mailnews, or ideally composer) shouldn't be penalized for their features.
Thanks for the explanations. So overlay loading performance improvement, has that been done for other places? Please make sure to test the menu in browser, composer and mail. Please contact email@example.com and firstname.lastname@example.org how to test the menu.
I understand a new testcase for this bug shall be available soon. However, I shall not. I've had a recent regression in my Internet connection. As in dial-up, limited time. Needless to say, this makes testing very difficult. Will someone please send me an e-mail when the checkin has landed and the revised testcase is available? Also, the checkin still has not received r= or sr=.
I am a dunce. The r= and sr= were waiting on my ability to test. I'm not able, either now or in the foreseeable future, to act on this bug. Would someone else be kind enough to take it off my hands and see it gets proper love? Resetting milestone; no way can this make it in for 0.9.9.
Target Milestone: mozilla0.9.9 → mozilla1.0
You can reassign to me. But I do not plan to do this for moz1.0.
I hope the patch hasn't bit-rotted. Any word whatsoever on a new testcase for this bug? Give me a testcase, and I'll tell you if it passes.
Assignee: jscript → nhotta
Target Milestone: mozilla1.0 → mozilla1.1alpha
Status: NEW → ASSIGNED
Target Milestone: mozilla1.1alpha → mozilla1.2alpha
Code issue, QA -> yokoyama for now.
QA Contact: ruixu → yokoyama
(In reply to comment #16) > I hope the patch hasn't bit-rotted. Just ever so slightly in the intervening seven years, since the overlay's in toolkit instead of xpfe now.
Assignee: nhottanscp → smontagu
Status: ASSIGNED → NEW
QA Contact: tetsuroy → i18n
OS: Windows 98 → All
Hardware: x86 → All
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.