427935 - [proto] Land Proto 0.16 changes

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Kevin Gerich]

Attachment: Patch against trunk. See Proto 0.16.1 theme on AMO to test

Round of style tweaks and icon additions

- hud style tweaks
- addons window style tweaks and revised icons
- key + remote icons
- new tab bar drop marker
- revised pref icons
- new notification icons
- remove firefox logo from JS alert

Comment 1

[Johnathan Nightingale [:johnath]]

Comment on attachment 314500 [details] [diff] [review]
Patch against trunk. See Proto 0.16.1 theme on AMO to test

>RCS file: /cvsroot/mozilla/toolkit/themes/pinstripe/global/jar.mn,v
>++  skin/classic/global/icons/information-16.png                       (icons/information-16.png)
>++  skin/classic/global/icons/information-24.png                       (icons/information-24.png)
>++  skin/classic/global/icons/information-32.png                       (icons/information-32.png)
>++  skin/classic/global/icons/information-64.png                       (icons/information-64.png)
>++  skin/classic/global/icons/information-large.png                    (icons/information-large.png)
>++  skin/classic/global/icons/warning-16.png                           (icons/warning-16.png)
>++  skin/classic/global/icons/warning-24.png                           (icons/warning-24.png)
>++  skin/classic/global/icons/warning-32.png                           (icons/warning-32.png)
>++  skin/classic/global/icons/warning-64.png                           (icons/warning-64.png)
>++  skin/classic/global/icons/error-16.png                             (icons/error-16.png)
>++  skin/classic/global/icons/error-24.png                             (icons/error-24.png)
>++  skin/classic/global/icons/error-64.png                             (icons/error-64.png)
>++  skin/classic/global/icons/error-large.png                          (icons/error-large.png)
>++  skin/classic/global/icons/question-16.png                          (icons/question-16.png)
>++  skin/classic/global/icons/question-24.png                          (icons/question-24.png)
>++  skin/classic/global/icons/question-32.png                          (icons/question-32.png)
>++  skin/classic/global/icons/question-64.png                          (icons/question-64.png)
>++  skin/classic/global/icons/question-large.png                       (icons/question-large.png)

Given these file additions, should this segment:

>RCS file: /cvsroot/mozilla/toolkit/themes/pinstripe/global/global.css,v
> /* ::::: alert icons :::::*/
> 
> .message-icon,
> .alert-icon,
> .error-icon,
> .question-icon {
>   width: 64px;
>   height: 64px;
>-  list-style-image: url("chrome://branding/content/icon64.png");
>+  list-style-image: url("chrome://global/skin/icons/warning-64.png");
>   margin: 6px 20px 6px 6px !important;
> }

be broken up to use different icons for the different classes?  Or am I missing something?

Comment 2

[Kevin Gerich]

Attachment: patch with additional dialog icon rules

Comment 3

[Reed Loden [:reed]]

Comment on attachment 314520 [details] [diff] [review]
patch with additional dialog icon rules

>+.question-icon {
>+  list-style-image: url("chrome://global/skin/icons/error-64.png");
>+}

This one should be .error-icon, no?

Comment 4

[Kevin Gerich]

yeah. thanks for pointing that out

Comment 5

[cris]

any thoughts for https://bugzilla.mozilla.org/show_bug.cgi?id=427780 ?

Comment 6

[cris]

also, when you type some thing in urlabar, then press "delete" to remove them, the whole toolbar flashes...

Comment 7

[Stephen Horlander [:shorlander]]

(In reply to comment #6)
> also, when you type some thing in urlabar, then press "delete" to remove them,
> the whole toolbar flashes...
> 

I am not seeing that.

Comment 8

[cris]

(In reply to comment #7)
> (In reply to comment #6)
> > also, when you type some thing in urlabar, then press "delete" to remove them,
> > the whole toolbar flashes...
> > 
> 
> I am not seeing that.
> 
huh, i m wondering if this has anything to do the fact I enabled color management. I will try out later...

Comment 9

[cris]

no, it still flashes. Im on Tiger 10.4.11, its not 100%, but about 50%. e.g., I type in "bbcc" then press "delete" 4 times quickly, there are at least twice the toolbar flashes.

Comment 10

[cris]

i tested it again, it doesn't happen all the time...strange, but still, just try "bbcc" and "delete" 4 times quickly..

Comment 11

[Stephen Horlander [:shorlander]]

(In reply to comment #10)
> i tested it again, it doesn't happen all the time...strange, but still, just
> try "bbcc" and "delete" 4 times quickly..
> 

No, I still don't see it. Could be a Tiger specific thing? What kind of Mac is it(RAM, CPU Speed, etc.)?

Comment 12

[cris]

its a rev A Macbook, tiger 10.4.11, 2G RAM, 60G HDD, 1.8CD CPU, GMA 950

Comment 13

[Ed Lee :Mardak]

I see the "type something until it finds no results then delete until it finds a result" causing the whole chrome to flash white -- urlbar, bookmarks, tabs, status bar -- all the content disappears (or is covered?)

iBook G4 10.4.11

Comment 14

[Stephen Horlander [:shorlander]]

(In reply to comment #13)
> iBook G4 10.4.11
> 

10.4.11 seems to be the common factor here. I will test it out on 10.4.11 as well. I can't imagine it is something specific to the theme though. Does anyone have another theme installed that they could test for the same thing?

Comment 16

[Stephen Horlander [:shorlander]]

Comment on attachment 314520 [details] [diff] [review]
patch with additional dialog icon rules

uir+ on behalf of Beltzner

Comment 17

[Ed Lee :Mardak]

Do you see the flashing problem here? bug 428001

https://build.mozilla.org/tryserver-builds/2008-04-09_23:[e-mail]/

Comment 18

[Asaf Romano (gone)]

Comment on attachment 314520 [details] [diff] [review]
patch with additional dialog icon rules

r=mano

Comment 19

[Kevin Gerich]

Comment on attachment 314520 [details] [diff] [review]
patch with additional dialog icon rules

thanks for the review!

Comment 20

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #17)
> Do you see the flashing problem here? bug 428001
> 
> https://build.mozilla.org/tryserver-builds/2008-04-09_23:[e-mail]/

The problem is fixed with the Mac tryserver build, thanks!

Comment 21

[Mike Connor [:mconnor]]

Comment on attachment 314520 [details] [diff] [review]
patch with additional dialog icon rules

a=mconnor on behalf of 1.9 drivers

Comment 22

[Kevin Gerich]

checked in on trunk

Comment 23

[Mike Shaver (:shaver emeritus)]

Comment on attachment 314520 [details] [diff] [review]
patch with additional dialog icon rules

>RCS file: /cvsroot/mozilla/toolkit/themes/pinstripe/global/global.css,v
>retrieving revision 1.21
>diff -u -p -8 -r1.21 global.css
>--- toolkit/themes/pinstripe/global/global.css	7 Apr 2008 22:56:44 -0000	1.21
>+++ toolkit/themes/pinstripe/global/global.css	9 Apr 2008 06:21:45 -0000
>@@ -133,20 +133,32 @@ window.dialog { 
> /* ::::: alert icons :::::*/
> 
> .message-icon,
> .alert-icon,
> .error-icon,
> .question-icon {
>   width: 64px;
>   height: 64px;
>-  list-style-image: url("chrome://branding/content/icon64.png");
>+  list-style-image: url("chrome://global/skin/icons/information-64.png");
>   margin: 6px 20px 6px 6px !important;
> }
> 
>+.alert-icon {
>+  list-style-image: url("chrome://global/skin/icons/warning-64.png");
>+}
>+
>+.question-icon {
>+  list-style-image: url("chrome://global/skin/icons/error-64.png");
>+}
>+
>+.question-icon {
>+  list-style-image: url("chrome://global/skin/icons/question-64.png");
>+}
>+

Consensus on IRC is that the first .question-icon rule there should be .error-icon -- someone wanna do a follow-up?

Comment 24

[Kevin Gerich]

(In reply to comment #23)
> Consensus on IRC is that the first .question-icon rule there should be
> .error-icon -- someone wanna do a follow-up?
> 

That was a mistake that I corrected before I landed the patch. 

http://lxr.mozilla.org/seamonkey/source/toolkit/themes/pinstripe/global/global.css#149