1378075 - Missing visual separation of main window elements with W7 classic desktop

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Tilmann Reh]

Attachment: SM251-Menus.png

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:54.0) Gecko/20100101 Firefox/54.0 SeaMonkey/2.51
Build ID: 20170619124733

Steps to reproduce:

Update from SM 2.50 to SM 2.51


Actual results:

Under Windows 7 with classic desktop, there is no visual separation any more between menu line, symbol bar, adress bar, bookmarks etc.
It's all a single grey surface with elements loosely spread on it.
This applies to both MailNews and Browser.



Expected results:

The visual separation should not have changed. In fact, it changed only for Windows 7 with classic desktop (as far as I could test), while with Aero or at Windows 8.1 the separating lines are still there.
With SM 2.50, the display was correct for all desktop styles.

Comment 1

[Frank-Rainer Grahl (:frg)]

Attachment: Clipboard.png

Yes looks that way. 2.49.1 is still ok.

Comment 2

[Frank-Rainer Grahl (:frg)]

Caused by bug 1341036 for the menu borders.

Comment 3

[Frank-Rainer Grahl (:frg)]

Sorry copy/paste error Bug 1343196 in toolbar.css

Comment 4

[Frank-Rainer Grahl (:frg)]

For the toolbarbutton it is Bug 1340495

Comment 5

[Bill Gianopoulos [:WG9s]]

Attachment: test userChrome.css

Can you please test to see if using the userChrome.css file fixes all the issues?

Comment 6

[Bill Gianopoulos [:WG9s]]

Hmm if these are supposed to be fallback should i be defining them at the end of the file?  not sure what order these .css files are applied.  if the toolkit ones are applied after the ones in the SeamonkeyMonkey classic theme then I guess to duplicate the old behavior these definitions need to be at the end of the appropriate files.  if toolkit was applied first, then I should add them at the front.

Comment 7

[Tilmann Reh]

Hello Bill, using your userChrome.css apparently fixes the problem.
Many thanks!

Comment 8

[Bill Gianopoulos [:WG9s]]

(In reply to Tilmann Reh from comment #7)
> Hello Bill, using your userChrome.css apparently fixes the problem.
> Many thanks!

Thanks for testing this.  I should be submitting a patch for this later today, now that rebooting my Windows build machine has restored its ability to build SeaMonkey.  Nightly builds will still not work with Win7 classic because of Mozilla bug 1380629, so If you want to work with SeaMonkey nightly builds using Win7 classic desktop and the default SeaMonkey Theme, I would suggest you use my builds located here:

https://www.wg9s.com/mozilla/seamonkey/

Comment 9

[Bill Gianopoulos [:WG9s]]

Attachment: 1378075-restore-windows-classic-fallbacks.patch

Comment 10

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8886609 [details] [diff] [review]
1378075-restore-windows-classic-fallbacks.patch

Bill looks fine and works fine. Just two questions:

For Bug 1340495

> /* ::::: toolbarbutton menu ::::: */
> 
> .toolbarbutton-menu-dropmarker {
>   -moz-appearance: none !important;
> -  border: none !important;

You didn't add the "border: none". Was this intentional?

For Bug 1343196

> /* ::::: lightweight theme ::::: */
  
>  menubar:-moz-lwtheme,
>  toolbox:-moz-lwtheme,
>  toolbar:-moz-lwtheme {
>    -moz-appearance: none;
> -  background: none;
> -  border-color: transparent;
>  }

Shouldn't this block be in too? Lightweight themes do work in SeaMonkey 


r+ with the two questions answered. I asked IanN Sunday if he can do an additional review when I am finished. My css knowledge isn't hat good.

Comment 11

[Bill Gianopoulos [:WG9s]]

(In reply to Frank-Rainer Grahl (:frg) from comment #10)
> Comment on attachment 8886609 [details] [diff] [review]
> 1378075-restore-windows-classic-fallbacks.patch
> 
> Bill looks fine and works fine. Just two questions:
> 
> For Bug 1340495
> 
> > /* ::::: toolbarbutton menu ::::: */
> > 
> > .toolbarbutton-menu-dropmarker {
> >   -moz-appearance: none !important;
> > -  border: none !important;
> 
> You didn't add the "border: none". Was this intentional?


This was intentional.  There is code already here that sets the border differently, so I purposely tried not to mess that up.

/**
 * Fix the missing dropmarker bevel in Classic (bug 216266)
 * Not used in Firefox but used elsewhere,
 **/

@media (-moz-windows-classic) {

  .toolbarbutton-menubutton-dropmarker {
    border: 1px solid;
    padding: 2px;
    -moz-border-top-colors: transparent;
    -moz-border-right-colors: transparent;
    -moz-border-bottom-colors: transparent;
    -moz-border-left-colors: transparent;
  }


> 
> For Bug 1343196
> 
> > /* ::::: lightweight theme ::::: */
>   
> >  menubar:-moz-lwtheme,
> >  toolbox:-moz-lwtheme,
> >  toolbar:-moz-lwtheme {
> >    -moz-appearance: none;
> > -  background: none;
> > -  border-color: transparent;
> >  }
> 
> Shouldn't this block be in too? Lightweight themes do work in SeaMonkey 

I will check on that and either answer why it is not included or post a new patch.

Comment 12

[Bill Gianopoulos [:WG9s]]

(In reply to Bill Gianopoulos [:WG9s] from comment #11)
> (In reply to Frank-Rainer Grahl (:frg) from comment #10)
> This was intentional.  There is code already here that sets the border
> differently, so I purposely tried not to mess that up.
> 
> /**
>  * Fix the missing dropmarker bevel in Classic (bug 216266)
>  * Not used in Firefox but used elsewhere,
>  **/
> 
> @media (-moz-windows-classic) {
> 
>   .toolbarbutton-menubutton-dropmarker {
>     border: 1px solid;
>     padding: 2px;
>     -moz-border-top-colors: transparent;
>     -moz-border-right-colors: transparent;
>     -moz-border-bottom-colors: transparent;
>     -moz-border-left-colors: transparent;
>   }

It would probably be clearer here to figure out which border-style is desired here and fold the bug 216266 code into this change.

Comment 13

[Ian Neal]

(In reply to Bill Gianopoulos [:WG9s] from comment #11)
> (In reply to Frank-Rainer Grahl (:frg) from comment #10)
> > 
> > For Bug 1343196
> > 
> > > /* ::::: lightweight theme ::::: */
> >   
> > >  menubar:-moz-lwtheme,
> > >  toolbox:-moz-lwtheme,
> > >  toolbar:-moz-lwtheme {
> > >    -moz-appearance: none;
> > > -  background: none;
> > > -  border-color: transparent;
> > >  }
> > 
> > Shouldn't this block be in too? Lightweight themes do work in SeaMonkey 
> 
> I will check on that and either answer why it is not included or post a new
> patch.

Did you check?

Comment 14

[Bill Gianopoulos [:WG9s]]

(In reply to Bill Gianopoulos [:WG9s] from comment #11)
> (In reply to Frank-Rainer Grahl (:frg) from comment #10)
> > Comment on attachment 8886609 [details] [diff] [review]
> > 1378075-restore-windows-classic-fallbacks.patch
> > 
> > Bill looks fine and works fine. Just two questions:
> > 
> > For Bug 1340495
> > 
> > > /* ::::: toolbarbutton menu ::::: */
> > > 
> > > .toolbarbutton-menu-dropmarker {
> > >   -moz-appearance: none !important;
> > > -  border: none !important;
> > 
> > You didn't add the "border: none". Was this intentional?
> 
> 
> This was intentional.  There is code already here that sets the border
> differently, so I purposely tried not to mess that up.
> 

Well that was a case of right church wrong pew.  It was under toolbarbutton-menubutton-dropmarker that I intended to remove the border: none.

> 
> 
> > 
> > For Bug 1343196
> > 
> > > /* ::::: lightweight theme ::::: */
> >   
> > >  menubar:-moz-lwtheme,
> > >  toolbox:-moz-lwtheme,
> > >  toolbar:-moz-lwtheme {
> > >    -moz-appearance: none;
> > > -  background: none;
> > > -  border-color: transparent;
> > >  }
> > 
> > Shouldn't this block be in too? Lightweight themes do work in SeaMonkey 
> 
> I will check on that and either answer why it is not included or post a new
> patch.

This was just I somehow missed the lightweight theme change while creating the patch.  New patch coming today.

Comment 15

[Bill Gianopoulos [:WG9s]]

Attachment: 1378075-restore-windows-classic-fallbacks.patch

1378075-restore-windows-classic-fallbacks.patch

Addresses FRG's issues.

Comment 16

[Bill Gianopoulos [:WG9s]]

Attachment: interdiff.txt

Interdiff to make review easier.

Comment 17

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8891969 [details] [diff] [review]
1378075-restore-windows-classic-fallbacks.patch

Works fine

Comment 18

[Ian Neal]

Comment on attachment 8891969 [details] [diff] [review]
1378075-restore-windows-classic-fallbacks.patch

LGTM r=me

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/58eaeea16828
Restore Windows Classic Desktop fallbacks removed by bug 1340495  and 1343196. r=frg,IanN

Comment 20

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8891969 [details] [diff] [review]
1378075-restore-windows-classic-fallbacks.patch

[Approval Request Comment]
Regression caused by (bug #): bug 1340495 and bug 1343196.
User impact if declined: ugly theme
Testing completed (on m-c, etc.): c-c c-b
Risk to taking this patch (and alternatives if risky): none already broken
String changes made by this patch: none

Comment 21

[Ian Neal]

Comment on attachment 8891969 [details] [diff] [review]
1378075-restore-windows-classic-fallbacks.patch

a=me

Comment 22

[Frank-Rainer Grahl (:frg)]

https://hg.mozilla.org/releases/comm-beta/rev/0cc3de3a48d45173b9f03552d755decfd216781a