Closed Bug 1378075 Opened 2 years ago Closed 2 years ago

Missing visual separation of main window elements with W7 classic desktop

Categories

(SeaMonkey :: UI Design, defect)

SeaMonkey 2.51 Branch
x86_64
Windows 7
defect
Not set

Tracking

(seamonkey2.49esr unaffected, seamonkey2.51 wontfix, seamonkey2.52 wontfix, seamonkey2.53 fixed, seamonkey2.54 fixed)

RESOLVED FIXED
seamonkey2.54
Tracking Status
seamonkey2.49esr --- unaffected
seamonkey2.51 --- wontfix
seamonkey2.52 --- wontfix
seamonkey2.53 --- fixed
seamonkey2.54 --- fixed

People

(Reporter: tilmann.reh, Assigned: wgianopoulos)

Details

(Keywords: reproducible)

Attachments

(5 files, 1 obsolete file)

Attached image 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.
Component: General → UI Design
Keywords: reproducible
OS: Unspecified → Windows 7
Hardware: Unspecified → x86_64
Attached image Clipboard.png β€”
Yes looks that way. 2.49.1 is still ok.
Caused by bug 1341036 for the menu borders.
Sorry copy/paste error Bug 1343196 in toolbar.css
For the toolbarbutton it is Bug 1340495
Attached file test userChrome.css β€”
Can you please test to see if using the userChrome.css file fixes all the issues?
QA Contact: wgianopoulos
Assignee: nobody → wgianopoulos
QA Contact: wgianopoulos
Status: NEW → ASSIGNED
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.
Hello Bill, using your userChrome.css apparently fixes the problem.
Many thanks!
(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/
Attachment #8886609 - Flags: review?(frgrahl)
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.
Attachment #8886609 - Flags: review?(iann_bugzilla)
Attachment #8886609 - Flags: review?(frgrahl)
Attachment #8886609 - Flags: review+
(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.
(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.
(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?
Flags: needinfo?(wgianopoulos)
(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.
Flags: needinfo?(wgianopoulos)
1378075-restore-windows-classic-fallbacks.patch

Addresses FRG's issues.
Attachment #8886609 - Attachment is obsolete: true
Attachment #8886609 - Flags: review?(iann_bugzilla)
Attachment #8891969 - Flags: review?(iann_bugzilla)
Attached file interdiff.txt β€”
Interdiff to make review easier.
Comment on attachment 8891969 [details] [diff] [review]
1378075-restore-windows-classic-fallbacks.patch

Works fine
Attachment #8891969 - Flags: review+
Comment on attachment 8891969 [details] [diff] [review]
1378075-restore-windows-classic-fallbacks.patch

LGTM r=me
Attachment #8891969 - Flags: review?(iann_bugzilla) → review+
Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/58eaeea16828
Restore Windows Classic Desktop fallbacks removed by bug 1340495  and 1343196. r=frg,IanN
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.54
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
Attachment #8891969 - Flags: approval-comm-beta?
Comment on attachment 8891969 [details] [diff] [review]
1378075-restore-windows-classic-fallbacks.patch

a=me
Attachment #8891969 - Flags: approval-comm-beta? → approval-comm-beta+
You need to log in before you can comment on or make changes to this bug.