Closed Bug 548189 Opened 11 years ago Closed 11 years ago

Remove embedding/components/ui/

Categories

(Core Graveyard :: Embedding: GRE Core, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9.3a3

People

(Reporter: philor, Assigned: philor)

References

Details

Attachments

(2 files, 2 obsolete files)

There are two things in components/ui/, which have been idly puzzling me (from my "interested in packaging, but not so much in SeaMonkey packaging" perspective) for years.

progressDlg contains the SeaMonkey-only download progress dialogs (though the strings are in toolkit/), with XUL that's jarred up for every app even though only SeaMonkey uses it, and JS that's dropped in dist/bin/ for every app, even though Firefox doesn't package it (Thunderbird does, but only because we clean our plate, even if we're already stuffed full). It should move to suite/, so we can stop shipping unused things, stop making locales that aren't doing SeaMonkey translate strings that they can never see in action, and so it can be with a loving family instead of distant parents who quite frankly thought it died years ago.

helperAppDlg contains XUL which is jarred up for every app, even though no app uses it, and JS which is dropped in dist/bin/, where we're very fortunate that it is then overwritten by a file with the same name from toolkit/, because the JS is totally broken, trying to open an app picker which does not exist at all, but was suite-only at the end of its life. It should be taken out behind the barn and... um... "sent to a better world."
Could have saved myself some trouble if I looked where I was going to put it in suite/ first, since I see that SeaMonkey's current progress dialogs don't actually come from this, so the whole thing is dead.
Attached patch comm-central bits (obsolete) — Splinter Review
Give it a broad look - I did make sure progress dialogs and the download manager still work, but I don't know new or old SeaMonkey well enough to say for sure I didn't miss some dependency.
Attachment #428656 - Flags: review?(neil)
Attached patch mozilla-central bits (obsolete) — Splinter Review
No, dancing on the grave of some of those no-l10n-note strings wouldn't be unseemly, not in the least.
Attachment #428657 - Flags: review?(benjamin)
Blocks: 547915
Comment on attachment 428656 [details] [diff] [review]
comm-central bits

Don't we need to add it to removed-files.in? Also, doesn't this break Sunbird?
I slightly wonder if FTP upload does use the old progress dialogs or if we have broken that some time in a different way. The code of those old progress dialogs is ugly enough that I really love to see it go, though.
(In reply to comment #5)
> I slightly wonder if FTP upload does use the old progress dialogs or if we have
> broken that some time in a different way. The code of those old progress
> dialogs is ugly enough that I really love to see it go, though.
Sadly FTP upload was broken for some other reason, and then it broke again before I fixed the first reason, so it's still broken, and I forgot about it :-(
(In reply to comment #6)
> Sadly FTP upload was broken for some other reason, and then it broke again
> before I fixed the first reason, so it's still broken, and I forgot about it
> :-(

Looks like we should file an own bug for that, and probably write an own implementation of upload progress, possibly based on the new download progress dialog (as long as I don't have to touch that code myself).
Someday I'll remember removed-files.in the first time. Someday.

I certainly don't understand XPCOM, but my impression was that since the IDL has completely disappeared, Cc["@mozilla.org/progressdialog;1"].createInstance(Ci.nsIProgressDialog) would be failing (both in your FTP upload, and in the unused doDebug method in the toolkit helper app dialog that I missed), so removing the implementation of an interface that doesn't exist couldn't actually break anything.
Attachment #428656 - Attachment is obsolete: true
Attachment #428759 - Flags: review?(neil)
Attachment #428656 - Flags: review?(neil)
And the summary of various IRC conversations is that SeaMonkey's FTP upload does use this nsProgressDialog, and adding back the removed nsIProgressDialog.idl does unbreak it, and Neil and KaiRo are in agreement that they should put the IDL back in 1.9.1, but only KaiRo has thus far expressed the desire to remain broken on the trunk pending a rewrite, rather than have this foul code moved into suite/, far as I know.

I *think* the only way that affects me is whether or not you want me to removed-files the existing nsProgressDialog.js while you wait to see what you're going to do on the trunk - since it's a cross-repo move, you can just as easily copy this foul code from somewhere other than "currently in the tip of mozilla-central" at any time, whether or not I remove it first.
One added bit, since the toolkit nsHelperAppDlg.js was under the mistaken impression that trying and failing to open a progress dialog somehow amounted to "doDebug".
Attachment #428657 - Attachment is obsolete: true
Attachment #428866 - Flags: review?(benjamin)
Attachment #428657 - Flags: review?(benjamin)
Attachment #428759 - Flags: review?(neil) → review+
Attachment #428866 - Flags: review?(benjamin) → review+
http://hg.mozilla.org/mozilla-central/rev/269d2db8fbf3
http://hg.mozilla.org/comm-central/rev/2636b695ac2b
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a3
This fixes bug 288800, bug 341063, bug 535035. Great job!
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.