1222818 - Restore about:support UI (Troubleshooting Information) to old appearance by overriding Toolkit's Project Chameleon styles (revert bug 1161156 for SeaMonkey)

Status: RESOLVED FIXED

(SeaMonkey :: Themes, defect)

Description

[rsx11m]

Toolkit made various changes to about: pages within the scope of their "Chameleon" (bug 1097111), mostly mimicking Windows 8-10 appearance. Those are not going well with the SeaMonkey styles at all (see bug 1192276 for an established example). It is my understanding that bug 1189918 and bug 1190465 now enables us to fork those style pages without harming theme developers, thus is should be done.

This specific bug is for reverting the changes in the about:support page and involves reverting general style changes made (fonts, including the fixed size apparently designed for high-resolution screens; button and selector styling. There is already a global aboutSupport.css which inherits from plugins.css, thus it may be necessary to coordinate with bug 1222817 on the Addons Manager (and possibly rendered WFM once this is done).

Note that bug 752333 aims for forking the Troubleshooting page and to use the Thunderbird implementation instead (or a merge of both), thus facilitating debugging of MailNews Core code as well.

Comment 1

[rn10950]

I can take this.

Comment 2

[rn10950]

After closer inspection of the code, I saw aboutSupport.css, which looks like the file behind most of this metrofication. We may be able to fork/override that, and revert most of the toolkit pages back to the old way. I tried overriding aboutSupport.css, and the result is a mixture of the Metro and Regular styles.

Comment 3

[rsx11m]

Ratty, can you help here?

Comment 4

[rn10950]

When I said that aboutSupport.css was behind the Metrofication, I meant global/skin/in-content/common.css. The second part about overriding aboutSupport.css with the old one and getting a mix of the old and the new is correct as it is.

Comment 5

[Philip Chee]

I'm (still) waiting for Neil to review Bug 1022354. Once that lands we can use overrides in the default theme without clobbering third party themes.

Comment 6

[rsx11m]

rn10950: Fixed now, thus go ahead and try again.

Comment 7

[rn10950]

I did some experimenting with that CSS file that's behind most of this project chameleon redesign. I overrode global/skin/in-content/common.css with a null file, and most of the SM about: pages are relatively close to their original appearance. The major exceptions to this would be about:addons, which is a mixture of the old and new styles, and about:config, which is just like the original but lacking background color and with incorrect fonts. What we may be able to do is file a new bug to override common.css with a null file, and set this, about:config (bug 1222816) and about:addons (bug 1222817) as dependencies of that new bug. We should be able to copy over the old CSS files for these files from 2.33.1 and override the current ones without any issues. What do you guys think?

Comment 8

[Philip Chee]

Neil and I were talking about overriding global/skin/in-content/common.css with communicator/skin/communicator.css . Could you give that a try?

Comment 9

[rsx11m]

... and if it works but requires more work for the individual pages, maybe indeed spin it off as a separate bug /after/ you've verified that it does the job.

Comment 10

[rn10950]

I overrode in-content/common.css with communicator.css, and after restoring aboutSupport.css, it fixed this issue, along with about:config. The only thing it makes worse is the add-ons manager, but we should be able to restore the pre-chameleon CSS for that.

Comment 11

[rsx11m]

If you go this way, let's make sure that bug 1222817 is fixed to the extent necessary before or when checking in the common.css override (here or in a spin-off bug).

Comment 12

[Philip Chee]

So we have to fix the addon manager first?

Comment 13

[rn10950]

(In reply to Philip Chee from comment #12)
> So we have to fix the addon manager first?

Yes.

Comment 14

[Tony Mechelynck [:tonymec]]

@rn10950: The way to indicate which branches are affected is to set the Tracking flags. IIUC bug 1161156 for Firefox about:support was declared FIXED for Firefox 41 on 2015-05-18 (bug 1161156 comment #12); that would mean that the effects were first felt in SeaMonkey 2.38, which is of course beyond fixing. I'm using the Whiteboard as a fallback for tracking flags "too old" to be set anymore.

Comment 15

[rn10950]

(In reply to Tony Mechelynck [:tonymec] from comment #14)
> @rn10950: The way to indicate which branches are affected is to set the
> Tracking flags. IIUC bug 1161156 for Firefox about:support was declared
> FIXED for Firefox 41 on 2015-05-18 (bug 1161156 comment #12); that would
> mean that the effects were first felt in SeaMonkey 2.38, which is of course
> beyond fixing. I'm using the Whiteboard as a fallback for tracking flags
> "too old" to be set anymore.

Yeah, sorry. I misclicked and submitted the changes by accident and by the time I realized what I've done, it was too late.

Comment 16

[Tony Mechelynck [:tonymec]]

(In reply to rn10950 from comment #15)
> Yeah, sorry. I misclicked and submitted the changes by accident and by the
> time I realized what I've done, it was too late.

Happens to all of us, no prob. :-)

Comment 17

[Philip Chee]

Taking.

Comment 18

[Philip Chee]

Attachment: v1.0 Proposed fix.

aboutSupport.xhtml isn't an XUL file so we can't use XUL overlays or style directives from *.manifest's.

Strategy:
1. Override chrome://global/skin/aboutSupport.css with
   chrome://communicator/skin/aboutSupport.css

2. chrome://global/skin/aboutSupport.css imports
   chrome://communicator/skin/common.css

3. chrome://communicator/skin/common.css contains overrides for
   chrome://global/skin/in-content/common.css
It is impractical to override this file directly as it is large and constantly changing. Our common.css contains only the overrides necessary for this bug. Further additions will be made for other about: pages I plan to restyle (like about:addons).

> suite/themes/classic/communicator/aboutSupport.css
This is essentially the pre-chameleon aboutSupport.css but adjusted for the changes in the UI, and with the colours moved to CSS variables in communicator/skin/common.css.

> suite/themes/classic/communicator/common.css
Overrides for in-content/common.css.

> @import url("chrome://communicator/skin/communicator.css");
> @import url("resource://gre-resources/forms.css");
> @import url("resource://gre-resources/html.css");
in-content/common.css override many rules from forms.css and html.css. Importing these stykesheets again reasserts their priority.

> *|*:root {
>   --in-content-page-color: -moz-DialogText;
>   --in-content-page-background: -moz-Field;
>   --in-content-text-color: -moz-DialogText;
>   --in-content-selected-text: HighlightText;
>   --in-content-header-border-color: #c8c8c8;
Override CSS variables from in-content/common.css
Not all hard coded colours have been replaced by system colours. Only those used by aboutSupport.

> suite/themes/modern/global/aboutSupport.css
Update Modern for changes in aboutSupport.xhtml.

> suite/themes/modern/global/plugins.css
stefanh made modern aboutSupport.css import from plugins.css

Comment 19

[rsx11m]

Attachment: Screenshot Linux (GTK3/KDE4)

Looks almost good. I'm happy with Modern, and Classic on the Windows Classic desktop theme. Also, the fonts are fine in all cases.

Some concerns with Classic on Windows 7 Aero theme and Linux on KDE4. Those show a rather flat styling of the buttons with rectangular shape, additionally on Linux the highlight on hover doesn't work (stays the same).

Ideally, the shape of the underlying desktop theme should be taken here. I don't know how this is inherited by the application's theme, but it works for other buttons like those in the Preferences dialog.

>+++ b/suite/themes/classic/communicator/aboutSupport.css
>+html {
>+  background-color: var(--in-content-page-background);

>+table {
>+  background-color: var(--in-content-table-background);

>+++ b/suite/themes/classic/communicator/common.css
>+  --in-content-page-background: -moz-Field;
>+  --in-content-table-background: -moz-Dialog;

That's a bit of an issue with the KDE4 default theme on Linux, where apparently -moz-Field and -moz-Dialog are the same shade of grey. This looks rather dull. Should --in-content-page-background maybe be replaced with the general background color for pages, thus white in most cases?

Comment 20

[Philip Chee]

> Some concerns with Classic on Windows 7 Aero theme and Linux on KDE4. Those
> show a rather flat styling of the buttons with rectangular shape,
> additionally on Linux the highlight on hover doesn't work (stays the same).

> Ideally, the shape of the underlying desktop theme should be taken here. I
> don't know how this is inherited by the application's theme, but it works
> for other buttons like those in the Preferences dialog.

Unfortunately aboutSupport.xhtml uses/inherits styling from us.css/forms.css etc. While the Preferences dialog is XUL and inherits from xul.css. I think in modern I @import url("chrome://global/skin/button.css"); to make the buttons look more Modern-like.

> >+++ b/suite/themes/classic/communicator/aboutSupport.css
> >+html {
> >+  background-color: var(--in-content-page-background);
> 
> >+table {
> >+  background-color: var(--in-content-table-background);
> 
> >+++ b/suite/themes/classic/communicator/common.css
> >+  --in-content-page-background: -moz-Field;
> >+  --in-content-table-background: -moz-Dialog;
> 
> That's a bit of an issue with the KDE4 default theme on Linux, where
> apparently -moz-Field and -moz-Dialog are the same shade of grey. This looks
> rather dull. Should --in-content-page-background maybe be replaced with the
> general background color for pages, thus white in most cases?

Is there a system colour for this? Does color: window; work?
(Phil doesn't have KDE4/Linux)

Comment 21

[Philip Chee]

> Is there a system colour for this? Does color: window; work?
What does color: window; look like on KDE4?

https://developer.mozilla.org/en/docs/Web/CSS/color_value

> -moz-default-background-color
>     User's preference for the document background color.
What does -moz-default-background-color look like on KDE4?

Comment 22

[rsx11m]

Comment on attachment 8763660 [details] [diff] [review]
v1.0 Proposed fix.

Weird, I'm getting consistently gray background for that page, regardless of what I'm doing.

>+++ b/suite/themes/classic/communicator/common.css
>+*|*:root {
>+  --in-content-page-color: -moz-DialogText;
>+  --in-content-page-background: -moz-Field;
>+  --in-content-text-color: -moz-DialogText;

So, I've changed "--in-content-page-background: -moz-Field;" to "--in-content-page-background: window;" and "--in-content-page-background: -moz-default-background-color;" with no change in appearance.

Looking at how it's used again,

>+++ b/suite/themes/classic/communicator/aboutSupport.css
>+html {
>+  background-color: var(--in-content-page-background);
>+  color: -moz-FieldText;
>+  font: message-box;
>+}

This modifies the outermost <html> element. It appears to me that it should rather be defined here?

>+body {
>+  width: 90%;
>+  margin-left: 5%;
>+  margin-right: 5%;
>+}

What happens if you try this on Windows with an extreme like "--in-content-page-background: #ff0000;"?
Apparently the <html> definition is overridden somewhere (<body> or elsewhere) which was missed?

Alternatively, I'm doing something stupid, but I've unapplied and reapplied your patch completely, used a fresh profile, etc. I don't have the time for a clobber build right now, but that may not be necessary. Those files are definitely visible in dist/bin (which are symbolic links only on Linux).

> What does -moz-default-background-color look like on KDE4?

In a regular content window, it shows up as white with my desktop settings.

Comment 23

[rsx11m]

Comment on attachment 8763660 [details] [diff] [review]
v1.0 Proposed fix.

>+++ b/suite/themes/classic/communicator/common.css

>+html|html,
>+xul|page,
>+xul|window {
>+  font: message-box;
>+  /*-moz-appearance: window;*/
>+  background-color: var(--in-content-page-background);
>+  color: var(--in-content-page-color);
>+}

That's the problem. If I comment out -moz-appearance: window; as shown, the background turns white as desired without changing the value for --in-content-page-background. Thus, either it should be removed here or reverted/adjusted in aboutSupport.css.

Comment 24

[rsx11m]

I've looked at an old SM 2.24 build, and the background of the Troubleshooting page there is white as well, so this matches the historic appearance. Where is the -moz-appearance: window; coming from, I didn't see it in any older style sheets?

(In reply to Philip Chee from comment #20)
> Unfortunately aboutSupport.xhtml uses/inherits styling from us.css/forms.css
> etc. While the Preferences dialog is XUL and inherits from xul.css.

Based on the experience that -moz-appearance seems to be very aggressively overriding other styles, maybe -moz-appearance: button; could help here getting a bit of the underlying OS's styling?

Comment 25

[rsx11m]

Comment on attachment 8763660 [details] [diff] [review]
v1.0 Proposed fix.

(In reply to rsx11m from comment #24)
> -moz-appearance: button;

Well, the brute-force approach of simply adding this to the button rule in aboutSupport.css didn't do the trick, thus apparently not *that* straight-forward. Too bad.

So, I'm fine with the styling (with comment #23 addressed), but I'm open to any ideas how to improve the buttons to match the desktop/platform better if possible.

Comment 26

[Ian Neal]

Comment on attachment 8763660 [details] [diff] [review]
v1.0 Proposed fix.

This looks a good starting point, any minor issues can be dealt with in follow-ups r=me

Comment 27

[Philip Chee]

Pushed to comm-central: http://hg.mozilla.org/comm-central/rev/119ebe23dce8

Comment 28

[rsx11m]

Do we want this on the branches or just let it ride the release train?

Comment 29

[Philip Chee]

(In reply to rsx11m from comment #28)
> Do we want this on the branches or just let it ride the release train?
Cosmetic issues -> Normally I'd say: Ride the trains unless we want to get this into ESR??

Comment 30

[rsx11m]

Unless you are talking 45 ESR (which would be SM 2.42), the next branch is 52 ESR (thus 2.49).
This landed for SM 2.48, thus should be fine for that route.