Closed Bug 752331 Opened 8 years ago Closed 5 years ago

Define TopLevelImageDocument.css and TopLevelVideoDocument.css for SeaMonkey themes

Categories

(SeaMonkey :: Themes, defect)

defect
Not set

Tracking

(seamonkey2.10 unaffected, seamonkey2.11 affected, seamonkey2.12 affected, seamonkey2.33 fixed)

RESOLVED FIXED
seamonkey2.33
Tracking Status
seamonkey2.10 --- unaffected
seamonkey2.11 --- affected
seamonkey2.12 --- affected
seamonkey2.33 --- fixed

People

(Reporter: rsx11m.pub, Assigned: philip.chee, Mentored)

References

Details

(Keywords: modern)

Attachments

(4 files, 3 obsolete files)

Toolkit introduced styling to stand-alone image content with bug 376997, and to video content earlier. The background is hard-coded and may not go well with desktop and/or application themes, especially with image transparency. There is no visible effort or support from the Toolkit side to make either configurable or accessible without needing a separate extension.

Per bug 713487, the color and background definitions are now moved into the themes. This becomes effective with SM 2.11 (Gecko 14.0). SeaMonkey's default theme uses the Toolkit style sheets, but the modern theme throws an error that modern@themes.mozilla.org.xpi!/chrome/modern/skin/modern/global/TopLevelImageDocument.css cannot be found.

The charge for this bug is to define those style sheets for both SeaMonkey themes and to revisit the color and background definitions to fit better SeaMonkey's appearance. Personally, I think that the default (usually white) background is most neutral and may fit best for most cases, and I never understood the meaning of the "noise" introduced by the background pattern, but that's certainly up for discussion.
Version: unspecified → Trunk
Keywords: modern
Whiteboard: [good first bug][mentor=Ratty][lang=css]
i would like to contribute for this bug. Will you explain how should i proceed towards the bug ?
Hello Amod,

First thing is to get SeaMonkey building. Please read:
https://developer.mozilla.org/en-US/docs/Simple_SeaMonkey_build
By the way which platform are you going to use for your development? Windows, OSX, or Linux?
i have built the entire mozilla central source code in Linux (Ubuntu). So preferably i would use Ubuntu only.
Great! Once you get SeaMonkey building you can copy/port the two files:
TopLevelImageDocument.css
TopLevelVideoDocument.css
From mozilla-central winstripe to

http://mxr.mozilla.org/comm-central/source/suite/themes/modern/global/
Assignee: nobody → amod.narvekar
Status: NEW → ASSIGNED
ok Sir. Can you tell me where i can meet you at IRC. So that i can ask you my doubts there.
(In reply to Amod from comment #5)
> ok Sir. Can you tell me where i can meet you at IRC. So that i can ask you
> my doubts there.

With ChatZilla (or SeaMonkey, but not in Safe Mode):
irc://moznet/seamonkey

With other clients: connect to irc://irc.mozilla.org:6667/ or ircs://irc.mozilla.org:6697/ then /join #seamonkey
Additional changeset to be aware of:
Bug 793366 - Add noise to the white background used for transparent images in image documents.
Hello Amond. Are you still working on this?
Flags: needinfo?(amod.narvekar)
yes. Having many problems on the SeaMonkey Build and trying to solve it. Sorry for the delay and i request you to give me some more time.
Flags: needinfo?(amod.narvekar)
sudo apt-get build-dep seamonkey

http://pastebin.mozilla.org/1868513

its giving error.


sudo apt-get install mercurial libasound2-dev libcurl4-openssl-dev libnotify-dev

it is working but halting forever on the font installer console.
Thanks for the update. I'll try to get someone to help you install the perquisites.
In reply to comment #10:
I'm not on Ubuntu and manage my packages with other than sudo apt-get. Software management software is one of the things which varies most from one distro to the next.

If the error is during the apt-get run, maybe try to install one package at a time (or a few at a time) to see where the problem is?

What is "the font installer console"?
Amod: Maybe https://developer.mozilla.org/en-US/docs/Developer_Guide/Build_Instructions/Linux_Prerequisites#Ubuntu could be helpful to you.

You say you have built the entire mozilla-central source on Ubuntu. To build SeaMonkey, you may have to move or remove your mozilla-central repository (especially if you don't have huge gobs of disk space): building SeaMonkey requires cloning no fewer than six repositories, with mozilla-central one of them at a specific location under the comm-central repo, as follows:

top (comm-central repo)
  |           |
  |        ldap/sdks (LDAP SDK repo)
  |
mozilla (mozilla-central repo)
  |
mozilla/extensions
  |     |      |
  |     |    mozilla/extensions/irc (Chatzilla repo)
  |     \--  mozilla/extensions/inspector (DOMi repo)
  \--------  mozilla/extensions/venkman (Venkman repo)

The first step in the SeaMonkey build process ("python client.py checkout") invokes hg twelve times (a pull and an update for each repo) to keep them all up-to-date at their proper directory locations.
but which commands should i use to install correctly !!!
(In reply to Amod from comment #14)
> but which commands should i use to install correctly !!!

cd to the parent of where you want your comm-central clone to live, and then:

hg clone -U http://hg.mozilla.org/comm-central/
cd comm-central
hg pull -r default
python client.py checkout

The above should install all 6 repositories correctly and up-to-date. If you omit the -U switch in the first command you can also omit the hg pull (but not the cd), at the risk of getting a timeout during the cloning process.

You may then (or even juts before it, if you are short of HD space) remove your existing mozilla-central clone, because you will have one at comm-central/mozilla

Once you have this, the following set of commands (the stdout and stderr of which it is recommended to log) should produce an installable .tar.bz2:

    (in the comm-central directory)
python client.py checkout
make -f client.mk build

    (in your objdir, which should be set in your mozconfig)
make package

If any of these three phases fails (ends with a nonzero status code) you should of course not run the rest.

Note also that at the moment, SeaMonkey is not building on trunk, see http://tinderbox.mozilla.org/showbuilds.cgi?tree=SeaMonkey&hours=36 — so you may want to wait until the column corresponding to your platform goes green again.
P.P.S. I have Linux but not Ubuntu. Maybe you can get better help than I can give you by asking on the mozilla.dev.apps.seamonkey newsgroup. This newsgroup is not found on your ISP's news server (if any) but on news.mozilla.org, and it is also mirrored on (among others) Google Groups.
(In reply to Tony Mechelynck [:tonymec] from comment #16)
> P.S. You should still install the required packages before compiling. Did
> you read
> https://developer.mozilla.org/en-US/docs/Developer_Guide/Build_Instructions/
> Linux_Prerequisites#Ubuntu ?

i have read and installed the prerequisites already.
i am going the work..
Currently i have exams till 3rd, so will continue the work after that. Thanks !
Hello, I am back...If you are not comfortable with Ubuntu...shall i shift to Windows...since i will be requiring your help and guidance very much...So i wont like to keep you waiting for long and that too because of our difference in Operating System.
> shall i shift to Windows
Depends. Did you manage to get SeaMonkey building correctly on Ubuntu? If not then try Windows.
19:08	greatwarrior	Mnyromyr: actually i wanted to convey that i will be continuing the bug after my exam gets over..
19:09	Mnyromyr	"pleased to hear that you will continue" :)
19:10	greatwarrior	actually wanted to apologize too since my system problems made a lot of delay...
19:11	greatwarrior	so if the mentor has any urgency then he may assign to one who is more efficient.
19:12	Mnyromyr	it's not exactly that hordes are threatening to run us over … ;-)
19:12	greatwarrior	means ??
19:14	Mnyromyr	== don't worry ;-)
19:15	greatwarrior	i am expecting same reply from my mentor !
Yes. Don't worry. We'll wait for you to complete your exams.
thanks a lot !!!
Amod says he is unable to work on this bug. Setting status back to NEW
Assignee: amod.narvekar → nobody
Status: ASSIGNED → NEW
Thanks. I will try my level best to work on the bug. You keep it "unassigned" until i upload my first patch. :)
Hello, Philip Chee. I built SeaMonkey successfully. Can I take this bug?
> Hello, Philip Chee. I built SeaMonkey successfully. Can I take this bug?
Thank you and welcome. I've assigned this to you. If you have any questions you can join us on IRC: Server: moznet Channel: SeaMonkey . Here is a convenient link:
irc://moznet/seamonkey
Assignee: nobody → carl
Status: NEW → ASSIGNED
I added TopLevelImageDocument.css and TopLevelVideoDocument.css.
Ah, I was insufficiently clear in my instructions. There are several CSS files named TopLevelImageDocument.css

http://mxr.mozilla.org/comm-central/find?text=&string=TopLevelImageDocument.css

The ones I wanted to be copied and adapted are "skin" (or theme) files:
http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/themes/windows/global/media/TopLevelImageDocument.css
http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/themes/windows/global/media/TopLevelVideoDocument.css

Note the new files should go into this directory:
http://mxr.mozilla.org/comm-central/source/suite/themes/modern/global/media/
and not:
http://mxr.mozilla.org/comm-central/source/suite/themes/modern/global/

TopLevelImageDocument.css:
>   body {
>     color: #eee;
>     background: #222 url("chrome://global/skin/media/imagedoc-darknoise.png");
>   }
> 
>   img.decoded {
>     background: hsl(0,0%,90%) url("chrome://global/skin/media/imagedoc-lightnoise.png");
>     color: #222;
>   }
We can leave the above out as the defaults are what we were using previously which were:
color: #000 and background-color: transparent .

>   .completeRotation {
>     transition: transform 0.3s ease 0s;
We don't need this style yet, so you can leave this out.

TopLevelVideoDocument.css:

I think the only bit we want from TopLevelVideoDocument.css is:

> video {
>   box-shadow: 0 0 15px #000;
> }

Now that we have the two files, we also need to package it when we build SeaMonkey. The file that controls packaging for the Modern Theme is a jar.mn file:

http://mxr.mozilla.org/comm-central/source/suite/themes/modern/jar.mn

This file is sorted more or less in alphabetical order so you would need to insert a couple of lines around here:

http://hg.mozilla.org/comm-central/annotate/8a50d60dd7a0/suite/themes/modern/jar.mn#l306

e.g.:

  skin/modern/global/media/TopLevelImageDocument.css               (global/media/TopLevelImageDocument.css)
  skin/modern/global/media/TopLevelVideoDocument.css               (global/media/TopLevelVideoDocument.css)
Carl, can you confirm that you're still working on this?
Flags: needinfo?(carl)
Mentor: philip.chee
Whiteboard: [good first bug][mentor=Ratty][lang=css] → [good first bug][lang=css]
Assignee: carl → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(carl)
Whiteboard: [good first bug][lang=css]
Attached image UI proposal (A)
Transparent background chequerboard pattern 1
Attachment #8496583 - Flags: ui-review?(rsx11m.pub)
Attachment #8496583 - Flags: ui-review?(neil)
Attached image UI proposal (B)
Transparent background chequerboard pattern 2
Attachment #8496584 - Flags: ui-review?(rsx11m.pub)
Attachment #8496584 - Flags: ui-review?(neil)
Attachment #8496583 - Flags: ui-review?(neil) → ui-review+
Attachment #8496584 - Flags: ui-review?(neil) → ui-review+
I don't really have a preference between the two proposals, but if you really need me to I'll pick one based on the patch.
Attached patch Patch v.0 Proposed fix. (obsolete) — Splinter Review
> +    /* checkerboard background image */
> +    background-image: url("chrome://global/skin/media/checkerboard.png");
> +    box-shadow: -5px 5px 5px 0px rgba(50, 50, 50, 0.75);

I based the above on the Nicer Media Pages extension, but I use a PNG instead of a data: uri and used a different box-shadow.

> +++ b/suite/themes/modern/global/media/TopLevelVideoDocument.css
> +body {
> +  background-color: #333;
> +}
Nicer Media Pages uses this background colour.

> +video {
> +  box-shadow: 0 0 15px #000;
> +}
From toolkit windows theme.
Assignee: nobody → philip.chee
Attachment #727160 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8497114 - Flags: review?(neil)
> +    background-image: url("chrome://global/skin/media/checkerboard.png");

Ok, so that's the 10x10 one from attachment 8496584 [details]. I actually like the finer 5x5 grid and more subtle coloring in attachment 8496583 [details] as it may be less disturbing if you have an image with lots of transparency in it where the checkerboard may be quite dominant. On the other hand, the larger grid is what's frequently seen in other applications, so...

Anyway, looks like you've made a pick and Neil approved either, thus let me know if you still need any ui-review from my side.
(In reply to rsx11m from comment #35)
> > +    background-image: url("chrome://global/skin/media/checkerboard.png");
> 
> Ok, so that's the 10x10 one from attachment 8496584 [details]. I actually
> like the finer 5x5 grid and more subtle coloring in attachment 8496583 [details]
> [details] as it may be less disturbing if you have an image with lots of
> transparency in it where the checkerboard may be quite dominant. On the
> other hand, the larger grid is what's frequently seen in other applications,
> so...
> 
> Anyway, looks like you've made a pick and Neil approved either, thus let me
> know if you still need any ui-review from my side.

You now need to flip the ui-review flags (+/-) otherwise your ui-review will be pending forever ;)
Flags: needinfo?(rsx11m.pub)
Comment on attachment 8496583 [details]
UI proposal (A)

As said, I'd prefer this one.
Attachment #8496583 - Flags: ui-review?(rsx11m.pub) → ui-review+
Attachment #8496584 - Flags: ui-review?(rsx11m.pub)
Flags: needinfo?(rsx11m.pub)
Comment on attachment 8497114 [details] [diff] [review]
Patch v.0 Proposed fix.

>+  /* N.B.: Remember to update ImageDocument.css in the tree or reftests may fail! */
???

>+  body {
>+    background-color: #FFF;
>+  }
If we have to set this, then I'd prefer the same colour as the video document.

>+  img.decoded {
>+    background-color: hsl(0, 0%, 90%);
#e5e5e5, but...

>+    /* checkerboard background image */
>+    background-image: url("chrome://global/skin/media/checkerboard.png");
... prefer to make the background image use solid colours instead of alpha transparency over a known background.

>+    box-shadow: -5px 5px 5px 0px rgba(50, 50, 50, 0.75);
And I think the box shadow should be consistent with the video document too.
(In reply to neil@parkwaycc.co.uk from comment #38)
> Comment on attachment 8497114 [details] [diff] [review]
> Patch v.0 Proposed fix.
> >+  body {
> >+    background-color: #FFF;
> >+  }
> If we have to set this, then I'd prefer the same colour as the video document.

I'm kind-of wondering about this too, but in reverse. What's the motivation for #333 rather than something more neutral? Would it fit with the overall color scheme of the modern theme? I know that toolkit introduced at some point a dark-gray background for stand-alone images. What purpose does it serve? Ok, if you look at a bright image with white background, you get a better contrast of the image's boundaries; but, if you have a dark-grayish background instead, #333 doesn't help you either, and the problem is only shifted in range. Thus, any background color is as good as none defined.

Just my two cents, make it a neutral #FFF or leave it up to the desktop theme to define the background (same as default content pages, I'd suggest).

Video pages may be a different story - it's moving content (thus background delineation may not be as important) and you are starring at it possibly for a long time, thus a more subtle background color might help.
>>+  /* N.B.: Remember to update ImageDocument.css in the tree or reftests may fail! */
> ???
Removed

>>+  body {
>>+    background-color: #FFF;
>>+  }
> If we have to set this, then I'd prefer the same colour as the video document.
Disagree. See below.

>>+  img.decoded {
>>+    background-color: hsl(0, 0%, 90%);
> #e5e5e5, but...
Fixed.

>>+    /* checkerboard background image */
>>+    background-image: url("chrome://global/skin/media/checkerboard.png");
> ... prefer to make the background image use solid colours instead of alpha transparency over a known background.
Fixed.

>>+    box-shadow: -5px 5px 5px 0px rgba(50, 50, 50, 0.75);
> And I think the box shadow should be consistent with the video document too.
Fixed.

(In reply to rsx11m from comment #39)
> (In reply to neil@parkwaycc.co.uk from comment #38)
>> Comment on attachment 8497114 [details] [diff] [review]
>> Patch v.0 Proposed fix.
>>>+  body {
>>>+    background-color: #FFF;
>>>+  }
>> If we have to set this, then I'd prefer the same colour as the video document.
> 
> I'm kind-of wondering about this too, but in reverse. What's the motivation
> for #333 rather than something more neutral? Would it fit with the overall
> color scheme of the modern theme? I know that toolkit introduced at some
> point a dark-gray background for stand-alone images. What purpose does it
> serve? Ok, if you look at a bright image with white background, you get a
> better contrast of the image's boundaries; but, if you have a dark-grayish
> background instead, #333 doesn't help you either, and the problem is only
> shifted in range. Thus, any background color is as good as none defined.
> 
> Just my two cents, make it a neutral #FFF or leave it up to the desktop
> theme to define the background (same as default content pages, I'd suggest).

Historically the background was white, or rather a default value which usually maps to white. Recently toolkit switched to a dark background upsetting some users. In response several extensions have appeared on AMO that offer to return the background colour to white (or to a colour the user chooses). My motivation in this bug is to fix the "regression" and put it back to what it was before.

+  body {
+    background-color: #FFF;
+    background-color: Window;
+    background-color: -moz-default-background-color;
+  }

Option 0: don't set the background colour at all. This matches the historical behaviour.

Option 1: hard code to white.

Option 2: Use a system colour like "Window"
https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#System_Colors
Window
    Window background.  Should be used with WindowText foreground color.
This maps the background to the window background colour from the OS theme.

Option 3: -moz-default-background-color;
https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#Mozilla_Color_Preference_Extensions
-moz-default-background-color
    User's preference for document background-color.
This maps to "browser.display.background_color"
  Edit -> Preferences -> Appearance -> Colours -> Text and Background -> Background
Bonus: if the user selects "Use system colours" this is effectively Option 2.

> Video pages may be a different story - it's moving content (thus background
> delineation may not be as important) and you are starring at it possibly for
> a long time, thus a more subtle background color might help.

Video is where an immersive experience has significant impact. A dark background is better in this case.

I looked at the toolkit imagedoc-darknoise.png . Poking at it with a colour picker I find that the colour varies between #202020 and #232323 . So I've set the background colour for the video document to #222 which is somewhere in between.
Attachment #8497114 - Attachment is obsolete: true
Attachment #8497114 - Flags: review?(neil)
Attachment #8512174 - Flags: review?(neil)
(In reply to Philip Chee from comment #40)
> Option 3: -moz-default-background-color;
> https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#Mozilla_Color_Preference_Extensions
> -moz-default-background-color
>     User's preference for document background-color.
> This maps to "browser.display.background_color"
>   Edit -> Preferences -> Appearance -> Colours -> Text and Background ->
> Background
> Bonus: if the user selects "Use system colours" this is effectively Option 2.

This sounds like a good pick to me, keeping the background consistent with unstyled web pages.
(In reply to Philip Chee from comment #40)
> Option 3: -moz-default-background-color;

Surely this ends up being the same as Option 0?
Probably yes.
(In reply to neil@parkwaycc.co.uk from comment #42)
> (In reply to Philip Chee from comment #40)
>> Option 3: -moz-default-background-color;

> Surely this ends up being the same as Option 0?

I tested this. When set to -moz-default-background-color, changing the browser.display.background_color changes the background in the image document.
So, if you just don't define *any* body "{ background-color: ... ; }" (which is "Option 0"), do you get the same behavior as for Option 3 with an explicit -moz-default-background-color? The assumption being that it's taken anyway if no other background is defined.
(In reply to rsx11m from comment #45)
> So, if you just don't define *any* body "{ background-color: ... ; }" (which
> is "Option 0"), do you get the same behavior as for Option 3 with an
> explicit -moz-default-background-color? The assumption being that it's taken
> anyway if no other background is defined.
Yeah that's correct. I've tested it. Silly me I made a incorrect assumption. New patch coming up.
(In reply to neil@parkwaycc.co.uk from comment #42)
> (In reply to Philip Chee from comment #40)
>> Option 3: -moz-default-background-color;
> 
> Surely this ends up being the same as Option 0?
Yes. Sorry about the thinko.

Changes since the last patch:
1. Don't specify a background-color for the body
2. Removed background-color from img.decoded since colours in the background image are now solid.
3. chequerboard now with solid colours (#e5e5e5) and no transparency.
4. chequerboard now using 5x5px squares down from 10x10px.
Attachment #8512174 - Attachment is obsolete: true
Attachment #8512174 - Flags: review?(neil)
Attachment #8515642 - Flags: review?(neil)
@media not print gone missing?
Attached image checkerboard.gif
50 bytes as a GIF!
Comment on attachment 8515642 [details] [diff] [review]
Patch v3.0 don't style body.

r=me with the @media fixed.
Attachment #8515642 - Flags: review?(neil) → review+
> Created attachment 8518476 [details]
> checkerboard.gif
> 
> 50 bytes as a GIF!
Ran PNGGauntlet on the PNG. Now 80 bytes :P 

Pushed to comm-central http://hg.mozilla.org/comm-central/rev/9903659144c7
comm-central changeset 9903659144c7
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.33
Blocks: 1112196
You need to log in before you can comment on or make changes to this bug.