700184 - Add mozapps/extensions/selectAddons.css to Modern

Status: RESOLVED WONTFIX

(SeaMonkey :: Themes, defect)

Description

[Jens Hatlak (:InvisibleSmiley)]

Bug 596343, which landed for FF 8, introduced a new CSS file to mozapps (selectAddons.css). This file is only used by selectAddons.xul, which is supported starting with SM 2.5 (unfortunately it's too late to fix that version). Modern needs a copy for the new functionality to work correctly with the alternate theme.

STR:
1. Update SM to 2.5 or later, or open the following in a browser window:
chrome://mozapps/content/extensions/selectAddons.xul

Comment 1

[Justin Wood (:Callek)]

Attachment: Copy winstripe version (stripping Aero ifdefs)

I'm far from a good themer, but in interests of making sure this works fine in modern I copied the winstripe version of this file.

I am not interested in doing color-changes here, unless ones in this file make this dialog unusable.

Someone else is free to take this from me, but the only reason I even bother is so we can try to get this into 2.6 (since this dialog is only ever run, ONCE per profile)

Comment 2

[Jens Hatlak (:InvisibleSmiley)]

Attachment: Modern'ized version

Actually there were so many things that I didn't like about the Winstripe copy that I created a patch on top of that, but since I ended up in a half-rewrite, I think it's OK to call this mine now and provide a full patch. ;-)

Neil, if you can, please give this review priority since it's scheduled for inclusion into SM 2.6 still.

I tried to align the visual appearance with the Sync wizard layout (Tools/Set Up Sync). I wasn't able to fully match it since the wizard has a separator at the bottom while this screen doesn't. Still I think it looks much more integrated and "Modern" now that before.

Please note that I'll likely not be able to answer "do we need this-or-that" questions on short notice. ;-)

Comment 3

[[e-mail]]

Comment on attachment 580654 [details] [diff] [review]
Modern'ized version

Well, I'd just finished reviewing this when Mossop turned up on IRC and confirmed that the dialog only appears early on in startup before any of the extensions and themes have been loaded. So there's no point theming this.

(Detail: Themes are loaded by LoadExtensionDirectories, which is called by LoadExtensionBundleDirectories, which is called by nsXREDirProvider::DoStartup just after the Addons Manager shows the select addons dialog.)

>+  border: none;
Unnecessary.

>+  margin: 0;
Nit: 0px (etc.; I won't point all the lone zeros out)

>+  color: #ffffff;
Nit: #FFFFFF

>+  border-top: 1px solid #D6E5F5;
#DFD5EF or possibly #E0ECF6 (etc.)

>+#select-grid label {
>+  margin-top: 0;
>+  margin-bottom: 0;
Seems to be unnecessary.

>+  -moz-box-sizing: border-box;
Unnecessary (etc.; see xul.css)

>+  width: 13px;
Doesn't seem to work, fortunately.

>+  margin: 0;
>+  padding-top: 1px;
>+  padding-bottom: 2px;
>+  -moz-padding-start: 6px;
>+  -moz-padding-end: 5px;
I don't know what they've done to make it necessary to use padding instead, but Modern defaults to margin: 1px 5px 2px;

>+  color: #85959E;
#8C99AB
Tip: MXR a colour before you use it.

>+  color: #85959E;
This should be #8C99AB too. (There is a use in MXR; that's wrong too.)

Comment 4

[Jens Hatlak (:InvisibleSmiley)]

(In reply to [e-mail] from comment #3)
> Well, I'd just finished reviewing this when Mossop turned up on IRC and
> confirmed that the dialog only appears early on in startup before any of the
> extensions and themes have been loaded. So there's no point theming this.

D'oh! Sorry I wasted your time. I should have better checked before filing this bug.

Revised STR (example, details may vary):
1. Launch SM 2.4
2. Switch theme to Modern, but don't restart yet
3. Install Firebug or some other third-party extension
4. Restart SM (Firebug and Modern should be installed now), then quit SM
5. Launch SM 2.8a1 using the same SM profile as above.

What happens with the above STR is that the add-on selection screen (selectAddons.xul) appears, themed using the default theme. Then, after submission, SM appears using Modern.

*Question:* Shall we create a dummy CSS file including some explanation, or just leave it and close this bug right away?

In any case I'll be so bold and reset the tracking flags since this doesn't deserve any special attention anymore either way.

Comment 5

[Tony Mechelynck [:tonymec]]

If there's no way this dialog can be themed, then WONTFIX? Or perhaps user-doc-needed?

Comment 6

[Jens Hatlak (:InvisibleSmiley)]

I don't think this needs documentation, at least not outside the source. My question was only whether we want to add a same-named file to Modern that explains why we don't need it. Hmm, thinking about that, maybe this bug is really all we need, so I'm really not opposed to just WONTFIX this, but I'll leave that to Callek. Anyway, I'm not actively working on anything here anymore -> NEW for now.

Comment 7

[Jens Hatlak (:InvisibleSmiley)]

Callek, ping re comment 6 (or comment 4, really): The file discussed in this bug is not themable (cf. beginning of comment 3). Shall we add a dummy file for Modern explaining that, or do nothing and close this bug?

Comment 8

[Justin Wood (:Callek)]

Sorry didn't realize the ball was in my court here.

WONTFIX due to it being loaded so early in startup that modern themeing is irrelevant