Closed
Bug 483537
Opened 16 years ago
Closed 15 years ago
-moz-appearance belongs in themes, not in xul.css
Categories
(Toolkit :: Themes, defect)
Toolkit
Themes
Tracking
()
RESOLVED
FIXED
People
(Reporter: neil, Assigned: neil)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
Firefox 3.0 apparently has six uses, so you're 50% better already :-)
Flags: blocking1.9.1?
Assignee | ||
Comment 1•16 years ago
|
||
.autocomplete-richlistbox
* Moved into autocomplete.css where it belongs
radio[pane]
* Seems to be catered for by all toolkit themes already
richlistbox
* Moved into richlistbox.css where it belongs
Note: pinstripe has no changes; it seems not to use native themed listboxes.
Comment 2•16 years ago
|
||
Don't think this blocks, so minusing, but Neil, if you have rationale, please renominate.
Flags: blocking1.9.1? → blocking1.9.1-
Assignee | ||
Updated•15 years ago
|
Attachment #367525 -
Flags: review?(gavin.sharp) → review?(dao)
Comment 3•15 years ago
|
||
Comment on attachment 367525 [details] [diff] [review]
Proposed patch
> radio[pane] {
> -moz-binding: url("chrome://global/content/bindings/preferences.xml#panebutton") !important;
> -moz-box-orient: vertical;
>- -moz-appearance: none;
> -moz-box-align: center;
> }
Does pinstripe have a replacement for this?
Assignee | ||
Comment 4•15 years ago
|
||
(In reply to comment #3)
> Does pinstripe have a replacement for this?
I'm not sure what pinstripe is on; its listbox.css doesn't use -moz-appearance: listbox, but it does use -moz-appearance: listbox; four times in other files; these may have been blindly copied from winstripe.
Comment 5•15 years ago
|
||
Note that the cited snippet was about radio[pane]...
Assignee | ||
Comment 6•15 years ago
|
||
(In reply to comment #5)
> Note that the cited snippet was about radio[pane]...
Sorry.
radio[pane] {
-moz-appearance: none;
}
is duplicated in the following files:
toolkit/content/xul.css
toolkit/themes/gnomestripe/global/preferences.css
toolkit/themes/winstripe/global/preferences.css
browser/themes/pinstripe/browser/preferences/preferences.css
Comment 7•15 years ago
|
||
That looks like it needs to be moved from browser to toolkit for pinstripe.
Assignee | ||
Comment 8•15 years ago
|
||
Sure, but that's a separate bug which I'm not immediately interested in ;-)
Comment 9•15 years ago
|
||
It seems to become necessary as soon as you remove -moz-appearance:none from xul.css. That's not just a matter of organizing that stuff nicely -- I suspect that the patch actually breaks stuff as it stands.
Assignee | ||
Comment 10•15 years ago
|
||
(In reply to comment #9)
> It seems to become necessary as soon as you remove -moz-appearance:none from
> xul.css.
Oh, so for some reason the -moz-appearance: none; in pinstripe's preferences.css isn't taking effect? Since I don't have a Mac, perhaps you can look at the applied style rules in DOM Inspector to see what's conflicting.
Comment 11•15 years ago
|
||
I'm not sure that all documents with radio[pane] elements include browser/preferences/preferences.css, even if we ignore everything besides browser/ (which I think we shouldn't do).
What I said wasn't based on testing, and I also don't have a Mac.
Assignee | ||
Comment 12•15 years ago
|
||
(In reply to comment #11)
> I'm not sure that all documents with radio[pane] elements include
> browser/preferences/preferences.css
Then they're not going to get any other radio[pane] styles...
Comment 13•15 years ago
|
||
Ok, since the pane attribute is preferences-specific, that won't be an issue for Firefox. That leaves us with potential breakage for other preferences.xml users.
Comment 14•15 years ago
|
||
Comment on attachment 367525 [details] [diff] [review]
Proposed patch
>diff -r 388c17a510e8 toolkit/content/xul.css
> richlistbox {
> -moz-binding: url('chrome://global/content/bindings/richlistbox.xml#richlistbox');
> -moz-user-focus: normal;
>- -moz-appearance: listbox;
> -moz-box-orient: vertical;
> }
>diff -r 388c17a510e8 toolkit/themes/winstripe/global/richlistbox.css
> richlistbox {
>+ -moz-appearance: listbox;
> margin: 2px 4px;
> background-color: -moz-Field;
> color: -moz-FieldText;
Since this was part of xul.css, it seems reasonable to me to add this to pinstripe's richlistbox.css as well (and then autocomplete.css accordingly). I'm not sure that it's necessary, but it would minimize the risk of breaking something.
Assignee | ||
Comment 15•15 years ago
|
||
(In reply to comment #14)
> (From update of attachment 367525 [details] [diff] [review])
> > richlistbox {
> >+ -moz-appearance: listbox;
> > margin: 2px 4px;
> > background-color: -moz-Field;
> > color: -moz-FieldText;
> Since this was part of xul.css, it seems reasonable to me to add this to
> pinstripe's richlistbox.css as well (and then autocomplete.css accordingly).
What about pinstripe's listbox.css? Should I swap border for -moz-appearance?
Comment 16•15 years ago
|
||
Seems like that would make sense.
Assignee | ||
Comment 17•15 years ago
|
||
It seems trees should use listbox appearance too.
I ordered the appearance lines to match the equivalent win/gnomestripe file(s).
Attachment #367525 -
Attachment is obsolete: true
Attachment #389380 -
Flags: review?(dao)
Attachment #367525 -
Flags: review?(dao)
Comment 18•15 years ago
|
||
Comment on attachment 389380 [details] [diff] [review]
Updated for review comments
Looks good to me. Markus, would you mind testing this on Mac?
Attachment #389380 -
Flags: review?(mstange)
Attachment #389380 -
Flags: review?(dao)
Attachment #389380 -
Flags: review+
Comment 19•15 years ago
|
||
I've found three problems with this patch on Mac:
- class="plain" on trees no longer turns off the border (bug 468860)
- The border is too dark. I should fix -moz-appearance: listbox to use the
right colors.
- The richlistbox in the download manager no longer has a white background. I
think we should keep the border and background styles for graceful fallback.
Assignee | ||
Comment 20•15 years ago
|
||
(In reply to comment #19)
> I've found three problems with this patch on Mac:
> - class="plain" on trees no longer turns off the border (bug 468860)
That's not Mac-specific; all our native themes have the same bug.
> - The border is too dark. I should fix -moz-appearance: listbox to use the
> right colors.
Beyond the scope of this bug? Or limit -moz-appearance: listbox to richlistbox?
> - The richlistbox in the download manager no longer has a white background. I
> think we should keep the border and background styles for graceful fallback.
Actually the previous background was -moz-Field and there was no border.
Assignee | ||
Comment 21•15 years ago
|
||
(In reply to comment #19)
> - class="plain" on trees no longer turns off the border (bug 468860)
...
> - The richlistbox in the download manager no longer has a white background.
Aha, downloads.css reimplements class="plain" ;-)
Assignee | ||
Comment 22•15 years ago
|
||
OK, this version should be good to go for now.
Attachment #389380 -
Attachment is obsolete: true
Attachment #389675 -
Flags: review?(mstange)
Attachment #389380 -
Flags: review?(mstange)
Updated•15 years ago
|
Attachment #389675 -
Flags: review?(mstange) → review+
Assignee | ||
Comment 23•15 years ago
|
||
Pushed changeset 62d18b867afb to mozilla-central.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•