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

NEW
Unassigned

Status

enhancement
Last year
7 months ago

People

(Reporter: frg, Unassigned)

Tracking

(Blocks 1 bug, {leave-open})

Trunk
Dependency tree / graph

SeaMonkey Tracking Flags

(seamonkey2.57esr affected)

Details

Attachments

(9 attachments, 8 obsolete attachments)

2.66 KB, text/plain
Details
9.89 KB, patch
frg
: review+
iann_bugzilla
: approval-comm-esr60+
Details | Diff | Splinter Review
20.53 KB, patch
Details | Diff | Splinter Review
5.92 KB, image/png
Details
8.29 KB, image/png
Details
41.42 KB, image/png
Details
67.77 KB, image/png
Details
65.27 KB, patch
Details | Diff | Splinter Review
5.67 KB, image/png
Details
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.
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,
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,
Flags: needinfo?(frgrahl)
Posted file 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.
Flags: needinfo?(frgrahl)
The list was actually produced by Total Commander where I searched for 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)
Flags: needinfo?(frgrahl)
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
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,
Flags: needinfo?(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.
Flags: needinfo?(stefanh)
Flags: needinfo?(frgrahl)
Assignee: nobody → mmrxyz
Status: NEW → ASSIGNED
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
Attachment #8960578 - Attachment is obsolete: true
Attachment #8961255 - Flags: review?(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).
Attachment #8961255 - Flags: review?(stefanh)
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
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.
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.
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?
Attachment #8964165 - Flags: review?(stefanh)
Attachment #8964165 - Attachment is obsolete: true
Attachment #8964165 - Flags: review?(stefanh)
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
Attachment #8964166 - Attachment is obsolete: true
Attachment #8964167 - Flags: review?(stefanh)
Sorry, I'm a bit slow atm, but I'll try to look at this over the weekend.
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
Attachment #8964167 - Flags: review?(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.
Attachment #8961255 - Attachment is obsolete: true
Attachment #8964167 - Attachment is obsolete: true
Attachment #8966136 - Flags: review?(stefanh)
Comment on attachment 8966136 [details] [diff] [review]
Bug_1444740_changing_deprecated_moz_border_mac_classic_theme.diff

Thanks a lot :-)
Attachment #8966136 - Flags: review?(stefanh) → review+
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.
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.
Attachment #8966136 - Attachment is obsolete: true
Flags: needinfo?(stefanh)
Attachment #8989168 - Flags: review+
Attachment #8989168 - Flags: approval-comm-esr60?
Keywords: leave-open
MMR, would you like to continue this with the classic theme?
Flags: needinfo?(mmrxyz)
(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.
Flags: needinfo?(stefanh)
(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!
Flags: needinfo?(mmrxyz)
Posted image 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.
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.
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.
Have a wip patch for the spin buttons in bug 1474490.
I have come up with the first patch for modern theme. It looks less ugly now.

Before & After: https://imgur.com/a/AbWh3Xq
Attachment #8993889 - Flags: review?(frgrahl)
Fixed some bugs in the patch. Improved readability by adding more shorthand notations.
Attachment #8993889 - Attachment is obsolete: true
Attachment #8993889 - Flags: review?(frgrahl)
Attachment #8993890 - Flags: review?(frgrahl)
Posted image 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;
Posted image 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 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.
Attachment #8989170 - Flags: feedback?(rsx11m.pub)
Posted image 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.
Attachment #8993969 - Flags: feedback?(mmrxyz)
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.
Attachment #8993890 - Flags: review?(frgrahl) → feedback+
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
Flags: needinfo?(frgrahl)
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.
Flags: needinfo?(frgrahl)
Comment on attachment 8989168 [details] [diff] [review]
1444740-part1-moz-border-osx.patch (PUSHED comment #43 #44)

a=me
Attachment #8989168 - Flags: approval-comm-esr60? → approval-comm-esr60+
Pushed by frgrahl@gmx.net:
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 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
Attachment #8989168 - Attachment description: 1444740-part1-moz-border-osx.patch → 1444740-part1-moz-border-osx.patch (PUSHED comment #43 #44)
Removed the black border around the menulist. Fixed more black borders around scroll bars.
Attachment #8993890 - Attachment is obsolete: true
Attachment #8995726 - Flags: review?(frgrahl)
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;
  }
Attachment #8995728 - Flags: feedback?(frgrahl)
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 on attachment 8995728 [details]
box_shadow_problem.PNG

see last comment
Attachment #8995728 - Flags: feedback?(frgrahl)
(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].
Flags: needinfo?(frgrahl)
Actually, you didn't push the wrong patch (assumed interdiff worked)...
Flags: needinfo?(frgrahl)
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.
Attachment #8995726 - Flags: review?(frgrahl) → feedback+
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.
Clearing assignment :(
Assignee: mmrxyz → nobody
Status: ASSIGNED → NEW
Attachment #8989170 - Flags: feedback?(rsx11m.pub)
Attachment #8993969 - Flags: feedback?(mmrxyz)
You need to log in before you can comment on or make changes to this bug.