+++ This bug was initially created as a clone of Bug #456757 +++ Splitting off work in global: Bug 456757 Comment 12 > At least, these changes are also necessary with /mozapps/ . > button.css, textbox.css, wizard.css, toolbarbutton.css, > progressmeter.css, richlistbox.css, notification.css > and > /global/throbber/ > /global/icons/(added new icons) Bug 456757 Comment 10 Created an attachment (id=349042) global/config.css I liked some of your config.css changes: * use background-color not just background * #warningBox needs a colour * ThreeDShadow should be a Modern colour * #exclam should have a width and height but not others: * comments restored * bracing and CSS properties reverted to file style * useless and incorrect #configTree margin removed * #filterRow margin removed, I don't think we need it What do you think?

I didn't like these netError.css changes: * I reverted CSS properties to file style * I removed the #securityOverrideContent border and restored border-radius I also forgot to credit you for config.css last time. Oops.

(In reply to comment #1) > I also forgot to credit you for config.css last time. Oops. I do not know the requirement of writing the name. I hope if necessary like this. Akihiro Misaki (spitfire.kuden@gmail.com)

I cannot make the patch now... I am a tested stage still. The completed part is up-loaded with ZIP for the time being.

Comment on attachment 349512 [details] [diff] [review] global/netError.css [Checkin: Comment 5] Pushed changeset to comm-central. D'oh, I meant to fix the config.css credit in the same commit :-(

Comment on attachment 349512 [details] [diff] [review] global/netError.css [Checkin: Comment 5] Pushed changeset 47b0035e4982 to comm-central. D'oh, I meant to fix the config.css credit in the same commit :-(

Sigh. I am having no luck today :-(

This contains Kuden's changes to about.css. It gives a nice border around about:* similar to the NetError pages.

+ background: -moz-Field; Don't use system colours in modern. +h1, h2, h3, h4 { + color: #5d616e; ... +a:link, +a:visited, +a:active { + color: rgb(49, 48, 99); Sometimes you use hex values and sometimes rgb. I know Standard8 introduced rgb() but try to stick to one style which in Modern appears to be #hex values. #version { - color: #000; - margin-top: -40px; - margin-bottom: 9px; - -moz-margin-start: 10px; - -moz-margin-end: 0px; - font-size: 130%; + font-weight: bold; + color: #8290a5; + margin: -24px 0 9px 17px; You regressed RTL support introduced in Bug 474807. ul > li { - margin-top: .5em; + margin-top: 0.5em; Why this change? + th, + td { + padding: 3px; + } move/consolidate this style into the preceeding styles for th and td.

Comment on attachment 372356 [details] [diff] [review] Kuden's Changes to Global/about.css Not bad, but I don't quite like it. Hopefully these issues should be easy to fix. > html { >- background: #FFF; I think that this was in fact the correct outer background colour. (See also below.) >- color: #22262F; >+ color: #22262f; Nit: capital letters in colour codes >+ background-color: #ffffff; > position: relative; > min-width: 330px; > max-width: 50em; >- margin: 2em auto; >- border: 1px solid rgb(73, 79, 93); >+ margin: 4em auto; >+ border: 1px solid #7a8490; > -moz-border-radius: 10px; >- padding: 2em; >+ padding: 3em; > -moz-padding-start: 30px; >- background: #C7D0D9; >+ background: -moz-Field; Two backgrounds :-( In fact, I think that the colour removed here was intended to be the inner colour. Compare with, say, about:config (from which you should also use the border colour). > #aboutLogoContainer { >+ border: 1px solid #c7d0d9; >+ -moz-border-radius: 3px; The radius doesn't belong here. I don't think this is the correct border colour either. >+h1:first-child { >+ text-align: center; >+} >+ >+h1 { >+ font-size: x-large; >+} >+ >+h1, h2, h3, h4 { >+ color: #5d616e; >+} >+ >+a:link, >+a:visited, >+a:active { >+ color: rgb(49, 48, 99); >+} I don't think any of these belong here. >+ color: #8290a5; I think this is meant to be #8C99AB. >+ margin: -24px 0 9px 17px; RTL bug in toolkit? >- margin-top: .5em; >+ margin-top: 0.5em; Don't bother changing this. >+/* about:buildconfig */ Is it perhaps possible to @include plugins.css to achieve the same effect? Note: I didn't look too carefully at the styles below but if you can't get @include to work then they should at least match up fairly closely to plugins.css >+@-moz-document url(about:buildconfig) { Don't bother with this, there are no tables in about:config

This should simplify the patch and fix all of the issues.

Comment on attachment 372804 [details] [diff] [review] Changes global/about.css v.2 Very nice. Would you mind a few final tweaks? > body { >- margin: 0; >- padding: 0 1em; >+ margin: 4em auto; >+ padding: 3em; > color: #22262F; > font: message-box; >-} >- >-#aboutPageContainer { >+ text-align: left; > position: relative; > min-width: 330px; > max-width: 50em; >- margin: 2em auto; Please move the margin down here. >- border: 1px solid rgb(73, 79, 93); >+ border: 1px solid #494F5D; > -moz-border-radius: 10px; >- padding: 2em; And the padding down here please. > -moz-padding-start: 30px; > background: #C7D0D9; And the text-align here please. > #version { > color: #000; >- margin-top: -40px; >+ font-weight: bold; And please could you put the font-weight before the colour.

This should include all of the moves.

Comment on attachment 372922 [details] [diff] [review] Changes global/about.css v.2.1 [Checkin: Comment 13] http://hg.mozilla.org/comm-central/rev/f2197d762a30

>- <richlistbox id="feedListbox" class="inset" flex="1"/> >+ <richlistbox id="feedListbox" flex="1"/> This never worked anyway because we didn't have |-moz-appearance: none;| on the richlistboxes (XP Luna again?). > richlistbox { >+ -moz-appearance: none !important; In my limited testing with the pageinfo and addons manager windows I found that I didn't need !important however Kuden has vastly more experience theming and coping with various edge cases so I'm leaving this in.

(In reply to comment #14) >Created an attachment (id=385748) >>- <richlistbox id="feedListbox" class="inset" flex="1"/> >>+ <richlistbox id="feedListbox" flex="1"/> >This never worked anyway because we didn't have |-moz-appearance: none;| on the >richlistboxes (XP Luna again?). Not in Classic, no, but it did at least stop them looking silly on Modern ;-) >> richlistbox { >>+ -moz-appearance: none !important; >In my limited testing with the pageinfo and addons manager windows I found that >I didn't need !important however Kuden has vastly more experience theming and >coping with various edge cases so I'm leaving this in. "-moz-appearance" should not appear in Modern. Excuse the seven oversights ;-)

Comment on attachment 385748 [details] [diff] [review] global/richlistbox.css v1.0 >+richlistbox:focus { >+ -moz-border-top-colors: #98A5B2 #000000; >+ -moz-border-right-colors: #98A5B2 #000000; >+ -moz-border-bottom-colors: #98A5B2 #000000; >+ -moz-border-left-colors: #98A5B2 #000000; >+} Sorry, but I don't like this. Please remove it and the -moz-appearance too. > richlistbox[disabled="true"] { >- color: #999999; >+ color: #8C99AB; Nice catch!

carrying forward r=Neil >>> richlistbox { >>>+ -moz-appearance: none !important; >>In my limited testing with the pageinfo and addons manager windows I found that >>I didn't need !important however Kuden has vastly more experience theming and >>coping with various edge cases so I'm leaving this in. > "-moz-appearance" should not appear in Modern. Excuse the seven oversights ;-) We stil need -moz-appearance due to xul.css. Over irc neil says: > in that case can you add a comment /* need to override xul.css */ Done. >>+richlistbox:focus { > Sorry, but I don't like this. Fixed.

Comment on attachment 386664 [details] [diff] [review] [checked in] global/richlistbox.css v1.1 r+sr=Neil *actually* carrying forward r+

(In reply to comment #17) > We stil need -moz-appearance due to xul.css. For some reason I have the patch to bug 483537 in my tree ;-)

Comment on attachment 386664 [details] [diff] [review] [checked in] global/richlistbox.css v1.1 r+sr=Neil Requesting a2.0b1 since we are now in slushy code freeze.

Comment on attachment 386664 [details] [diff] [review] [checked in] global/richlistbox.css v1.1 r+sr=Neil We're not in slushy code freeze until 23:59 US/Pacific tonight...

Comment on attachment 386664 [details] [diff] [review] [checked in] global/richlistbox.css v1.1 r+sr=Neil Checked in: http://hg.mozilla.org/comm-central/rev/a309b1faced3

.textbox-search-icon { - list-style-image: url("chrome://global/skin/icons/search.gif"); + list-style-image: url(chrome://global/skin/icons/Search-glass.png); The current search icon seems to have been borrowed from the navigator urlbar search button. Now that we have LTR and RTL images from Kuden we can flip the search icon so that it's pointing in the same direction as classic/*stripe. .textbox-search-clear { - list-style-image: url("chrome://global/skin/icons/closebox.gif"); + list-style-image: url(chrome://global/skin/icons/Search-close.png); Get a proper close image. The current one borrowed from (I think) the classic tab close button is too weedy by half. +.textbox-input-box menupopup { + cursor: default; I didn't include this bit from Kuden because I don't know why this is needed. +wizard[branded="true"] .wizard-header-icon { +/* XXXRatty: we don't have a 48px image for branding. + list-style-image: url("chrome://branding/content/icon48.png"); +*/ + list-style-image: url("chrome://communicator/skin/brand/throbber-single.png"); + -moz-margin-end: 6px; +} Perhaps KaiRo can whip up a suitable 48px icon? +.wizard-buttons-separator { + margin-bottom: 0 !important; +} I couldn't find a rule that was more specific than something in global.css that would allow me to avoid !important. +/* ..... OS X ..... */ +/* XXXRatty: Don't have a Mac here. +.wizard-buttons-btm { + padding: 5px; +} I don't know about this. Perhaps I'll just leave it out?

notification.css is a bit more tricky. We would need the 16px icons from port bug 512254 first.

(In reply to comment #24) > +wizard[branded="true"] .wizard-header-icon { > +/* XXXRatty: we don't have a 48px image for branding. > + list-style-image: url("chrome://branding/content/icon48.png"); > +*/ > + list-style-image: > url("chrome://communicator/skin/brand/throbber-single.png"); > + -moz-margin-end: 6px; > +} > > Perhaps KaiRo can whip up a suitable 48px icon? Where does this show up? Does the default theme not have it as well?

> Where does this show up? Does the default theme not have it as well? http://mxr.mozilla.org/comm-central/search?string=branded=%22true%22 http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/mozapps/extensions/content/update.xul#55 http://mxr.mozilla.org/comm-central/source/mozilla/browser/components/migration/content/migration.xul#50 http://mxr.mozilla.org/comm-central/source/mail/components/migration/content/migration.xul#49 http://mxr.mozilla.org/comm-central/source/calendar/base/content/migration.xul#54 The suite version appears to not use branded="true". Perhaps we should start doing so? http://mxr.mozilla.org/comm-central/source/suite/profile/migration.xul#43 For Classic we would need to put it in chrome://branding/content/icon48.png We would probably need a manifest line like (stolen from minefield): content branding jar:classic.jar!/content/communicator/brand/ xpcnativewrappers=yes

Grr, I'm not in a mood to read a few files of code to answer if I need tio do some work for this at all or not. If I read correctly between the lines of your comment, the answers to my questions seems to be "It shows up in a branded version of the migration dialog which we are not using right now" (or is there something with the update dialog as well?) and "the default also doesn't have it", which for me seems to mean we should leave this completely out here and possibly file a bug to do/use a branded version, in which we'd do it for both themes.

I scrounged around the net and found a 48px version of the SeaMonkey logo.

(In reply to comment #24) > Get a proper close image. The current one borrowed from (I think) the classic > tab close button is too weedy by half. Sidebar close button, actually, but I don't like Kuden's one either, sorry.

(In reply to comment #30) >> Get a proper close image. The current one borrowed from (I think) the classic >> tab close button is too weedy by half. > Sidebar close button, actually, but I don't like Kuden's one either, sorry. Well what sort of close button do you like then?

Comment on attachment 396699 [details] [diff] [review] textbox.css and wizard.css >+textbox[empty="true"] { [Bah, this got changed to isempty in 1.9.2] >+/* ::::: textboxes inside toolbarpaletteitems ::::: */ >+ >+toolbarpaletteitem > toolbaritem > textbox > .textbox-input-box > html|*.textbox-input { >+ visibility: hidden; Where do we use this? >+wizard[description=""] .wizard-header-description { >+ display: none; >+} Not sure what this achieves, but you'd be better off with a rule on .wizard-header-description[value=""] >+ margin-bottom: 0 !important; Nit: 0px etc. >+/* ..... OS X ..... */ >+/* XXXRatty: Don't have a Mac here. >+.wizard-buttons-btm { This is the Mac "name" for .wizard-buttons-box-2; the Mac version of .wizard-buttons-box-1 doesn't have a name, while rather annoyingly there is no Mac version of .wizard-buttons-separator so we just lose that :-( I don't know why one uses margin and the other uses padding; we should be consistent with padding. If you were really sneaky you could hide the .wizard-buttons-separator and add a 2px top border to the .wizard-buttons-btm/box-2 ;-) >+ skin/modern/global/icons/Search-close.png (global/icons/Search-close.png) >+ skin/modern/global/icons/Search-glass.png (global/icons/Search-glass.png) >+ skin/modern/global/icons/Search-glass-rtl.png (global/icons/Search-glass-rtl.png) Lowercase s please?

(In reply to comment #32) > If you were really sneaky you could hide the .wizard-buttons-separator and add > a 2px top border to the .wizard-buttons-btm/box-2 ;-) Although I'm not sure what stefanh would have to say about this...

Comment on attachment 397861 [details] icon48.png Candidate. I thought we agreed to add it in a different bug for both themes at once? And while this is a logo image, it may not be the one we want int hat window, as we might want a drop shadow there, but I'd need to see it in the build itself to judge.

> I thought we agreed to add it in a different bug for both themes at once? This goes into suite/branding/content/ in parallel with the existing icon64.png so it should be picked up by both themes.

>> Get a proper close image. The current one borrowed from (I think) the classic >> tab close button is too weedy by half. > Sidebar close button, actually, but I don't like Kuden's one either, sorry. Reverting to closebox.gif. >>+textbox[empty="true"] { > [Bah, this got changed to isempty in 1.9.2] Now using both: +textbox[empty="true"], +textbox[isempty="true"] { + color: #999999; +} >>+/* ::::: textboxes inside toolbarpaletteitems ::::: */ >>+ >>+toolbarpaletteitem > toolbaritem > textbox > .textbox-input-box > html|*.textbox-input { >>+ visibility: hidden; > Where do we use this? Not currently used in the Suite, but extensions with toolbaritems might use it. Also a hypothetical google search box. If the quicksearch widget in messenger is converted to a customizable toolbaritem it would be used there too. >>+wizard[description=""] .wizard-header-description { >>+ display: none; >>+} > Not sure what this achieves, but you'd be better off with a rule on > .wizard-header-description[value=""] Fixed. >>+ margin-bottom: 0 !important; > Nit: 0px etc. Fixed. >>+/* ..... OS X ..... */ >>+/* XXXRatty: Don't have a Mac here. >>+.wizard-buttons-btm { > This is the Mac "name" for .wizard-buttons-box-2; the Mac version of > .wizard-buttons-box-1 doesn't have a name, while rather annoyingly there is no > Mac version of .wizard-buttons-separator so we just lose that :-( > I don't know why one uses margin and the other uses padding; we should be > consistent with padding. Switched both to padding: 5px; I hope stefanh likes this. >> If you were really sneaky you could hide the .wizard-buttons-separator and add >> a 2px top border to the .wizard-buttons-btm/box-2 ;-) > Although I'm not sure what stefanh would have to say about this... I'm going to have to punt this to stefanh to do a followup bug since I don't have a Mac to test. >>+ skin/modern/global/icons/Search-close.png (global/icons/Search-close.png) >>+ skin/modern/global/icons/Search-glass.png (global/icons/Search-glass.png) >>+ skin/modern/global/icons/Search-glass-rtl.png (global/icons/Search-glass-rtl.png) > Lowercase s please? Fixed.

Comment on attachment 398173 [details] [diff] [review] textbox.css and wizard.css vTw1.1 > .textbox-search-icon { >- list-style-image: url("chrome://global/skin/icons/search.gif"); >+ list-style-image: url(chrome://global/skin/icons/search-glass.png); >+ -moz-image-region: rect(0, 48px, 16px, 32px); >+ -moz-padding-end: 3px; >+} >+ >+.textbox-search-icon[chromedir="rtl"] { >+ list-style-image: url(chrome://global/skin/icons/search-glass-rtl.png); >+} >+ >+.textbox-search-icon[searchbutton] { >+ -moz-image-region: rect(0px, 16px, 16px, 0px); While I like the idea of a "greyed" search icon for quick search, I'd like to stick with a 15px high icon if possible. I also looked more closely at the close icon, and I think it must have lost its alpha transparency somewhere. So I guess we won't be changing textbox.css just yet ;-) >+wizard[branded="true"] .wizard-header { >+ padding-bottom: 8px; >+} >+ >+wizard[branded="true"] .wizard-header-icon { >+/* XXXRatty: we don't have a 48px image for branding. >+ list-style-image: url("chrome://branding/content/icon48.png"); >+*/ >+ list-style-image: url("chrome://communicator/skin/brand/throbber-single.png"); >+ -moz-margin-end: 6px; >+} Not sure we should do this until we get a suitable image. (It's no rush, after all who uses a branded wizard?) >+ margin-bottom: 0px. !important; Nit: no . >+.wizard-buttons-box-2 { Nit: combine .wizard-buttons-btm with this (no need for OS X comment)

[textbox.css] > While I like the idea of a "greyed" search icon for quick search, I'd like to > stick with a 15px high icon if possible. I also looked more closely at the > close icon, and I think it must have lost its alpha transparency somewhere. So > I guess we won't be changing textbox.css just yet ;-) Removed the searchbox changes (and the search-*png images) [wizard.css] > Not sure we should do this until we get a suitable image. Removed the branding bits. > (It's no rush, after all who uses a branded wizard?) Is this a trick question? Firefox and Thunderbird in their migrate.xul > >+ margin-bottom: 0px. !important; > Nit: no . Oops. Fixed. > >+.wizard-buttons-box-2 { > Nit: combine .wizard-buttons-btm with this (no need for OS X comment) Fixed.

Comment on attachment 398654 [details] [diff] [review] [Checked in] textbox.css and wizard.css vTw1.2 http://hg.mozilla.org/comm-central/rev/173d24f3935c

Philip, I'm not sure if you're finished here, so I leave the resolve/keep open decision to you.

This bug is getting long in the tooth. I've split off Bug 515622 for notification.css. Once that is fixed I'm closing this bug as well.

Closing bug. Remaining polish bugs can be fixed/closed later.