css enhancements for editBMPanel

RESOLVED FIXED in Firefox 4.0b10

Status

()

Firefox
Theme
--
trivial
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: stefanh, Assigned: stefanh)

Tracking

Trunk
Firefox 4.0b10
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 6 obsolete attachments)

(Assignee)

Description

8 years ago
While doing bug 620132, I found some stuff that could be improved a bit. It's basically getting rid of some important rules, descendant selectors. I also spotted a new thing; the checkbox list-style-image could be switched to a background-image (override the one in listbox.css instead of adding a list-style-image).
(Assignee)

Comment 1

8 years ago
Created attachment 502400 [details] [diff] [review]
Fix style rules

I did some more in bug 620132, but I don't think you're interested in that (grouped focus rules etc together and styled a hidden checkbox).
Assignee: nobody → stefanh
Status: NEW → ASSIGNED
Attachment #502400 - Flags: review?(dao)
Comment on attachment 502400 [details] [diff] [review]
Fix style rules

>-#editBookmarkPanel .expander-up,
>-#editBookmarkPanel .expander-down:hover:active {
>+.expander-up,
>+.expander-down:hover:active {
>   list-style-image: url("chrome://browser/skin/hud-style-expander-open.png") !important;
> }
> 
>-#editBookmarkPanel .expander-down,
>-#editBookmarkPanel .expander-up:hover:active {
>+.expander-down,
>+.expander-up:hover:active {
>   list-style-image: url("chrome://browser/skin/hud-style-expander-closed.png") !important;
> }

Extensions may use this class in browser.xul but outside of the panel, so I don't think these changes are correct.
(Assignee)

Comment 3

8 years ago
Created attachment 502474 [details] [diff] [review]
New version

Yeah, you're right. How about this? Might be an overkill, but I couldn't resist. Otherwise I'll just keep the old rules.
Attachment #502400 - Attachment is obsolete: true
Attachment #502474 - Flags: review?(dao)
Attachment #502400 - Flags: review?(dao)
I think we should keep the old selectors.
(Assignee)

Comment 5

8 years ago
Created attachment 502480 [details] [diff] [review]
Keep rules

Btw, note that extensions will get pushbutton styling if they use the expander classes. But I suppose that's better than the arrow-panel look.
Attachment #502474 - Attachment is obsolete: true
Attachment #502480 - Flags: review?(dao)
Attachment #502474 - Flags: review?(dao)
Comment on attachment 502480 [details] [diff] [review]
Keep rules

>-#editBookmarkPanel .expander-up,
>-#editBookmarkPanel .expander-down {
>+#editBMPanel_foldersExpander,
>+#editBMPanel_tagsSelectorExpander {
>   @hudButton@
>   border-radius: 3px;
>   -moz-margin-start: 4px;
>   -moz-margin-end: 2px;
>   padding: 0;
>   -moz-padding-start: 4px;
>   min-width: 10px;
>   min-height: 20px;
> }
> 
>-#editBookmarkPanel .expander-up:-moz-focusring,
>-#editBookmarkPanel .expander-down:-moz-focusring {
>+#editBMPanel_foldersExpander:-moz-focusring,
>+#editBMPanel_tagsSelectorExpander:-moz-focusring {
>   @hudButtonFocused@
> }
> 
>-#editBookmarkPanel .expander-up:hover:active,
>-#editBookmarkPanel .expander-down:hover:active {
>+#editBMPanel_foldersExpander:hover:active,
>+#editBMPanel_tagsSelectorExpander:hover:active {
>   @hudButtonPressed@
> }

Please also avoid these changes.
Attachment #502480 - Flags: review?(dao) → review+
(Assignee)

Comment 7

8 years ago
Created attachment 502494 [details] [diff] [review]
For check-in, comment #6 addressed

I also removed a change that wasn't supposed to be there:
 #customizeToolbarSheetPopup {
   -moz-window-shadow: sheet;
+  -moz-appearance: none;
+  background-color: transparent;
 }
(Assignee)

Comment 8

8 years ago
Comment on attachment 502494 [details] [diff] [review]
For check-in, comment #6 addressed

css-cleanup, small and safe.
Attachment #502494 - Flags: approval2.0?
(Assignee)

Comment 9

8 years ago
Created attachment 502953 [details] [diff] [review]
Final version

Sorry, just found one thing I want to fix... The only new thing here is the "#editBMPanel_tagsSelector .listcell-check" background-color put together with the new "background" and the listbox.css adjustment (I suddenly remember that I forgot to remove that in 2009 and sooner or later someone will remove the no-repeat from there and regress the panel).
Attachment #502480 - Attachment is obsolete: true
Attachment #502494 - Attachment is obsolete: true
Attachment #502953 - Flags: review?(dao)
Attachment #502494 - Flags: approval2.0?
Comment on attachment 502953 [details] [diff] [review]
Final version

> #editBMPanel_tagsSelector .listcell-check {
>-  -moz-appearance: none !important;
>-  background-color: transparent;
>+  -moz-appearance: none;
>   border: 0;
>-  list-style-image: url("chrome://browser/skin/hud-style-check-box-empty.png");
>+  background: transparent url("chrome://browser/skin/hud-style-check-box-empty.png") no-repeat 50% 50%;

remove 'transparent'

> .listcell-check[checked="true"] {
>-  background-image: url("chrome://global/skin/checkbox/cbox-check.gif");
>+  background-image: url("chrome://global/skin/checkbox/cbox-check.gif") no-repeat 50% 50%;
> }

background-image doesn't support these values.
Attachment #502953 - Flags: review?(dao) → review-
(Assignee)

Comment 11

8 years ago
Agh. OK, so this would also mean that I can remove the -moz-field background-color of the -listcell-check in listbox-css since using 'background' instead of background-image will override it anyway. Anyway, will look at it tomorrow.
(Assignee)

Comment 12

8 years ago
Created attachment 503197 [details] [diff] [review]
A new final version

Sorry for the confusion, I should go to bed earlier :-(

OK, so I think it's better to keep the background-color in listbox.css despite my previous comment. Another option would be to leave out the listbox.css changes while keeping all the other changes.
Attachment #502953 - Attachment is obsolete: true
Attachment #503197 - Flags: review?(dao)
Please leave out the listbox.css.
(Assignee)

Comment 14

8 years ago
Created attachment 503216 [details] [diff] [review]
Really, really final

(In reply to comment #13)
> Please leave out the listbox.css.
Done.
Attachment #503197 - Attachment is obsolete: true
Attachment #503216 - Flags: review?(dao)
Attachment #503197 - Flags: review?(dao)
Comment on attachment 503216 [details] [diff] [review]
Really, really final

Thanks!
Attachment #503216 - Flags: review?(dao) → review+
(Assignee)

Comment 16

8 years ago
Comment on attachment 503216 [details] [diff] [review]
Really, really final

css-cleanup, small and safe.
Attachment #503216 - Flags: approval2.0?
Comment on attachment 503216 [details] [diff] [review]
Really, really final

a=beltzner
Attachment #503216 - Flags: approval2.0? → approval2.0+
(Assignee)

Comment 18

8 years ago
http://hg.mozilla.org/mozilla-central/rev/8fdde63f610e
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b10
You need to log in before you can comment on or make changes to this bug.