Closed Bug 583510 Opened 9 years ago Closed 8 years ago

Toolbar separators are invisible

Categories

(Firefox :: Theme, defect)

All
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 11

People

(Reporter: rvjanc, Assigned: markus.magnuson)

References

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

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.
Whiteboard: 4b2
Whiteboard: 4b2 → 4b3
This is still broken in FF4 beta3
This is still broken in FF4 beta4
Whiteboard: 4b3 → 4b3 4b4
Whiteboard: 4b3 4b4 → 4b3 4b4 4b5
This is still broken in FF4 beta5
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
Blocks: 574090
Component: Toolbars → Theme
QA Contact: toolbars → theme
This is still broken in FF4 beta6
Whiteboard: 4b3 4b4 4b5 → 4b3 4b4 4b5 4b6
This is still broken in FF4 beta7
Whiteboard: 4b3 4b4 4b5 4b6 → 4b3 4b4 4b5 4b6 4b7
This is still broken in FF4 beta8
Whiteboard: 4b3 4b4 4b5 4b6 4b7 → 4b3 4b4 4b5 4b6 4b7 4b8
These updates aren't really useful.
What would be more useful is a regression range, assuming this is a regression.
Whiteboard: 4b3 4b4 4b5 4b6 4b7 4b8
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.
This is probably a regression from bug 559033.
Blocks: 559033
Keywords: regression
Hmm, this is a very convincing argument for changing the box align back to stretch.
Hardware: x86 → All
Version: unspecified → Trunk
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"
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.
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;
}
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.
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.
Assignee: nobody → markus.magnuson
Status: NEW → ASSIGNED
Attachment #574876 - Flags: review?(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.
Attachment #574876 - Flags: review?(mstange)
Attachment #574876 - Flags: review?(dao)
Attachment #574876 - Flags: review+
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.
Attachment #574876 - Flags: review?(dao) → review-
Here's the same change but applied in browser.css instead.
Attachment #574876 - Attachment is obsolete: true
Attachment #577092 - Flags: review?(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!
Attachment #577092 - Flags: review?(dao) → review+
Same patch as the last one but with the requested comment added. Thanks for the feedback!
Attachment #577092 - Attachment is obsolete: true
Attachment #577093 - Flags: review?(dao)
Attachment #577093 - Flags: review?(dao) → review+
What additional reviews do I need for this?
(In reply to Markus Amalthea Magnuson from comment #22)
> What additional reviews do I need for this?

none
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/58394a391fd2
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Depends on: 726132
You need to log in before you can comment on or make changes to this bug.