1444740 - Drop support for -moz-border-*-colors in SeaMonkey

Status: RESOLVED FIXED

(SeaMonkey :: Themes, enhancement)

Description

[Frank-Rainer Grahl (:frg)]

This is gone now and especially the Modern theme has been hit by it.

TB replaced them in

Bug 1432251
Bug 1430653
Bug 1430655
Bug 1430490
Bug 1430482

+++ This bug was initially created as a clone of Bug #1429723 +++

Bug 1417200 made -moz-border-*-colors chrome-only. We've now removed most uses from chrome stylesheets, so before long there will be no point in maintaining support for -moz-border-*-colors.

Comment 1

[MMR]

Hello,

I would like to work on this as my Good Second Bug. I haven't setup the SeaMonkey environment yet, but have the resources to do so. 

I will update as soon as I can.

Thanks,

Comment 2

[MMR]

Hello,

I am trying to find the occurrences of "-moz-border-*-colors" and this is the query I am using (I don't know yet which directories i have to modify)

https://dxr.mozilla.org/comm-central/search?q=%22-colors%3A%22&redirect=false

Can you provide an exact query (or atleast some important/relevant folders in which I start looking) which returns all the occurrences?

Thanks,

Comment 3

[MMR]

I got this idea from here:
https://bugzilla.mozilla.org/show_bug.cgi?id=1429723#c3

Comment 4

[Frank-Rainer Grahl (:frg)]

Attachment: moz-border.txt

Looks good. I narrowed it down to suite:

https://dxr.mozilla.org/comm-central/search?q=%22-colors%3A%22+path%3Asuite&redirect=false

SeaMonkey has two themes. The default classic theme with the icons from the 90ies :) (needs updating for hi-dpi but that can wait) and the Modern theme. I would split any  patch at least in two and start with classic. This theme mostly uses the standard theme files from toolkit.

Comment 5

[Frank-Rainer Grahl (:frg)]

The list was actually produced by Total Commander where I searched for moz-border.

Comment 6

[MMR]

Attachment: changing the deprecated moz-border

There were multiple colors in the (-moz-border-*-colors: color1 color2) aforementioned attribute. By taking reference from Bug 1430653, i have replaced it with the first color (border-*-color: color1)

Comment 7

[Stefan [:stefanh]]

Thanks for the patch :-)

Drive-by comments:

-  -moz-border-right-colors: ThreeDDarkShadow ThreeDShadow;
-  -moz-border-bottom-colors: ThreeDDarkShadow  ThreeDShadow;
+  border-right-colors: ThreeDDarkShadow;
+  border-bottom-colors: ThreeDDarkShadow;
--> border-right-color and border-bottom-color

- There are some shorthand properties that can be used when having the same color on multiple sides
- I think you need to shrink the borders in a lot of cases and compensate with padding if needed

Comment 8

[MMR]

Hello Stefan,

I thank you for taking out time to look at the patch. Oh and thanks for the tip on "s" in border-*-colors. I have several questions.

Question 1:

Yes there are shorthand properties (https://www.w3schools.com/cssref/pr_border-color.asp) and I have used two of them (in the patch above).

1. border-color: color_for_all_borders;
2. border-color: color_top color_right color_bottom color_left;

My question is, did I miss any case where I could've applied the shorthand? Can you cite those cases for me please?

Question 2:

I faced a dilemma in this case (note: there is no border-right in editorModeToolbar.css:.tab-bottom[selected="true"]):

  -moz-border-top-colors: -moz-Field;
  -moz-border-bottom-colors: ThreeDShadow -moz-Field;
  -moz-border-left-colors: ThreeDHighlight -moz-Field;

Approach 1 (existing in the patch submitted above)
  border-top-color: -moz-Field;
  border-bottom-color: ThreeDShadow;
  border-left-color: ThreeDHighlight;

Approach 2
  border-color: -moz-Field unset ThreeDShadow ThreeDHighlight;

So is it advisable to use the "unset" property in order to make it shorter? (Or is Approach 2 better than Approach 1?)

Question 3:

I know that when you specify two colors in the border-*-colors: color1 color2, color2 is applied on the "outer side" of border, and color1 is applied on the "inner side" of border.

But now, we are just using one color for the border (the new border-*-color, doesn't allow us to use multiple colors), so do you want me to set the border width to 1px, wherever we are using single color instead of two colors?

(For example, https://bugzilla.mozilla.org/attachment.cgi?id=8960578&action=diff#a/suite/themes/classic/communicator/sidebar/sidebar.css_sec2 has 2px border and 2px padding, what should I do here?)

Question 4:

When you say compensate with padding if needed, how will I know, that I need to compensate? I am a little lost in shrinking and padding concept
https://jsfiddle.net/gfcznz66/  <-- some JS fiddles will help me perhaps? :)

Thanks,

Comment 9

[Stefan [:stefanh]]

(In reply to MMR from comment #8)
> Hello Stefan,
> 
> I thank you for taking out time to look at the patch. Oh and thanks for the
> tip on "s" in border-*-colors. I have several questions.
> 
> Question 1:
> 
> Yes there are shorthand properties
> (https://www.w3schools.com/cssref/pr_border-color.asp) and I have used two
> of them (in the patch above).
> 
> 1. border-color: color_for_all_borders;
> 2. border-color: color_top color_right color_bottom color_left;
> 
> My question is, did I miss any case where I could've applied the shorthand?
> Can you cite those cases for me please?

My mistake, I now see that 'ThreeDHighlight ThreeDShadow ThreeDShadow ThreeDLightShadow;' can't be done shorter :-). So you haven't missed anything.

> 
> Question 2:
> 
> I faced a dilemma in this case (note: there is no border-right in
> editorModeToolbar.css:.tab-bottom[selected="true"]):
> 
>   -moz-border-top-colors: -moz-Field;
>   -moz-border-bottom-colors: ThreeDShadow -moz-Field;
>   -moz-border-left-colors: ThreeDHighlight -moz-Field;
> 
> Approach 1 (existing in the patch submitted above)
>   border-top-color: -moz-Field;
>   border-bottom-color: ThreeDShadow;
>   border-left-color: ThreeDHighlight;
> 
> Approach 2
>   border-color: -moz-Field unset ThreeDShadow ThreeDHighlight;
> 
> So is it advisable to use the "unset" property in order to make it shorter?
> (Or is Approach 2 better than Approach 1?)

Approach 1 is fine.

> 
> Question 3:
> 
> I know that when you specify two colors in the border-*-colors: color1
> color2, color2 is applied on the "outer side" of border, and color1 is
> applied on the "inner side" of border.
> 
> But now, we are just using one color for the border (the new border-*-color,
> doesn't allow us to use multiple colors), so do you want me to set the
> border width to 1px, wherever we are using single color instead of two
> colors?
> 
> (For example,
> https://bugzilla.mozilla.org/attachment.cgi?id=8960578&action=diff#a/suite/
> themes/classic/communicator/sidebar/sidebar.css_sec2 has 2px border and 2px
> padding, what should I do here?)
> 
> Question 4:
> 
> When you say compensate with padding if needed, how will I know, that I need
> to compensate? I am a little lost in shrinking and padding concept
> https://jsfiddle.net/gfcznz66/  <-- some JS fiddles will help me perhaps? :)
> 

When you decrease the border-widths the element will decrease in size and it's hard to make a general statement here, it really depends on the surrounding elements. So I think we need to test your patch carefully.

If you attach a new patch, I can see how it looks on mac (I don't have access to win/linux) and give you feedback on 3 & 4.

Oh, and you can ask me for review instead.

Comment 10

[MMR]

Attachment: changing_deprecated_border_attribute_for_classic_theme.patch

1. Changed borders to border in some places
2. this is comm-esr52 based patch, classic theme only
3. I have compiled and built seamonkey after the patch
4. launched console from tools->webdeveloper, to check if there were any syntax errors in CSS because of my changes; there were none

Comment 11

[Stefan [:stefanh]]

Comment on attachment 8961255 [details] [diff] [review]
changing_deprecated_border_attribute_for_classic_theme.patch

Thanks for the patch.

(In reply to MMR from comment #10)

> 2. this is comm-esr52 based patch, classic theme only

Support for -moz-border-*-colors wasn't completely dropped until Firefox 60 (SeaMonkey 2.57) - see bug 1429723. So we just need this in 2.57 (which will be comm-esr60) and trunk. The patch actually fails for me when I try to apply it since scrollbars.css have moved to suite/themes/classic/linux/communicator/scrollbars.css in SeaMonkey 2.56 (bug 1269145).

Looking at the patch I found some files which also needs to be changed (not in the patch):
suite/themes/classic/communicator/communicator.css
suite/themes/classic/communicator/profile/profile.css
suite/themes/classic/communicator.css
suite/themes/classic/mac/editor/editorModeToolbar.css

In suite/themes/classic/mac/navigator/tabbrowser.css, you missed changing the colors:
 .scrollbutton-up[disabled="true"]:-moz-locale-dir(ltr),
 .scrollbutton-down[disabled="true"]:-moz-locale-dir(rtl) {
   -moz-image-region: rect(0, 14px, 11px, 7px);
-  -moz-border-right-colors: transparent transparent;
+  border-right-color: transparent transparent;
 }

I suspect that there could have happened a lot between esr52 and now with some of the files, so I think you need to provide a patch for at least comm-esr60 which right now is comm-beta (https://hg.mozilla.org/releases/comm-beta/), but most preferable is of course a patch for trunk (https://hg.mozilla.org/comm-central/) which is what I currently have built (I can get a comm-beta build, though).

Comment 12

[MMR]

Thank you for your inputs.

I have started to pull the comm-central repo, I didn't take this before as I was warned that it is broken, but nevermind, if I have build errors, I can always pester you or FRG :)

I will submit the updated patch next week once I have a working seamonkey setup.

Thanks again!
MMR

Comment 13

[Frank-Rainer Grahl (:frg)]

stefan, I suggested comm-esr52 first because everything still works there.

MMR Mozilla trashed linking the comm-central apps with recent build environment changes (and a few other things too). Still not fixed I suggest you do a patch for comm-beta.

Comment 14

[Stefan [:stefanh]]

MMR, sorry for the confusion. Yeah, comm-beta will be fine (I didn't know comm-central was broken - haven't pulled recently).


(In reply to Frank-Rainer Grahl (:frg) from comment #13)
> stefan, I suggested comm-esr52 first because everything still works there.
It's a bit painful to transfer it to comm-beta with all the changes that have happened.

Comment 15

[MMR]

Attachment: changing_deprecated_border_attribute_for_classic_theme_beta_repo.patch

Attaching the diff for comm-beta, classic theme. Sorry took a lot of time to get the build working, but still my SM crashes upon launch, therefore couldn't verify. Stefan, can you look at the diff in the meantime, while I try to figure out more about the crash?

Comment 16

[MMR]

Attachment: changing_deprecated_border_attribute_for_classic_theme_beta_repo.patch

Comment 17

[MMR]

Attachment: changing_deprecated_border_attribute_for_classic_theme_beta_repo.patch

Comment 18

[MMR]

Comment on attachment 8964166 [details] [diff] [review]
changing_deprecated_border_attribute_for_classic_theme_beta_repo.patch

forgot the "-c" switch while taking the diff. My bad

Comment 19

[Stefan [:stefanh]]

Sorry, I'm a bit slow atm, but I'll try to look at this over the weekend.

Comment 20

[Stefan [:stefanh]]

Comment on attachment 8964167 [details] [diff] [review]
changing_deprecated_border_attribute_for_classic_theme_beta_repo.patch

OK, so I've now tested the mac parts. The tab parts took really long time and we might want to re-visit this in another bug once SM gets to work properly.

Anyway, I think you should split the patch in 2 parts, one for the mac files and one for the other files. The mac files I've looked at are:
suite/themes/classic/mac/communicator/sidebar/sidebar.css
suite/themes/classic/mac/editor/editorModeToolbar.css
suite/themes/classic/mac/messenger/mailWindow1.css
suite/themes/classic/mac/navigator/tabbrowser.css

In general, we should always adjust the widths of the borders - it doesn't look good with a 2px border in one color. The tricky part is to figure out if you need to compensate, but that can be tested by the reviewer. So, I'd suggest that you adjust the border-widths in the non-mac files and upload that as a separate patch and ask frg for review.

Anyway, here are my comments - if you upload a new version with my suggested changes we're done with the mac part and I'll r+ it once I looked through it.


>diff --git a/suite/themes/classic/mac/communicator/sidebar/sidebar.css b/suite/themes/classic/mac/communicator/sidebar/sidebar.css
>--- a/suite/themes/classic/mac/communicator/sidebar/sidebar.css
>+++ b/suite/themes/classic/mac/communicator/sidebar/sidebar.css
>@@ -104,36 +104,30 @@
>   text-shadow: none;
> }
> 
> .box-texttab,
> .box-texttab[selected="true"],
> .box-texttab[selected="true"]:hover,
> .box-texttab[selected="true"]:hover:active {
>   border: 2px solid;

Should be 1px.

>diff --git a/suite/themes/classic/mac/editor/editorModeToolbar.css b/suite/themes/classic/mac/editor/editorModeToolbar.css
>--- a/suite/themes/classic/mac/editor/editorModeToolbar.css
>+++ b/suite/themes/classic/mac/editor/editorModeToolbar.css
>@@ -27,17 +27,17 @@
>   list-style-image: url("chrome://editor/skin/icons/editmode-preview.gif"); 
> }
> 
> #EditModeTabs {
>   background-color: rgba(0, 0, 0, 0.1);
>   padding: 0;
>   margin: 0;
>   border-top: 2px solid;
>-  -moz-border-top-colors: #888 rgba(0, 0, 0, 0.08);
>+  border-top-color: #888;

We want the width here to be 1px, so you can just change the border-top rule to "1px solid #888;"

> .tab-bottom {
>@@ -58,19 +58,19 @@
> }

Can you please add a new rule under the .tab-bottom rule:

.tab-bottom:first-of-type {
  border-inline-start: 1px solid rgba(0, 0, 0, 0.19);
}

> 
> .tab-bottom[visuallyselected=true] {
>   color: #000;
>   text-shadow: none;
>   border: solid #888;
>   border-width: 0 2px 2px;

0 1px 1px

>   border-radius: 2px;
>-  -moz-border-left-colors: rgba(0, 0, 0, 0.08) #888;
>-  -moz-border-right-colors: rgba(0, 0, 0, 0.08) #888;
>-  -moz-border-bottom-colors: rgba(0, 0, 0, 0.08) #888;
>+  border-left-color: rgba(0, 0, 0, 0.08);
>+  border-right-color: rgba(0, 0, 0, 0.08);
>+  border-bottom-color: rgba(0, 0, 0, 0.08);

Just remove all the above color rules, the color here should be the same as the border-top color of #EditModeTabs (#888) since that's actually the top border of non-selected tabs.

>   margin-inline-end: -1px;
>   margin-top: -2px;

Please remove the margin-top rule


Also, please remove the following rules - beforeselected doesn't exist anymore and I think we're OK without the other rule:

.tab-bottom[beforeselected=true] {
  border-inline-end-color: transparent;
  margin-inline-end: -2px;
}

.tab-bottom:first-of-type[visuallyselected=true] {
  margin-inline-start: -2px;
}



>diff --git a/suite/themes/classic/mac/messenger/mailWindow1.css b/suite/themes/classic/mac/messenger/mailWindow1.css
>--- a/suite/themes/classic/mac/messenger/mailWindow1.css
>+++ b/suite/themes/classic/mac/messenger/mailWindow1.css
>@@ -243,39 +243,39 @@
>   border: 0;
>   padding: 0 4px;
>   margin: 0;
> }
> 
> .tab-scrollbutton-up:-moz-locale-dir(ltr),
> .tab-scrollbutton-down:-moz-locale-dir(rtl) {
>   border-right: 2px solid;

1px here
 
> .tab-scrollbutton-down:-moz-locale-dir(ltr),
> .tab-scrollbutton-up:-moz-locale-dir(rtl) {
>   border-left: 2px solid;

And here

> .tab-scrollbutton-up:hover:active:not([disabled="true"]),
>diff --git a/suite/themes/classic/mac/navigator/tabbrowser.css b/suite/themes/classic/mac/navigator/tabbrowser.css
>--- a/suite/themes/classic/mac/navigator/tabbrowser.css
>+++ b/suite/themes/classic/mac/navigator/tabbrowser.css
>@@ -14,17 +14,17 @@ tabbox {
> }
> 
> tabpanels {
>   -moz-appearance: none;
> }
> 
> .tabs-bottom {
>   border-bottom: 3px solid;
>-  -moz-border-bottom-colors: #888 rgba(0, 0, 0, 0.08);
>+  border-bottom-color: #888;
> }

Please change the above to "border-bottom: 1px solid rgba(0, 0, 0, 0.08);"


> 
> .tabbrowser-tabs {
>   font: icon;
>   background-color: rgba(0, 0, 0, 0.1);
>   border-top: 1px solid rgba(0, 0, 0, 0.08);
>   padding: 0;
>   margin: 0;

Please add "margin-bottom: 2px;" here (under the margin rule).


>@@ -47,22 +47,22 @@ tabpanels {
>   list-style-image: url("chrome://communicator/skin/bookmarks/bookmark-item.svg");
>   border: solid transparent;
>   border-width: 0 2px;

border-width: 0 1px;

>   -moz-appearance: none;
> }
> 
> .tabbrowser-tab:not([selected="true"]):-moz-locale-dir(ltr),
> .tabbrowser-tab:not([selected="true"]):-moz-locale-dir(rtl):first-child {
>-  -moz-border-right-colors: transparent rgba(0, 0, 0, 0.19);
>+  border-right-color: transparent;

Color should be rgba(0, 0, 0, 0.19);

> }
> 
> .tabbrowser-tab:not([selected="true"]):-moz-locale-dir(ltr):first-child,
> .tabbrowser-tab:not([selected="true"]):-moz-locale-dir(rtl) {
>-  -moz-border-left-colors: transparent rgba(0, 0, 0, 0.19);
>+  border-left-color: transparent;

Same here

> 
> .tabbrowser-tab:not(:first-child) {
>   margin-inline-start: -3px;

margin-inline-start: -1px;


> 
> .tab-middle {
>   padding: 4px 7px;
>@@ -97,34 +97,34 @@ tabpanels {
>   list-style-image: url("chrome://communicator/skin/icons/loading.png");
> }
> 
> .tabbrowser-tab[selected="true"] {
>   color: #000000;
>   text-shadow: none;
>   border-radius: 2px;
>   border-top-width: 2px;

border-top-width should be 1px here


>-  -moz-border-top-colors: rgba(0, 0, 0, 0.1) #888;
>-  -moz-border-left-colors: rgba(0, 0, 0, 0.08) #888;
>-  -moz-border-right-colors: rgba(0, 0, 0, 0.08) #888;
>+  border-top-color: rgba(0, 0, 0, 0.1);
>+  border-left-color: rgba(0, 0, 0, 0.08);
>+  border-right-color: rgba(0, 0, 0, 0.08);
>   margin-top: 1px;

Please change the margin-top to 2px

> 
> .scrollbutton-up:-moz-locale-dir(ltr),
> .scrollbutton-down:-moz-locale-dir(rtl) {
>   border-right: 2px solid;

1px

>-  -moz-border-right-colors: rgba(0, 0, 0, 0.19) transparent;
>+  border-right-color: rgba(0, 0, 0, 0.19);
>   list-style-image: url("chrome://messenger/skin/icons/tab-arrow-left.png");
>   -moz-image-region: rect(0, 7px, 11px, 0);
> }
> 
> .scrollbutton-up[disabled="true"]:-moz-locale-dir(ltr),
> .scrollbutton-down[disabled="true"]:-moz-locale-dir(rtl) {
>   -moz-image-region: rect(0, 14px, 11px, 7px);
>-  -moz-border-right-colors: transparent transparent;
>+  border-right-color: transparent;
> }
> 
> .scrollbutton-down:-moz-locale-dir(ltr),
> .scrollbutton-up:-moz-locale-dir(rtl) {
>   border-left: 2px solid;

Same here

Comment 21

[Stefan [:stefanh]]

BTW, suite/themes/classic/communicator/bookmarks/bookmarksWindow.css looks unused (it's not even packaged), so I don't think you need to change that.

Comment 22

[MMR]

Attachment: Bug_1444740_changing_deprecated_moz_border_mac_classic_theme.diff

Comment 23

[Stefan [:stefanh]]

Comment on attachment 8966136 [details] [diff] [review]
Bug_1444740_changing_deprecated_moz_border_mac_classic_theme.diff

Thanks a lot :-)

Comment 24

[Stefan [:stefanh]]

BTW, the Modern theme seems to depend a lot on -moz-border-*-colors so we might want to do that in a separate bug. Doing that, we could land both the mac and non-mac parts together and close this bug.

Comment 25

[Frank-Rainer Grahl (:frg)]

Attachment: 1444740-part1-moz-border-osx.patch (PUSHED comment #43 #44)

Stefan, I missed that this got r+. Can I check it into central?

I would like to do 3 parts in this bug (classic macOS; classic else; Modern) rather than opening a new one. Central is busted anyway so no hurry.

Comment 26

[Frank-Rainer Grahl (:frg)]

Attachment: 1444740-part2-WIP-moz-border.patch

MMR, would you like to continue this with the classic theme?

Comment 27

[Stefan [:stefanh]]

(In reply to Frank-Rainer Grahl (:frg) from comment #25)
> Created attachment 8989168 [details] [diff] [review]
> 1444740-part1-moz-border-osx.patch
> 
> Stefan, I missed that this got r+. Can I check it into central?

Sure.

> 
> I would like to do 3 parts in this bug (classic macOS; classic else; Modern)
> rather than opening a new one. Central is busted anyway so no hurry.

It's harder to track changes when the bug is leaved open for a long time and multiple patches are checked in, but I guess that might not matter in this case.

Comment 28

[MMR]

(In reply to Frank-Rainer Grahl (:frg) from comment #26)
> Created attachment 8989170 [details] [diff] [review]
> 1444740-part2-WIP-moz-border.patch
> 
> MMR, would you like to continue this with the classic theme?

It would be my pleasure. I should be able to come up with a patch for "classic else" in a couple of days. And yes, It would be awesome if in the meantime, we could find a good reviewer!

Comment 29

[Frank-Rainer Grahl (:frg)]

Attachment: Capture.PNG

Great. Set me as reviewer. I can check under Windows and Linux.

The part 2 is based on yyour previous patch and does include all occurences. What I see under Windows especially in the preferences is that some edit files are pretty tight with no padding and that boxes with a scrollvalue are missing the right border. But these might have been caused by other removals in mozilla code. I didn't check under Windows 7 yet. Looking forward to test your first new patch.

Comment 30

[Frank-Rainer Grahl (:frg)]

There seems to be a general problem with the spinbuttons not working so has nothing to do with the borders. Probably some xbl bindings are missing.

Comment 31

[Frank-Rainer Grahl (:frg)]

For the spinbuttons it Bug 1446329 needs to be ported to SeaMonkey too. The entryfield seems to have the same height in older builds. So probabyl only some cleanup needed.

Comment 32

[Frank-Rainer Grahl (:frg)]

Have a wip patch for the spin buttons in bug 1474490.

Comment 33

[MMR]

Attachment: 1444740-part3-WIP-moz-border-modern.patch

I have come up with the first patch for modern theme. It looks less ugly now.

Before & After: https://imgur.com/a/AbWh3Xq

Comment 34

[MMR]

Attachment: 1444740-part3-WIP-moz-border-modern.patch

Fixed some bugs in the patch. Improved readability by adding more shorthand notations.

Comment 35

[Frank-Rainer Grahl (:frg)]

Attachment: Capture.PNG

I Installed 2.57 with both patches in a Windows 7 VM.

The second patch already looks mostly good but you are right it might need some padding in the input fields. I just looked at some prefs especially the search fields. Could you check if these where using moz-border before. Some were imho already a little too tight.

The borders in Modern are so thick because this was previously used for a 3D effect. Each color represented one pixel width. Maybe you could replace this with a standard shadow effect. 

   border: 2px solid;
-  -moz-border-top-colors: #858B97 #2D3B49;
+  border-top-color: #858B97;

-  border: 6px solid;
-  -moz-border-top-colors: transparent transparent transparent #000000 #BBC6D1 #B1BBC9;
+  border: 6px solid transparent;

Comment 36

[Frank-Rainer Grahl (:frg)]

Attachment: Capture1.PNG

Preferences->Browser->Helper Application is one example which looks really tight now. But this might be because of other changes.

On the left 2.57. Right 2.53.

Comment 37

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8989170 [details] [diff] [review]
1444740-part2-WIP-moz-border.patch

rsx11m could you try the Classic patch using Linux. Looks mostly good under Windows. The menus lost their shadow but it is so subtile that I would care much about it.

Comment 38

[Frank-Rainer Grahl (:frg)]

Attachment: menus.PNG

One problem with the menus in the Classic patch. If you use the Windows Classic theme the text is black now instead of white if you highlight a menu. This needs to be fixed. The lost shadow at the right and at the bottom is imho not important.

search bar seems not to be affected so the pref changes comes from somewhere else. probably a textbox change in toolkit for Quantum we need to fix in another bug.

Aside from the menu problem the classic patch seem good to go.

Comment 39

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8993890 [details] [diff] [review]
1444740-part3-WIP-moz-border-modern.patch

Covers all occurences but I would like to see a patch with the black border fixed.  A good start.

Comment 40

[MMR]

Thank you for your kind inputs FRG. Let me formulate the changes that I have to do before I get lost.

I. Classic

a. Figure out if some component used -moz-border before the 1444740-part2-WIP-moz-border-classic.patch, if yes, fix the padding
1. need help with determining the necessary file in which i will be able to find the component
2. how much padding should be set?

b. Fix the black color of the highlighted menu item (it should be white)
1. Again, I need help with the relevant file in which I will be able to fix this
2. Also, is this related to -moz-border? 

II. Modern

a. Replace all occurrences of border-color usage for shadow with standard shadow (box-shadow)
1. Need help with the parameters that I must use for the box-shadow effect (blur, spread, color, etc.)
2. Will most probably look for this pattern while replacing: top & right borders are same and bottom and left borders are same

b. fix a black border
1. I need to know which black border needs fixing.
2. Where to find it in the CSS files

I have formulated the above points just for the sake of clarity and progress tracking purposes. As soon I have enough information regarding any of the above issues, I will start working on it. 

Also in your comment 35, you mentioned second patch, I assumed that you meant classic patch

Comment 41

[Frank-Rainer Grahl (:frg)]

I. Classic

> a. Figure out if some component used -moz-border before the 1444740-part2-WIP-moz-border-classic.patch, if yes, fix the padding

Looked at the patch and I think this is good. Where it occurs probably related to another bug. We should find out which one caused it and fix in a follow-up. First things first.

> 1. need help with determining the necessary file in which i will be able to find the component
> 2. how much padding should be set?

See a. Just needs testing under Linux now to see if it works there too.

> b. Fix the black color of the highlighted menu item (it should be white)

You are right. Should be unrelated to the border change. I also remembered there might already be a bug. Just ignore. We will fix it in another bug.

Best concentrate on Modern and find a way to get some shadows or transient effectes into it. For Classic maybe look at the Firefox and Thunderbird menu styling and see how they handle the shadow.

Comment 42

[Ian Neal]

Comment on attachment 8989168 [details] [diff] [review]
1444740-part1-moz-border-osx.patch (PUSHED comment #43 #44)

a=me

Comment 43

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e9b76764d29c
Part 1. Replace obsolete moz-border css styles in SeaMonkey macOS classic theme. r=stefanh a=IanN

Comment 44

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8989168 [details] [diff] [review]
1444740-part1-moz-border-osx.patch (PUSHED comment #43 #44)

Part 1 pushed
https://hg.mozilla.org/releases/comm-esr60/rev/fd981f3f431c

Comment 45

[MMR]

Attachment: 1444740-part3-WIP-moz-border-modern.patch

Removed the black border around the menulist. Fixed more black borders around scroll bars.

Comment 46

[MMR]

Attachment: box_shadow_problem.PNG

I tried simulating the multiple border colors with multiple box-shadow effects. I ran into this problem :(

I don't think we can convert it using box-shadow. 

What I also tried was, for each color in -moz-border-*-colors, I created a new div with that border color. But the nesting looks too bad, although the end result looks great.

So for example, to mimic
  -moz-border-top-colors: #000000 #BBC4D1 #B3BBC9;
  -moz-border-bottom-colors: #000000 #99A7B7 #9EACBB;
  -moz-border-right-colors: #000000 #99A7B7 #9EACBB;
  -moz-border-left-colors: #000000 #BBC4D1 #B3BBC9;

I tried doing
  .inner1
  {
    border: 5px solid #000000;
  }
  .inner2
  {
    border: 5px solid;
    padding: 0px;
    border-top-color:#BBC4D1;
    border-right-color:#99A7B7;
    border-bottom-color:#99A7B7;
    border-left-color:#BBC4D1;
  }
  .inner3
  {
    border: 5px solid;
    padding: 0px;
    border-top-color:#B3BBC9;
    border-right-color:#9EACBB;
    border-bottom-color:#9EACBB;
    border-left-color:#B3BBC9;
  }

Comment 47

[Frank-Rainer Grahl (:frg)]

I wouldn't try to mimic it a 100%. The border styles groove and ridge might look good when using the modern colors.

You can experiment with them here:

https://www.w3schools.com/css/css_border.asp

Just did a quick test:
    border-left-style: groove;
    border-left-color: red;
    border-right-color: orange;
    border-top-color: blue;
    border-bottom-color: green;
    border-right-style: ridge;
    border-top-style: groove;
    border-bottom-style: ridge;
    border-width: 4px;

Otherwise we could ask paenglab who did the TB conversion. But I think more sophisticated effects will affect the performance so I would try to keep it simple.

Comment 48

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8995728 [details]
box_shadow_problem.PNG

see last comment

Comment 49

[Stefan [:stefanh]]

(In reply to Frank-Rainer Grahl (:frg) from comment #44)
> Comment on attachment 8989168 [details] [diff] [review]
> 1444740-part1-moz-border-osx.patch (PUSHED comment #43 #44)
> 
> Part 1 pushed
> https://hg.mozilla.org/releases/comm-esr60/rev/fd981f3f431c

I just noticed that you pushed the wrong patch. The correct patch (which has r+) is attachment #8966136 [details] [diff] [review].

Comment 50

[Stefan [:stefanh]]

Actually, you didn't push the wrong patch (assumed interdiff worked)...

Comment 51

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8995726 [details] [diff] [review]
1444740-part3-WIP-moz-border-modern.patch

This still needs some work. The borders around buttons and tabs are too thick and uniform. Tabs still have the black border. Buttons seem to be washed out.

Comment 52

[Frank-Rainer Grahl (:frg)]

Oh with the current borders the pref now look like embossed. Not bad but completly different from  before. Maybe with a thinnner shadow as suggested above it would look more like the current one.

Comment 53

[Frank-Rainer Grahl (:frg)]

Clearing assignment :(

Comment 54

[VEG]

You can use this code snippet to recreate standard Windows Classic context menus. I created it for Firefox's userchrome.css, but it seems that it may be useful here also.

@media (-moz-windows-classic)
{
/* Windows Classic theme: Make context menu 3D again! */
menupopup
{
padding: 1px !important;
border: 2px solid transparent !important;
border-image-source: url('data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHZpZXdCb3g9IjAgMCA2IDYiIHdpZHRoPSI2IiBoZWlnaHQ9IjYiPjxyZWN0IHg9IjAiIHk9IjAiIHdpZHRoPSI2IiBoZWlnaHQ9IjYiIGZpbGw9IlRocmVlRERhcmtTaGFkb3ciLz48cmVjdCB4PSIwIiB5PSIwIiB3aWR0aD0iNSIgaGVpZ2h0PSI1IiBmaWxsPSJUaHJlZURMaWdodFNoYWRvdyIvPjxyZWN0IHg9IjEiIHk9IjEiIHdpZHRoPSI0IiBoZWlnaHQ9IjQiIGZpbGw9IlRocmVlRFNoYWRvdyIvPjxyZWN0IHg9IjEiIHk9IjEiIHdpZHRoPSIzIiBoZWlnaHQ9IjMiIGZpbGw9IlRocmVlREhpZ2hsaWdodCIvPjxyZWN0IHg9IjIiIHk9IjIiIHdpZHRoPSIyIiBoZWlnaHQ9IjIiIGZpbGw9IlRocmVlREZhY2UiLz48L3N2Zz4=') !important;
border-image-slice: 2 !important;
border-image-width: 2px !important;
border-image-repeat: repeat !important;
}
}

Comment 55

[Frank-Rainer Grahl (:frg)]

Taking the Bug for Bill

Comment 56

[Frank-Rainer Grahl (:frg)]

Attachment: 1444740-2-moz-border.patch

Classic theme patch from Bill. Looks good to me

Comment 57

[Frank-Rainer Grahl (:frg)]

Attachment: 1444740-3-moz-border-modern.patch (PUSHED comment #60)

Patch for Modern. Prefs are looking a bit flat now but otherwise looks good to me.

Comment 58

[Frank-Rainer Grahl (:frg)]

Attachment: 1444740-2-moz-border.patch (PUSHED comment #60)

Minor typo border-top-color: ThreeDDarkShadoww; fixed

Comment 59

[Ian Neal]

Comment on attachment 9123969 [details] [diff] [review]
1444740-3-moz-border-modern.patch  (PUSHED comment #60)

I think this is an improvement, I did think about looking at box-shadow but should probably be spun off into a new bug.

Comment 60

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/3f673febb509
Part 2. Replace obsolete moz-border css styles in SeaMonkey Linux/Windows classic theme. r=IanN
https://hg.mozilla.org/comm-central/rev/4d69d0a1d74a
Part 3. Replace obsolete moz-border css styles in SeaMonkey Modern theme. r=IanN