Closed Bug 399571 Opened 12 years ago Closed 12 years ago
Is communicator part of toolkit?
As it stands, toolkit, and therefore presumably XULRunner, ships a stub communicator skin. Why? If it's necessary for toolkit to provide it, can a XULRunner app reliably override this stub skin with a real skin? If so, how?
The files in http://mxr.mozilla.org/mozilla/find?string=stripe%2Fcommunicator&tree=mozilla do really sound like a bug in toolkit to me, IMHO they just should go away...
Judging by usage (in PKI, in mailnews, in Bugzilla's sidebar template, in XUL Planet tutorials, and in random googleable things) I assumed that chrome://communicator/skin/ was at one time what's now chrome://global/skin/, and that providing it was part of anything that consumed XUL, and thus, yeah, part of toolkit. But maybe I'm misunderstanding the question.
I believe the idea was that it defined styles shared by communicator components such as the sidebar (and its panels, including bookmarks and history), taskbar, preferences, or the primary toolbar.
Yes, global/ was always there in addition to communicator/ - from how I understand it, global/ was always for stuff that applies to any XUL anywhere, where communicator/ was meant to handle stuff that is used for the whole suite but might not make sense in other XUL-based applications. With the sparse use of XUL outside of the suite in earlier years, the distinction of that was sometimes not that visible and not even respected in some cases, but nowadays, communicator/ should in theory be SeaMonkey-only - and we're actually not far from that in practice.
Actually, since both *stripe communicator.css just include chrome://global/skin, I think that their existence in toolkit is just lazyness and should get removed. ;-) (Maybe DOMI/Venkman/Chatzilla need to be fixed.)
http://mxr.mozilla.org/mozilla/source/security/manager/pki/resources/content/crlManager.xul#41 http://mxr.mozilla.org/mozilla/source/extensions/sroaming/resources/content/prefs/firefox.xul#40 (plus several more in sroaming, though I wouldn't be surprised if it doesn't work these days) http://mxr.mozilla.org/mozilla/source/mailnews/compose/resources/content/MsgAttachPage.xul#43 http://mxr.mozilla.org/mozilla/source/mailnews/base/search/resources/content/CustomHeaders.xul#39 http://mxr.mozilla.org/mozilla/source/extensions/layout-debug/ui/content/layoutdebug.xul#53 So, say you fix those, plus the ones I'm too lazy to copy-paste, plus the ones in mail/ that I'll fix. Then... http://mxr.mozilla.org/mozilla/source/webtools/bugzilla/template/en/default/sidebar.xul.tmpl#27 means you turn the blue links on grey of however many hundred bugzillas there are to black underlined words on white in Firefox, and do who knows what to how many other sidebar panels and XUL apps written over the last seven years of that working, right or wrong, in every XUL consuming application. So, is this targetted at Moz2, where the whole world's supposed to break anyway?
Uh, I haven't made myself clear enough. What I meant is "Since both *stripe communicator.css just include chrome://global/skin/, I think all occurences of chrome://communicator/skin/ should get replaced by chrome://global/skin/ whereever that isn't already specified." I don't see why this should break anything? (I don't know how many 3rd party themes overlay/use chrome://communicator/skin/ for toolkit, but they'd just be plain wrong in doing so...)
(In reply to comment #7) >Uh, I haven't made myself clear enough. What I meant is "Since both *stripe >communicator.css just include chrome://global/skin/, I think all occurrences of >chrome://communicator/skin/ should get replaced by chrome://global/skin/ >wherever that isn't already specified." Right, but how do we fix third-party occurrences? (In reply to comment #6) >... means you turn the blue links on grey of however many hundred bugzillas >there are to black underlined words on white in Firefox... Firefox is of course free to provide its own stub communicator skin. I just don't think toolkit should do it.
Could we use chrome overrides instead?
@import is more efficient.
Comment on attachment 287303 [details] [diff] [review] Proposed patch sr=me, but you'll still need moa for the browser changes.
Attachment #287303 - Flags: review?(mscott) → review+
Comment on attachment 287303 [details] [diff] [review] Proposed patch sold, r=mano.
Attachment #287303 - Flags: review?(mano) → review+
Don't you need to do the same thing for gnomestripe, too, now that it has landed?
Assignee: nobody → neil
(In reply to comment #14) >Don't you need to do the same thing for gnomestripe No; both in toolkit and browser, gnomestripe is winstripe plus a few overrides.
(In reply to comment #15) > (In reply to comment #14) > >Don't you need to do the same thing for gnomestripe > No; both in toolkit and browser, gnomestripe is winstripe plus a few overrides. No, it's only an override in toolkit. It's a separate theme in browser.
Ah, I see now, the browser version of the makefile is confusingly written. (rev 1.5 works for me)
Assuming r=mano,mscott still holds with this trivial change.
Fix checked in, except for a couple of erroneous Makefile changes backed out.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
This bug will cause some grief for XULRunner apps that use extensions like DOMi and Venkman (and any others that use chrome://communicator/skin/ instead of chrome://global/skin/) I'll make a note on MDC that such XULRunner apps should create a fake "communicator" skin that just imports the "global" skin
...or you could just fix them to use global instead, you know...
You need to log in before you can comment on or make changes to this bug.