Is communicator part of toolkit?

RESOLVED FIXED

Status

RESOLVED FIXED
11 years ago
3 years ago

People

(Reporter: neil, Assigned: neil)

Tracking

Dependency tree / graph

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

11 years ago
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?

Comment 1

11 years ago
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.
(Assignee)

Comment 3

11 years ago
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.

Comment 4

11 years ago
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.

Comment 5

11 years ago
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?

Comment 7

11 years ago
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...)
(Assignee)

Comment 8

11 years ago
(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.
Depends on: 402382
(Assignee)

Comment 9

11 years ago
Created attachment 287303 [details] [diff] [review]
Proposed patch
Attachment #287303 - Flags: review?(mscott)
Attachment #287303 - Flags: review?(mano)
Could we use chrome overrides instead?
(Assignee)

Comment 11

11 years ago
@import is more efficient.

Comment 12

11 years ago
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
(Assignee)

Comment 15

11 years ago
(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.
(Assignee)

Updated

11 years ago
Attachment #287303 - Flags: approval1.9?
(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.
(Assignee)

Comment 17

11 years ago
Ah, I see now, the browser version of the makefile is confusingly written.
(rev 1.5 works for me)
(Assignee)

Comment 18

11 years ago
Created attachment 288887 [details] [diff] [review]
Updated for gnomestripe

Assuming r=mano,mscott still holds with this trivial change.
Attachment #287303 - Attachment is obsolete: true
Attachment #288887 - Flags: review+
Attachment #288887 - Flags: approval1.9?
Attachment #287303 - Flags: approval1.9?

Updated

11 years ago
Attachment #288887 - Flags: approval1.9? → approval1.9+
Version: unspecified → Trunk
(Assignee)

Comment 19

11 years ago
Fix checked in, except for a couple of erroneous Makefile changes backed out.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Depends on: 404292
Depends on: 404294
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
(Assignee)

Comment 21

11 years ago
...or you could just fix them to use global instead, you know...
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.