[proto] Land Proto 0.16 changes

VERIFIED FIXED in Firefox 3

Status

()

Firefox
Theme
VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: Kevin Gerich, Assigned: Kevin Gerich)

Tracking

Trunk
Firefox 3
All
Mac OS X
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

10 years ago
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
Flags: blocking-firefox3?
Attachment #314500 - Flags: ui-review?(beltzner)
Attachment #314500 - Flags: review?(mano)
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?
(Assignee)

Comment 2

10 years ago
Created attachment 314520 [details] [diff] [review]
patch with additional dialog icon rules
Attachment #314500 - Attachment is obsolete: true
Attachment #314520 - Flags: ui-review?(beltzner)
Attachment #314520 - Flags: review?(mano)
Attachment #314500 - Flags: ui-review?(beltzner)
Attachment #314500 - Flags: review?(mano)
(Assignee)

Updated

10 years ago
Blocks: 427464
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?
(Assignee)

Comment 4

10 years ago
yeah. thanks for pointing that out

Hardware: PC → All

Comment 5

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

Comment 6

10 years ago
also, when you type some thing in urlabar, then press "delete" to remove them, the whole toolbar flashes...
(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

10 years ago
(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

10 years ago
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

10 years ago
i tested it again, it doesn't happen all the time...strange, but still, just try "bbcc" and "delete" 4 times quickly..
(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

10 years ago
its a rev A Macbook, tiger 10.4.11, 2G RAM, 60G HDD, 1.8CD CPU, GMA 950
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
(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?
Flags: blocking-firefox3? → blocking-firefox3+
Duplicate of this bug: 428001
Comment on attachment 314520 [details] [diff] [review]
patch with additional dialog icon rules

uir+ on behalf of Beltzner
Attachment #314520 - Flags: ui-review?(beltzner) → ui-review+
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/

Updated

10 years ago
Whiteboard: [has patch][needs review mano]
Comment on attachment 314520 [details] [diff] [review]
patch with additional dialog icon rules

r=mano
Attachment #314520 - Flags: review?(mano) → review+
(Assignee)

Comment 19

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

thanks for the review!
Attachment #314520 - Flags: approval1.9?
(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!
Whiteboard: [has patch][needs review mano] → [has patch][needs approval]
Comment on attachment 314520 [details] [diff] [review]
patch with additional dialog icon rules

a=mconnor on behalf of 1.9 drivers
Attachment #314520 - Flags: approval1.9? → approval1.9+
(Assignee)

Comment 22

10 years ago
checked in on trunk
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
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?
(Assignee)

Comment 24

10 years ago
(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

Updated

10 years ago
Depends on: 430362
Status: RESOLVED → VERIFIED
Whiteboard: [has patch][needs approval]
Target Milestone: --- → Firefox 3
You need to log in before you can comment on or make changes to this bug.