583510 - Toolbar separators are invisible

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[RobertJ]

User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b2) Gecko/20100720 Firefox/4.0b2
Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b2) Gecko/20100720 Firefox/4.0b2

The toolbar separators on Mac are invisible on the toolbar. I also tried redefining them as a solid wide line in the userchrome.css since the dotted separator is a little hard to see on Mac. It was changed in the customize window to a solid wide line; however, when I dragged and dropped on the toolbar it disappeared.

Reproducible: Always

Actual Results:  
See description.

Expected Results:  
See the separator on the toolbar.

Using default theme.

Comment 1

[RobertJ]

This is still broken in FF4 beta3

Comment 2

[RobertJ]

This is still broken in FF4 beta4

Comment 3

[RobertJ]

This is still broken in FF4 beta5

Comment 4

[Jean-Yves Perrier [:teoli]]

I see the same with the latest trunk on Mac OS X 10.5.

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:2.0b6pre) Gecko/20100909 Firefox/4.0b6pre

Comment 5

[RobertJ]

This is still broken in FF4 beta6

Comment 6

[RobertJ]

This is still broken in FF4 beta7

Comment 7

[RobertJ]

This is still broken in FF4 beta8

Comment 8

[Dão Gottwald [:dao]]

These updates aren't really useful.
What would be more useful is a regression range, assuming this is a regression.

Comment 9

[RobertJ]

Sorry if I broke protocol. 

All I know is I first saw this on FF4b2 on OSX6. I don't test nightly builds so anything prior to FF4b2 is an unknown for me. As you can see from comment 4, teoli2003 confirmed it on 4.0b6pre on OSX5.

This is not and has never been a bug on 3.6.x Other than that I have nothing else to add.

Comment 10

[Dão Gottwald [:dao]]

This is probably a regression from bug 559033.

Comment 11

[Markus Stange [:mstange]]

Hmm, this is a very convincing argument for changing the box align back to stretch.

Comment 12

[RobertJ]

As an experiment I installed this theme nuvola-1.9.7 on FF4b8 on OSX 10.6.5. 

The toolbar separators work just fine. I may poke around the omni.jar to see if I can find it. Will start by searching for "box align"

Comment 13

[Markus Stange [:mstange]]

You don't really have to do anything here, except if you want to provide a patch.

The problem is in pinstripe's browser.css. We'll either need to give toolbar separators a min-height, or we'll have to revert the toolbars to -moz-box-align: stretch. The rationale for -moz-box-align: center is in bug 559033 starting at bug 559033 comment 11.

Comment 14

[RobertJ]

I added this to my userChrome.css. It solved the problem for me.

toolbarseparator {
  min-height: 17px !important;
}

Note that I checked this out for both the "normal" OSX dotted separator which I find hard to see and a beveled one which is easier to see that I add through the userChrome.css and they seem to work just fine.

My beveled userChrome.css

toolbarseparator {
  margin : 6px  !important;
  border-right : 1px solid #FFFFFF !important;
  border-left : 1px solid #808080 !important;
  width : 2px !important;
  opacity: 1.0 !important;
}

Comment 15

[RobertJ]

After playing around a bit I found this combination works best

toolbarseparator {
min-height: 20px !important;
margin: 0px 6px  !important;
<other properties>
}

The separator is vertically the same size as the buttons and the toolbar is not enlarged.

Comment 16

[Markus Amalthea Magnuson]

Attachment: Show toolbar separators correctly on Mac.

Here's a patch that fixes the separators on Mac.

I've set a default min-height of 22px that will be used for text-only and icon-only toolbars, and another min-height of 36px for icon+text toolbars.

Comment 17

[Markus Stange [:mstange]]

Comment on attachment 574876 [details] [diff] [review]
Show toolbar separators correctly on Mac.

Sounds good to me, let's see what Dão thinks.

Comment 18

[Dão Gottwald [:dao]]

Comment on attachment 574876 [details] [diff] [review]
Show toolbar separators correctly on Mac.

I think I'd prefer this to be in browser.css, since this is where we (unfortunately) set -moz-box-align: center.

Comment 19

[Markus Amalthea Magnuson]

Attachment: Show toolbar separators correctly on Mac.

Here's the same change but applied in browser.css instead.

Comment 20

[Dão Gottwald [:dao]]

Comment on attachment 577092 [details] [diff] [review]
Show toolbar separators correctly on Mac.

Can you add a comment, saying that this is needed because of "-moz-box-align: center" a couple of lines above?

Thanks!

Comment 21

[Markus Amalthea Magnuson]

Attachment: Show toolbar separators correctly on Mac.

Same patch as the last one but with the requested comment added. Thanks for the feedback!

Comment 22

[Markus Amalthea Magnuson]

What additional reviews do I need for this?

Comment 23

[Dão Gottwald [:dao]]

(In reply to Markus Amalthea Magnuson from comment #22)
> What additional reviews do I need for this?

none

Comment 24

[Dão Gottwald [:dao]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/58394a391fd2

Comment 25

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/58394a391fd2