Closed Bug 580964 Opened 14 years ago Closed 14 years ago

Style toolbarbutton[type="menu"]

Categories

(Firefox :: Theme, defect)

All
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 4.0b8

People

(Reporter: mstange, Assigned: mstange)

Details

Attachments

(1 file, 2 obsolete files)

Attached patch v1 (obsolete) — Splinter Review
Give them the right dropmarker image and fix up the text for extensions that display it inside the button, like Feedback/Input.
Attachment #459368 - Flags: review?(dao)
Comment on attachment 459368 [details] [diff] [review]
v1

Please use .toolbarbutton-1 instead of toolbarbutton. We require the toolbarbutton-1 class right now so that items can opt out of the primary toolbarbutton appearance. type="menu-button" is an unfortunate exception right now.

Even if we wanted to change this, we'd want to do it consistently across platforms.

>+toolbar[mode="icons"] toolbarbutton[type="menu"]:not(.bookmark-item) > .toolbarbutton-text {
>+  font: -moz-dialog;
>+  margin-top: 0;
>+  -moz-margin-start: 3px;
>+  -moz-margin-end: 1px;
>+}

The Feedback button is only relevant for betas and doesn't seem to follow a common pattern, so I don't really want this code here. If we are serious about supporting text in toolbarbuttons in icons mode, we should probably provide a class for this.
Attachment #459368 - Flags: review?(dao) → review-
Attached patch v2 (obsolete) — Splinter Review
Attachment #459368 - Attachment is obsolete: true
Attachment #477313 - Flags: review?(dao)
Comment on attachment 477313 [details] [diff] [review]
v2

>+.toolbarbutton-1 > .toolbarbutton-menu-dropmarker,
> .toolbarbutton-1 > .toolbarbutton-menubutton-dropmarker {
>   list-style-image: url(chrome://browser/skin/toolbarbutton-dropmarker.png);
>+}
>+
>+.toolbarbutton-1  > .toolbarbutton-menu-dropmarker {
>+  -moz-margin-end: 1px;
>+}
>+
>+toolbarbutton[type="menu-button"] > .toolbarbutton-menubutton-dropmarker {
>   width: 14px;
>   padding-top: 2px;
>   -moz-border-start: none !important;
> }

Seems like toolbarbutton[type="menu-button"] should be .toolbarbutton-1.
Attached patch v3Splinter Review
Sometimes I feel blind.
Attachment #477313 - Attachment is obsolete: true
Attachment #477493 - Flags: review?(dao)
Attachment #477313 - Flags: review?(dao)
Comment on attachment 477493 [details] [diff] [review]
v3

>+.toolbarbutton-1  > .toolbarbutton-menu-dropmarker {

redundant space after .toolbarbutton-1
Attachment #477493 - Flags: review?(dao) → review+
Attachment #477493 - Flags: approval2.0?
Attachment #477493 - Flags: approval2.0? → approval2.0+
http://hg.mozilla.org/mozilla-central/rev/207ba2088b65
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b8
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: