Closed Bug 560288 Opened 11 years ago Closed 11 years ago

consolidate hud-button style rules

Categories

(Firefox :: Theme, defect)

All
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 3.7a5

People

(Reporter: Gavin, Assigned: Gavin)

Details

Attachments

(1 file, 2 obsolete files)

They're duplicated a couple of times in browser.css unnecessarily.
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Attachment #440014 - Flags: review?(dao)
Comment on attachment 440014 [details] [diff] [review]
patch

>diff --git a/browser/themes/pinstripe/browser/browser.css b/browser/themes/pinstripe/browser/browser.css

>+.hud-button {

>+  -moz-border-right-colors: rgba(53,53,53,1) rgba(53,53,53,1) rgba(162,162,162,1);

> .editBookmarkPanelHeaderButton,
> .editBookmarkPanelBottomButton {

>-  -moz-border-top-colors: rgba(0,0,0,0.35) rgba(26,26,26,0.5) rgba(255,255,255,0.4);
>-  -moz-border-right-colors: rgba(0,0,0,0.35) rgba(26,26,26,0.5) rgba(255,255,255,0.4);

This -moz-border-right-colors was different than all the others, but I think that was a copy/pasto (from -moz-border-top-colors) rather than an intentional difference.

> #editBMPanel_folderMenuList {

>-  color: #ffffff !important;

The !important is lost here, but it doesn't appear to be needed.

> #editBookmarkPanel .expander-up,
> #editBookmarkPanel .expander-down {

>   -moz-border-radius: 5px;

This is left to override the hud-button default (20px).

>-  color: #ffffff !important;

!important also lost here, but these buttons have no text so I think that doesn't matter.
OS: All → Mac OS X
(In reply to comment #2)
> This -moz-border-right-colors was different than all the others, but I think
> that was a copy/pasto (from -moz-border-top-colors) rather than an intentional
> difference.

Also I wasn't able to see a visual difference caused by the discrepancy, so I wonder whether these rules have any effect at all. Or maybe I was just looking at the wrong buttons.
I don't think I like hud-button as a class. It seems to be set only based on whether pinstripe styles a panel black, but other themes might make different choices.

We could use http://mxr.mozilla.org/mozilla-central/source/browser/themes/pinstripe/browser/shared.inc to consolidate the styling without a class.
Comment on attachment 440014 [details] [diff] [review]
patch

>diff --git a/browser/components/places/content/editBookmarkOverlay.js b/browser/components/places/content/editBookmarkOverlay.js

>   toggleTagsSelector: function EIO_toggleTagsSelector() {
>     var tagsSelector = this._element("tagsSelector");
>     var tagsSelectorRow = this._element("tagsSelectorRow");
>     var expander = this._element("tagsSelectorExpander");
>     if (tagsSelectorRow.collapsed) {
>-      expander.className = "expander-up";
>+      expander.className = "expander-up hud-button";

This sets hud-button in the Library too, which certainly seems wrong.
Attachment #440014 - Flags: review?(dao) → review-
Attached patch %define patch (obsolete) — Splinter Review
Like this then? I don't think preprocessor.py supports multi-line #defines.
Attachment #440014 - Attachment is obsolete: true
Looks ok to me. I'd capitalize "Button" and include the semicolon in the %define.
Attachment #440227 - Attachment is obsolete: true
Attachment #440244 - Flags: review?(dao)
Attachment #440244 - Flags: review?(dao) → review+
http://hg.mozilla.org/mozilla-central/rev/6b4fa780bc5c
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a5
You need to log in before you can comment on or make changes to this bug.