1071101 - CSS border should be removed from Fullscreen window controls in Modern theme

Status: RESOLVED FIXED

(SeaMonkey :: Themes, defect)

Description

[rexyrexy2]

The Modern theme's fullscreen window controls have an unneeded border on them, which looks out of place. These borders should be removed with something like this: 

#window-controls > toolbarbutton {
    border: none !important;
}

Comment 1

[[e-mail]]

The "border" you see there is actually part of the image itself:
http://mxr.mozilla.org/comm-central/source/suite/themes/modern/navigator/icons/windowcontrols.png

Comment 2

[Philip Chee]

Should there be those gaps (padding? margin?) between the full screen controls?

Comment 3

[Philip Chee]

(In reply to Philip Chee from comment #2)
> Should there be those gaps (padding? margin?) between the full screen
> controls?
Firefox fullscreen controls don't have gaps between each control

Comment 4

[Tony Mechelynck [:tonymec]]

(In reply to Philip Chee from comment #3)
> (In reply to Philip Chee from comment #2)
> > Should there be those gaps (padding? margin?) between the full screen
> > controls?
> Firefox fullscreen controls don't have gaps between each control

Firefox also doesn't come with a Modern theme.

I'll attach for comparison screenshots of the SeaMonkey fullscreen buttons in Modern, Default, and in my favorite theme (KaiRo's "EarlyBlue"):

- Modern has images which look slightly spherical and outset, with borders. There is a neutral zone of a few pixels between each of them.
- Default has flat images with no borders; they are even wider apart (or with smaller graphics) than in Modern, and on mouseover I can see that there is also a neutral zone between each of them.
- EarlyBlue has flat images with no borders, and the neutral zone between them (if there is any) is no more than a pixel wide.

IMHO the presence or absence of borders and/or of a neutral zone between buttons is part of the "artistic freedom" of the theme's designer. A neutral zone takes up a little space on the screen, but OTOH it makes it easier to avoid actuating one button when another one was meant.

Comment 5

[Tony Mechelynck [:tonymec]]

Attachment: buttons for Modern

Comment 6

[Tony Mechelynck [:tonymec]]

Attachment: buttons for Default

Comment 7

[Tony Mechelynck [:tonymec]]

Attachment: buttons for EarlyBlue

Comment 8

[Tony Mechelynck [:tonymec]]

P.S. The first two attachments come from the following Trunk build (if it makes any difference):

Mozilla/5.0 (X11; Linux x86_64; rv:36.0) Gecko/20100101 Firefox/36.0 SeaMonkey/2.33a1 ID:20141112003001 c-c:8247ad339166 m-c:688f821edcd4

The third one is from the latest EarlyBlue 2.31 as found at http://git-public.kairo.at/themes (run "./maketheme EarlyBlue" in the top directory to build it; I think this requires a Unix-like environment. I'm on Linux but Mac OSX and/or Cygwin might work too); it may or may not have been already uploaded to AMO.

Comment 9

[rexyrexy2]

I think I failed to clarify what I meant.. I meant when you hover or press them, windows classic's 3d button borders show up. These look completely out of place.

Comment 10

[rexyrexy2]

They look out of place, because the buttons image is designed to be independently used, and with a border CSS property on it, it looks weird. As the button image was designed to be a button all its own, the effect is a rather weird button-in-a-button effect. So the CSS border should be removed, as it appears it wasn't meant to be there on these to begin with

Comment 11

[rexyrexy2]

Updated the bug title to clarify this.

Comment 12

[Phoenix]

This bug have CLOSEME INVA/WONT whiteboard, as 1 month passed from last comment and no date specified it should be closed now. Any objections?

Comment 13

[Tony Mechelynck [:tonymec]]

(In reply to Phoenix from comment #12)
> This bug have CLOSEME INVA/WONT whiteboard, as 1 month passed from last
> comment and no date specified it should be closed now. Any objections?

In principle WONTFIX should only be set by an owner or peer; OTOH the question mark at the end means that the person who set that whiteboard entry (me) thinks, but is not sure, that this is as designed.

Ratty, you're a Themes peer; what do you think?

Comment 14

[Philip Chee]

In Modern the border and padding come from the generic toolbar styles

http://mxr.mozilla.org/comm-central/source/suite/themes/modern/global/toolbarbutton.css?rev=fa4ea41c266d&mark=49-50#46

>  border: 1px solid transparent;
Setting the border to none improves the hover styles as rexyrexy2 points out
>  padding: 1px 2px;
Removing the padding makes the buttons look scrunched up.

Personally I think that to match the other buttons on the toolbar the button images should probably be 24x24px.

Comment 15

[[e-mail]]

(In reply to Philip Chee from comment #14)
> >  border: 1px solid transparent;
> Setting the border to none improves the hover styles as rexyrexy2 points out
Either none or transparent works; none reduces the space but not too much.

> >  padding: 1px 2px;
> Removing the padding makes the buttons look scrunched up.
Agreed.

> Personally I think that to match the other buttons on the toolbar the button
> images should probably be 24x24px.
Beyond the scope of this bug.

Comment 16

[rexyrexy2]

Just for reference, setting the border to none and the padding to 1px on both sides (left and right) it makes them look how they were in Mozilla 1.7.

Comment 17

[rpatel.1594]

Hi, I would like to solve this bug. I found this bug from Bugs Ahoy. Can you please let me know how to proceed from here on. This is my first bug.

Comment 18

[Philip Chee]

Hi! Have you built Firefox or SeaMonkey before? If not please read:
https://developer.mozilla.org/en-US/docs/Simple_SeaMonkey_build
Please join us on IRC irc://moznet/seamonkey if you have any questions. Since we are scattered across various timezones you may not get an immediate reply. Please leave your IRC client on idle so that you can see if there are any replies. An alternative is to use a web based client like mibbit.

Comment 19

[ousman bah]

HI, I would like to fix this bug. I have already downloaded and installed source code. Can you guide me to fix this bug

Comment 20

[ousman bah]

I have clone the mozilla source code in my laptop. What is the path to locate the available code for this bug, so that I can start working on it.

Comment 21

[Philip Chee]

Hi! I assume you have installed Mercurial and have followed the instructions in:
https://developer.mozilla.org/en-US/docs/Simple_SeaMonkey_build

If you have the comm-central source code it should be in comm-central/suite/themes/modern/global/toolbarbutton.css. See the following MXR link for reference.

http://mxr.mozilla.org/comm-central/source/suite/themes/modern/global/toolbarbutton.css?rev=fa4ea41c266d&mark=49-50#46

Comment 22

[ousman bah]

I have already installed Mercurial and clone the Mozilla source code. I have seen the file and presently working on it.

Comment 23

[ousman bah]

I have attempted fixing the bug by including 
+window-controls > toolbarbutton {
+    border: none !important;
+}

This should remove the CSS border

Comment 24

[Pas]

Hi, I am new to fixing bugs and I would like to work on the bug please.

Comment 25

[Philip Chee]

(In reply to Pas from comment #24)
> Hi, I am new to fixing bugs and I would like to work on the bug please.
Sorry ousman bah has already started on this. Please find another bug

Comment 26

[Philip Chee]

(In reply to ousman bah from comment #23)
> I have attempted fixing the bug by including 
> +window-controls > toolbarbutton {
> +    border: none !important;
> +}
> 
> This should remove the CSS border

Thanks. You should add the rule in suite/themes/modern/navigator/navigator.css where there is already a section for window-controls:

http://mxr.mozilla.org/comm-central/source/suite/themes/modern/navigator/navigator.css?rev=ae33ba8c297b&mark=252-255#241

Since |window-controls > toolbarbutton| is more specific than just toolbarbutton you probably don't need the !important.

Comment 27

[Medka]

Attachment: ahmed.patch

Comment 28

[Philip Chee]

> -  min-width: 0;
> +  border: none;
> + min-width: 0;
You seem to have removed a space on the min-width: 0; line.
Also please put the border: none; rule after the min-width line.
Thanks!

Comment 29

[Medka]

Attachment: Removed extra space and swapped border:none; with min-width: 0;

Comment 30

[Medka]

(In reply to Philip Chee from comment #28)
> > -  min-width: 0;
> > +  border: none;
> > + min-width: 0;
> You seem to have removed a space on the min-width: 0; line.
> Also please put the border: none; rule after the min-width line.
> Thanks!

Removed space and swapped.

Comment 31

[Philip Chee]

Comment on attachment 8696367 [details] [diff] [review]
Removed extra space and swapped border:none; with min-width: 0;

(In reply to Medka from comment #30)
> Removed space and swapped.
Works great! Thanks!

Comment 32

[Philip Chee]

Comment on attachment 8696367 [details] [diff] [review]
Removed extra space and swapped border:none; with min-width: 0;

> Removed extra space and swapped border:none; with min-width: 0;
This should go into the "attachment details" field in the Bugzilla new attachment form.

The commit comment should say what the patch does and also mention the bug number. E.g.

Bug 1071101 - Remove CSS border from the fullscreen window controls in the Modern theme.

Comment 33

[:aceman]

Comment on attachment 8696367 [details] [diff] [review]
Removed extra space and swapped border:none; with min-width: 0;

Review of attachment 8696367 [details] [diff] [review]:
-----------------------------------------------------------------

Next time please use a better commit message to say what the patch does. And also include bug number in it.

Comment 34

[:aceman]

https://hg.mozilla.org/comm-central/rev/d374ee5e4c20

I have done the fixes to the commit message.

Comment 35

[ousman bah]

I cannot find the file in suite/themes/modern/navigator/navigator.css. You stated. I have clone the mozilla source code and have mozilla-central and mozilla-build stored in my C:/.

Comment 36

[Philip Chee]

(In reply to ousman bah from comment #35)
> I cannot find the file in suite/themes/modern/navigator/navigator.css. You
> stated. I have clone the mozilla source code and have mozilla-central and
> mozilla-build stored in my C:/.

This file lives in comm-central: http://hg.mozilla.org/comm-central/