Last Comment Bug 427935 - [proto] Land Proto 0.16 changes
: [proto] Land Proto 0.16 changes
Status: VERIFIED FIXED
:
Product: Firefox
Classification: Client Software
Component: Theme (show other bugs)
: Trunk
: All Mac OS X
: -- normal (vote)
: Firefox 3
Assigned To: Kevin Gerich
:
: Dão Gottwald [:dao]
Mentors:
https://addons.mozilla.org/en-US/fire...
Depends on: 430362
Blocks: 386757 424877 427464 427697 427701
  Show dependency treegraph
 
Reported: 2008-04-08 21:45 PDT by Kevin Gerich
Modified: 2008-05-17 08:16 PDT (History)
13 users (show)
mbeltzner: blocking‑firefox3+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch against trunk. See Proto 0.16.1 theme on AMO to test (18.52 KB, patch)
2008-04-08 21:45 PDT, Kevin Gerich
no flags Details | Diff | Splinter Review
patch with additional dialog icon rules (17.54 KB, patch)
2008-04-08 23:31 PDT, Kevin Gerich
asaf: review+
shorlander: ui‑review+
mconnor: approval1.9+
Details | Diff | Splinter Review

Description Kevin Gerich 2008-04-08 21:45:24 PDT
Created attachment 314500 [details] [diff] [review]
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] 2008-04-08 23:06:00 PDT
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 2008-04-08 23:31:47 PDT
Created attachment 314520 [details] [diff] [review]
patch with additional dialog icon rules
Comment 3 Reed Loden [:reed] (use needinfo?) 2008-04-08 23:57:17 PDT
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 2008-04-09 00:03:05 PDT
yeah. thanks for pointing that out

Comment 5 cris 2008-04-09 07:36:49 PDT
any thoughts for https://bugzilla.mozilla.org/show_bug.cgi?id=427780 ?
Comment 6 cris 2008-04-09 08:07:06 PDT
also, when you type some thing in urlabar, then press "delete" to remove them, the whole toolbar flashes...
Comment 7 Stephen Horlander [:shorlander] 2008-04-09 08:36:33 PDT
(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 2008-04-09 08:41:42 PDT
(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 2008-04-09 08:46:47 PDT
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 2008-04-09 08:55:47 PDT
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] 2008-04-09 08:57:30 PDT
(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 2008-04-09 09:19:37 PDT
its a rev A Macbook, tiger 10.4.11, 2G RAM, 60G HDD, 1.8CD CPU, GMA 950
Comment 13 Ed Lee :Mardak 2008-04-09 10:45:10 PDT
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] 2008-04-09 11:01:45 PDT
(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 15 Mike Beltzner [:beltzner, not reading bugmail] 2008-04-09 15:07:45 PDT
*** Bug 428001 has been marked as a duplicate of this bug. ***
Comment 16 Stephen Horlander [:shorlander] 2008-04-09 17:13:55 PDT
Comment on attachment 314520 [details] [diff] [review]
patch with additional dialog icon rules

uir+ on behalf of Beltzner
Comment 18 Mano (::mano, needinfo? for any questions; not reading general bugmail) 2008-04-10 09:51:41 PDT
Comment on attachment 314520 [details] [diff] [review]
patch with additional dialog icon rules

r=mano
Comment 19 Kevin Gerich 2008-04-10 10:21:12 PDT
Comment on attachment 314520 [details] [diff] [review]
patch with additional dialog icon rules

thanks for the review!
Comment 20 Stephen Donner [:stephend] 2008-04-10 10:25:12 PDT
(In reply to comment #17)
> Do you see the flashing problem here? bug 428001
> 
> https://build.mozilla.org/tryserver-builds/2008-04-09_23:10-edward.lee@engineering.uiuc.edu-no.flash/

The problem is fixed with the Mac tryserver build, thanks!
Comment 21 Mike Connor [:mconnor] 2008-04-10 17:32:57 PDT
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 2008-04-11 04:10:50 PDT
checked in on trunk
Comment 23 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-04-13 21:50:28 PDT
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 2008-04-14 04:13:17 PDT
(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


Note You need to log in before you can comment on or make changes to this bug.