1344411 - Lightweight themes are not supported anymore in SeaMonkey

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Tony Mechelynck [:tonymec]]

Lightweight themes are not supported anymore in SeaMonkey 2.51a1 (Toolkit 54.0a1), see regression window below. AFAICT, Firefox is not affected.

I'm adding the Toolkit add-ons manager QA contact as CC in case someone there could (and is willing to) point us in the right direction.

Steps to reproduce, variant A:
1. Browse to https://addons.mozilla.org/en-US/seamonkey/themes/
2. Hover your mouse over the theme preview thumbnails.
Expected result:
  At each thumbnail, the browser is temporarily clad with the corresponding theme.
Actual result:
  Nothing happens.

Steps to reproduce, variant B:
1. Install a lightweight theme if necessary, and make sure that it is enabled. This may require a browser restart, depending on whether you previously had some heavyweight theme other than Default.
Expected result:
  The newly enabled lightweight theme is displayed on the browser chrome.
Actual result:
  Only the default theme is displayed, with no lightweight theme visible.

First known bad:
20170303003001
http://hg.mozilla.org/comm-central/rev/e32e7215e0e726eeef616940d4e57b3db70755e6
http://hg.mozilla.org/mozilla-central/rev/b23d6277acca

Last known good:
20170302003001
http://hg.mozilla.org/comm-central/rev/c26182d4b4d9134ad0448178bf98b10d76654795
http://hg.mozilla.org/mozilla-central/rev/e91de6fb2b3d

Comment 1

[Frank-Rainer Grahl (:frg)]

Confirmed under Windows. I suspect Bug 864562

Comment 2

[Frank-Rainer Grahl (:frg)]

Might also affect Thunderbird. Compiling a Daily now.

Comment 3

[Frank-Rainer Grahl (:frg)]

There is already a bug for it in TB: Bug 1343983

Comment 4

[Tony Mechelynck [:tonymec]]

In #seamonkey at the status meeting:
frg >	Tonymec: Patching the bug was easy but half the themes did overwrite the title bar with interesting results so I passed. I think they didn't take a regular titlebar into account with the original bug. Drawing needs to be limited to the inner window. There is a pref for not drawing into it but I ran out of time and can't say I know css really well.

I had a look at https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_variables and I'm interested. Do you have a WIP patch? You could attach it to this bug without ASSIGNing it to you. Then if I feel up to the task I might take it.

Comment 5

[Tony Mechelynck [:tonymec]]

Attachment: WIP patch v. 0.0 — not yet ready for review

I'm tentatively taking this bug for the time being.
I may or may not bring it to completion; if I don't, I'll record my ending point as I reassign it to nobody@

This is my starting point, I'm intentionally recording it as a Mercurial changeset patch in git mode, with frgrahl as the author and yesterday's date, as this is what he gave me. The changes in navigator.css are commented out by him.

There are two copies of each of these style sheets in omni.ja, as follows:
chrome/comm/content/communicator/communicator.css
chrome/comm/content/navigator/navigator.css
chrome/classic/skin/classic/communicator/communicator.css
chrome/classic/skin/classic/navigator/navigator.css

I have made a copy of yesterday's nightly distribution files for reference. I'll be installing the .tar.bz2 somewhere under my home directory in order to leave the /usr/local/seamonkey tree unchanged. For a start, I'll see what I can do by tweaking the style sheets in chrome/comm/content in the omni.ja of this local installation.

I'm not sure I'll get anywhere because in this GTK3 seamonkey, the window decorations, including the titlebar, are provided by Gnome or something, and the #titlebar element has zero height. However, SeaMonkey and its addons (such as Nightly Tester Tools) are able to set the titlebar text.

Comment 6

[Tony Mechelynck [:tonymec]]

No luck attempting to unzip/rezip only those two files.
unzip says:

Archive:  omni.jar
warning [omni.jar]:  19262456 extra bytes at beginning or within zipfile
  (attempting to process anyway)
error [omni.jar]:  reported length of central directory is
  -19262456 bytes too long (Atari STZip zipfile?  J.H.Holm ZIPSPLIT 1.1
  zipfile?).  Compensating...
  inflating: chrome/comm/content/communicator/communicator.css
  inflating: chrome/comm/content/navigator/navigator.css

and successfully extracts the two files.
zip says:

updating: chrome/comm/content/communicator/communicator.css (deflated 74%)
updating: chrome/comm/content/navigator/navigator.css (deflated 69%)

and seems to terminate successfully, but after that, "./seamonkey/seamonkey --no-remote --ProfileManager" crashes before opening the profile manager. The following messages are printed at the (stdout/stderr) console:

ExceptionHandler::GenerateDump cloned child 16767
ExceptionHandler::SendContinueSignalToChild sent continue signal to child
ExceptionHandler::WaitForContinueSignal waiting for continue signal...
Segmentation fault (core dumped)

After replacing omni.ja by the original file (which I had kept) SeaMonkey starts normally, but of course no background images.

Comment 7

[Tony Mechelynck [:tonymec]]

Attachment: patch v1

This works OK in my GTK3 build but I want opinions on other platforms.

To test it without recompiling, do the following (when I give actual command-lines they are for GNU bash as I use it on Linux, but similar results can be obtained with different shells, even with Windows cmd.exe or whatever they're using now, by modifying the commands appropriately):
1. Make sure that you have a Trunk version of SeaMonkey that you can change (for instance a copy of a not-too-old trunk build) installed at a non-default place, for instance, let's say, in ~/testing-lwthemes/ (from there, the executable will be ./seamonkey/seamonkey and the omni.ja will be ./seamonkey/omni.ja
2. Make sure that you have a reasonably fresh profile for testing, and that it contains the following lwthemes:
-- https://addons.mozilla.org/en-US/seamonkey/addon/little-flowers/
-- https://addons.mozilla.org/en-US/seamonkey/addon/eco-theme/
-- https://addons.mozilla.org/en-US/seamonkey/addon/sunset-over-water/
3. Make sure that the browser in 1) is not running and that the profile in 2. is not in use.
4. cd to the parent of the directory where this browser is installed (with the example value shown at step 1, this means: cd ~/testing-lwthemes )
5. Save the existing omni.ja
-- cp -v seamonkey/omni.ja .
6. Make a new, empty, directory into  which the whole omni.ja will be unpacked:
-- mkdir -v omnijar
7. Unzip the omnijar. (My unzip program complains about nonstandard zipfile format but does the job anyway)
-- unzip omni.ja -d omnijar
8. Apply the patch manually to the file ./chrome/comm/content/communicator/communicator.css
-- There are two lines to change, and the first character of every line in the patch is not found in the file.
9. Rezip everything into a _new_ file, e.g.
-- zip -r omni.ja1 omnijar/*
10. Replace the omni.ja the browser will use by this new file
-- cp -v omni.ja1 seamonkey/omni.ja
11. Start the browser with appropriate arguments, for example
-- ./seamonkey/seamonkey --no-remote -P newprofile
   where "newprofile" is the profile mentioned at step 2.
12. The browser should start normally. Now you can enable the lwthemes and see what they look like.

frg: Please post a screenshot if one or more of them look ugly.

Comment 8

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8854379 [details] [diff] [review]
patch v1

Works for the header but the status bar is not updated. If you want a screen shot let me know.

Comment 9

[Tony Mechelynck [:tonymec]]

Attachment: patch v1.1

Still doesn't set the statusbar, but at least the text of the menus, toolbars and tabs is legible, which was not the case before.

I've had weirdness with the statusbar before the bug happened anyway, it wouldn't always be set, or it remained set to the statusbar background-color of a lwtheme whose preview had been set onmouseover.

frg: I propose to have this patch reviewed and checked-in, and leave the statusbar for a followup: what do you think?

Comment 10

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8854450 [details] [diff] [review]
patch v1.1

Tony,

I got the statusbar working. Check the #seamonkey log from today for the fix.

Comment 11

[Tony Mechelynck [:tonymec]]

(In reply to Frank-Rainer Grahl from comment #10)
> Comment on attachment 8854450 [details] [diff] [review]
> patch v1.1
> 
> Tony,
> 
> I got the statusbar working. Check the #seamonkey log from today for the fix.

Thanks, I did, and the change looks promising.

Comment 12

[Tony Mechelynck [:tonymec]]

Attachment: patch v2

- This patch is ready for review. It is teamwork by frg and me, I hope hg.m.o won't reject the double username in the Mercurial header ("hg qrefresh" and "hg qpop" didn't on my clone, but I don't have any user hooks installed).
- Tested on Windows by frg and on GTK3 Linux64 by me. In particular:
  -- Header image is displayed
  -- Footer image is displayed
  -- Contrasting text is displayed over the background images
- Added an appropriate commit message.
- Last-minute correction: fixed a typo, the initial dash was missing in :root:-moz-lwtheme just after the multiline comment (which is inspired by a similar comment in bug 1343983 for Thunderbird).

This last-minute correction may make part of the rest redundant, but now "it works, I don't fix it" — unless of course there are review remarks.

Comment 13

[Frank-Rainer Grahl (:frg)]

Attachment: Capture.PNG

Firefox themes the tabs differently. There is a lot of additonal css for it in mozilla\browser\themes\shared\tabs.inc.css. 

I would probably leave the navigator.css part of it out here and do a followup bug for tabs. Make the text unreadable in some themes as it is now.

Comment 14

[Tony Mechelynck [:tonymec]]

(In reply to Frank-Rainer Grahl from comment #13)
> Created attachment 8854748 [details]
> Capture.PNG
> 
> Firefox themes the tabs differently. There is a lot of additonal css for it
> in mozilla\browser\themes\shared\tabs.inc.css. 
> 
> I would probably leave the navigator.css part of it out here and do a
> followup bug for tabs. Make the text unreadable in some themes as it is now.

I added it because in that sunset-over-water theme the black text was unreadable over a mostly black background. If it's unreadable on the tabs I would expect it to be unreadable on the toolbars and menus, with similar background and similar text colors.

Firefox also has its tabs on top of the chrome by default, unlike we do, IIUC.

Comment 15

[Tony Mechelynck [:tonymec]]

P.S. Your screenshot, with white text and slight black shadow over a dark brown and light orange picture, looks quite nice to me. Without the navigator.css change the text on the tabs would have been black, totally illegible on the left tab.

Comment 16

[Frank-Rainer Grahl (:frg)]

Attachment: Capture1.PNG

In 53 with CTR it looks this way. 

So probably not ideal in any case. Well its a theme and if the user doesn't like it he/she can install another one.

Comment 17

[Tony Mechelynck [:tonymec]]

Attachment: screenshot in patched SeaMonkey 2.52 on Linux64

This is what I see, and I like it. Without the tabbrowser patch the tab text was black and I definitely didn't like it.

Comment 18

[Tony Mechelynck [:tonymec]]

Attachment: patch v2.0.1

This is the exact same patch, but with different Mercurial headers: it is still teamwork of course, but frg had cold feet about being mentioned as co-author.

Comment 19

[Tony Mechelynck [:tonymec]]

Even if we checkin this patch (which I'm in favour of) further testing and possibly a followup may be needed to make sure we get the lwtheme-background-color if the toolbars and/or tabs extend below the lwtheme-background-image. Have to see if the setting of about:config boolean prefs lightweightThemes.persisted.headerURL and lightweightThemes.persisted.footerURL makes a difference.

Comment 20

[Tony Mechelynck [:tonymec]]

P.S. ATM I get a transparent background (and I see the ChatZilla window all the way through the browser chrome) which is not ideal. If anyone can tell me where I goofed the info will be welcome.

Comment 21

[Tony Mechelynck [:tonymec]]

Attachment: screenshot with Kairo's EarlyBlue plus an lwtheme, showing ChatZilla through the browser chrome

My previous tests were made with only the browser window on that particular virtual desktop. Notice ChatZilla showing through at bottom right of the top chrome and through the collapsed sidebar at left, and "Welcome to ChatZilla!" right of the cZ button on the statusbar.

Otherwise the display is what I would expect with my usual userChrome.css, except that below the lwtheme-image the flowing tabs are unusually dark.

Comment 22

[Tony Mechelynck [:tonymec]]

P.S. In the default theme (or in Safe Mode) it still looks OK. With this complete theme + lwtheme, when I expand the sidebar it has a proper non-transparent background. Its grippybar above and below the grippy, but not the grippy itself and not the rest of the sidebar, are transparent.

Comment 23

[Tony Mechelynck [:tonymec]]

@IanN: The remaining problem isn't seen in Default, but only when a lwtheme is used together with a nondefault complete theme, which is nontrivial, as it requires the following operations in the following order:
1. Enable a lwtheme and restart if necessary.
-- The lwtheme appears on top of the default theme.
2. Enable a complete theme and restart.
-- The new theme appears with no lwtheme.
3. In about:config, set lightweightThemes.selectedThemeID to the lwtheme's ID (normally the first id: in lightweightThemes.usedThemes)
-- It seems to do nothing.
4. Go to an lwtheme page at AMO and hover your mouse over a preview.
-- When you move the mouse away, your complete+lightweight themes will be married.

Modern is opaque to lwthemes except under the tab bar so using it with one is unattractive (the game isn't worth the candle IYSWIM).

Comment 24

[Tony Mechelynck [:tonymec]]

Found a complete theme which marries well with lwthemes, with no transparent backgrounds, and which, unlike more recent complete themes, does not kill preferences widgets: MicroMonkey 2.0.27, and it hasn't changed since 4 June 2015! Sets strict version checking but that can be overridden.
https://addons.mozilla.org/en-us/seamonkey/addon/micromonkey/

_And_ it doesn't push itself in all directions like Default does; instead, it leaves most of the real estate for the Content, as IMHO every well-behaved complete theme ought to do.

Sometimes newer is not better.

Comment 25

[Tony Mechelynck [:tonymec]]

Attachment: patch (not for checkin) for manual testing by patching the omni.ja

This patch is not for checkin, it is for the daring souls who want to test an advance copy of this fix. Use it as follows:

1. Install SeaMonkey normally.
2. Unzip its omni.ja into a separate, empty, directory somewhere non dangerous.
3. Download this patch at some other nondangerous position
4. cd to the directory from  step 2, which isn't empty anymore.
5. Apply the patch with -p1, e.g. "patch -p1 omnijar.diff"
6. Rezip the full contents of the current directory.
7. (optional) Make a backup copy of SeaMonkey's original omni.ja
8. Replace SeaMonkey's omni.ja with the new zipfile obtained at step 6.

Comment 26

[Robert Kaiser]

The combination of full themes and lightweight themes was never fully supported, and EarlyBlue or LCARStrek never cared about really supporting combination with lightweight themes.
Also, those my themes will be discontinued soon, once Firefox Nightly does not support real themes any more. I don't care enough for SeaMonkey any more to go through the long and obnoxious work of updating themes just for that product.

Comment 27

[Tony Mechelynck [:tonymec]]

(In reply to Robert Kaiser from comment #26)
> The combination of full themes and lightweight themes was never fully
> supported, and EarlyBlue or LCARStrek never cared about really supporting
> combination with lightweight themes.
> Also, those my themes will be discontinued soon, once Firefox Nightly does
> not support real themes any more. I don't care enough for SeaMonkey any more
> to go through the long and obnoxious work of updating themes just for that
> product.

OK, well after a long love story with EarlyBlue, which used to go well with my lwthemes even if it wasn't planned, I've finally divorced it in favour of MicroMonkey (unchanged since 2015) and the few changes I dislike in it (such as toolbar top & bottom borders even with a lightweight theme) I've set right by means of userChrome.css. Bye-bye, Robert, may you find happiness with Firefox or whatever you're into now and in the future.

Comment 28

[Tony Mechelynck [:tonymec]]

Attachment: (optional) CSS rules I found useful with lightweight themes

These are additional CSS rules which I found useful when using lightweight themes. They are intentionally not included in the patch, and using all, some, or none of them is purely a matter of taste. I started them when lightweight themes were still officially called Personas, hence the language used. The abundant commenting is part of my coding style.

If you want to use some of these rules, copy them into the file userChrome.css in the chrome/ subfolder of your profile, a file which does not exist by default. That file is only read at startup, and it is not used in Safe Mode.

Comment 29

[Ian Neal]

Comment on attachment 8854776 [details] [diff] [review]
patch v2.0.1

r=me LGTM
Did you resolve the ChatZilla issue?

Comment 30

[Tony Mechelynck [:tonymec]]

(In reply to Ian Neal from comment #29)
> Comment on attachment 8854776 [details] [diff] [review]
> patch v2.0.1
> 
> r=me LGTM
> Did you resolve the ChatZilla issue?

Which ChatZilla issue? If you mean the "transparent browser background" mentioned in comment #21, it happens only with some third-party complete themes. Using a complete theme other than Default together with a lightweight theme has never been fully supported, it requires some jumping through hoops, and some complete themes work better than others in this respect.

I've been patching the omni.ja of successive trunk nightlies this way ever since I requested review: there is no problem with the Default theme and there is no problem with MicroMonkey once I disregard its maxVersion. There used to be problems in EarlyBlue but I haven't tested its latest nightly; anyway, it will stop being maintained together with LCARStrek when Firefox stops supporting complete themes.

Comment 31

[Tony Mechelynck [:tonymec]]

Comment on attachment 8854776 [details] [diff] [review]
patch v2.0.1

[Approval Request Comment]
Regression caused by (bug #): bug 864562
User impact if declined: no lightweight theme backgrounds visible
Testing completed (on m-c, etc.): tested by the assignee on Linux and by Frank-Rainer Grahl on Windows, both on comm-central. The assignee has been applying the changes on successive nightlies ever since review was requested, with no serious problems.
Risk to taking this patch (and alternatives if risky): no risk, this change affects only the look and feel. When using lightweight themes together with some (but not all) third-party complete themes, an unsupported procedure, the part (if any) of the browser chrome which exceeds the background image may have transparent background.
String changes made by this patch: none

Comment 32

[Frank-Rainer Grahl (:frg)]

https://hg.mozilla.org/comm-central/rev/9067651a03901baa91feccd66981f63d8ded52a9

Comment 33

[Frank-Rainer Grahl (:frg)]

https://hg.mozilla.org/releases/comm-aurora/rev/509cbae906af8f1ece3a8f68b42ab1232abe6856

Comment 34

[Tony Mechelynck [:tonymec]]

The newly built hourly seems OK to me, but since I'm the assignee I'm not supposed to set VERIFIED even though I'm also the reporter.

I'm listing here where hourlies with the fix can be found:
- 2.52a1
  - Linux64: http://ftp.mozilla.org/pub/seamonkey/tinderbox-builds/comm-central-trunk-linux64/1492365713/
  - Linux32: http://ftp.mozilla.org/pub/seamonkey/tinderbox-builds/comm-central-trunk-linux/1492365713/seamonkey-2.52a1.en-US.linux-i686.txt
  - Mac OS X: n/a (not building)
  - Win32 (and Win64 with WOW64): n/a (not building)
- 2.51a2
  - Linux64: not yet built AFAICT
  - Linux32: http://ftp.mozilla.org/pub/seamonkey/tinderbox-builds/comm-aurora-linux/1492370873/
  - Mac OS X: n/a (not building)
  - Win32 (and Win64 with WOW64): n/a (not building)

For Linux64 on Aurora, you can either watch for the URL in comment #33 (or a later one according to https://hg.mozilla.org/releases/comm-aurora/pushloghtml ) in the .txt (not _info.txt) file accompanying the .tar.bz2, or you can wait for tomorrow's nightly.

Comment 35

[Tony Mechelynck [:tonymec]]

Something went wrong with the flag settings, let's put them back the way they ought to be.

Comment 36

[Tony Mechelynck [:tonymec]]

(In reply to Tony Mechelynck [:tonymec]. (NEEDINFO me if you want my attention) from comment #34)

Oops...
- 2.52a1 on Linux32: http://ftp.mozilla.org/pub/seamonkey/tinderbox-builds/comm-central-trunk-linux/1492365713/

Comment 37

[Tony Mechelynck [:tonymec]]

The 2.51a2 hourly for L64 has been built: http://ftp.mozilla.org/pub/seamonkey/tinderbox-builds/comm-aurora-linux64/1492370873/