Need a chrome://branding/content/icon48.png since mozapps css refers to this.

RESOLVED FIXED in seamonkey2.1a1

Status

SeaMonkey
Themes
--
enhancement
RESOLVED FIXED
8 years ago
4 years ago

People

(Reporter: Philip Chee, Assigned: Philip Chee)

Tracking

Trunk
seamonkey2.1a1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

8 years ago
While working on Bug 465924, I noticed that global/wizard.css refers to chrome://branding/content/icon48.png, as well as toolkit/mozapps/extensions/content/update.xul

Our migration.xul doesn't use branding but all the other apps in the tree do. So we should start doing this as well

So for this bug we ALL need a SeaMonkey 48x48px icon48.png similar to suite/branding/content/icon64.png and located in the same place.

===========================
From Bug 465924 Comment 27:

>> 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

> ...we would need to put it in chrome://branding/content/icon48.png
(Assignee)

Updated

8 years ago
Blocks: 465924
(Assignee)

Updated

8 years ago
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED

Comment 1

8 years ago
Created attachment 404793 [details]
Like this?
(Assignee)

Comment 2

7 years ago
Created attachment 419918 [details] [diff] [review]
Patch v1.0 (WIP) Add icon48.png 

This patch:

1. Adds icon48.png from previous attachment.
2. Adds CSS to modern/global/wizard.css to turn on branding for wizards that specify it. 
3. Specifies branding for migration.xul just like Thunderbird, Calendar, and Firefox.

To test the migration wizard:
1. Bookmarks->Manage Bookmarks.
2. Tools->Import.

To test the toolkit extension update wizard:
1. Open the Error Console and enter the following string into the evalute textbox (all on one line):

(function() {var URI_EXTENSION_UPDATE_DIALOG = "chrome://mozapps/content/extensions/update.xul"; var features = "chrome,centerscreen,dialog,titlebar"; openDialog(URI_EXTENSION_UPDATE_DIALOG, "", features, []); })()
Attachment #419918 - Flags: review?(neil)
(Assignee)

Comment 3

7 years ago
Comment on attachment 404793 [details]
Like this?

I think KaiRo muttered something about needing a drop shadow or some mysterious visual effect.
Attachment #404793 - Flags: ui-review?(kairo)

Comment 4

7 years ago
What's wrong with openDialog("chrome://mozapps/content/extensions/update.xul", "", "chrome,centerscreen,dialog,titlebar", []);

Comment 5

7 years ago
Comment on attachment 419918 [details] [diff] [review]
Patch v1.0 (WIP) Add icon48.png 

>diff --git a/suite/branding/content/icon48.png b/suite/branding/content/icon48.png
>new file mode 100644
[Assuming you get ui-r=KaiRo of course]

[not enough context to show 23px margin]
> .wizard-header-description {
>   -moz-margin-start: 44px;
>   -moz-margin-end: 5px;
(Bah, these don't actually work, since the rules for label/description win.)

>+wizard[branded="true"] .wizard-header {
>+  padding-bottom: 8px;
>+}
>+
>+wizard[branded="true"] .wizard-header-icon {
>+  list-style-image: url("chrome://branding/content/icon48.png");
>+  -moz-margin-end: 6px;
>+}
I don't see any reason not to copy winstripe.
(Assignee)

Comment 6

7 years ago
Created attachment 420072 [details] [diff] [review]
Patch v1.1

> [not enough context to show 23px margin]
>> .wizard-header-description {
>>   -moz-margin-start: 44px;
>>   -moz-margin-end: 5px;
> (Bah, these don't actually work, since the rules for label/description win.)
I played around with various selectors but finally gave up and used !important.

>>+wizard[branded="true"] .wizard-header {
>>+  padding-bottom: 8px;
>>+}
>>+
>>+wizard[branded="true"] .wizard-header-icon {
>>+  list-style-image: url("chrome://branding/content/icon48.png");
>>+  -moz-margin-end: 6px;
>>+}
> I don't see any reason not to copy winstripe.
I was copying Kudens toolkit-modern/global/wizard.css but I've switched to using the winstripe CSS in this patch.
Attachment #419918 - Attachment is obsolete: true
Attachment #420072 - Flags: review?(neil)
Attachment #419918 - Flags: review?(neil)

Comment 7

7 years ago
(In reply to comment #6)
>> [not enough context to show 23px margin]
>>> .wizard-header-description {
>>>   -moz-margin-start: 44px;
>>>   -moz-margin-end: 5px;
>> (Bah, these don't actually work, since the rules for label/description win.)
>I played around with various selectors but finally gave up and used !important.
But you overlooked my hint :-(

Comment 8

7 years ago
Comment on attachment 420072 [details] [diff] [review]
Patch v1.1

>   -moz-margin-start: 23px;
r=me with this fixed. (By the way optipng likes the graphic.)
Attachment #420072 - Flags: review?(neil) → review+

Comment 9

7 years ago
Comment on attachment 404793 [details]
Like this?

I'd like it better if I knew the source of the image, but it looks like it's the dropshadow version we should be using where possible, yes.
Attachment #404793 - Flags: ui-review?(kairo) → ui-review+
(Assignee)

Comment 10

7 years ago
Created attachment 421008 [details] [diff] [review]
Patch v1.2. Nits fixed (r=neil)
[Checkin: Comment 11]

Carrying forward r=neil

> (From update of attachment 420072 [details] [diff] [review])
>>   -moz-margin-start: 23px;
> r=me with this fixed. (By the way optipng likes the graphic.)
Fixed.
Attachment #420072 - Attachment is obsolete: true
Attachment #421008 - Flags: superreview?(neil)
Attachment #421008 - Flags: review+

Updated

7 years ago
Attachment #421008 - Flags: superreview?(neil) → superreview+
(Assignee)

Updated

7 years ago
Attachment #421008 - Attachment description: Patch v1.2. Nits fixed (r=neil) → [for checkin] Patch v1.2. Nits fixed (r=neil)
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
Target Milestone: --- → seamonkey2.1a1
Comment on attachment 421008 [details] [diff] [review]
Patch v1.2. Nits fixed (r=neil)
[Checkin: Comment 11]


http://hg.mozilla.org/comm-central/rev/161b63136587
Attachment #421008 - Attachment description: [for checkin] Patch v1.2. Nits fixed (r=neil) → Patch v1.2. Nits fixed [Checkin: Comment 11]
Attachment #421008 - Flags: approval-seamonkey2.0.3?
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Keywords: checkin-needed
(Assignee)

Updated

7 years ago
Attachment #421008 - Attachment description: Patch v1.2. Nits fixed [Checkin: Comment 11] → Patch v1.2. Nits fixed (r=neil) [Checkin: Comment 11]

Comment 12

7 years ago
Comment on attachment 421008 [details] [diff] [review]
Patch v1.2. Nits fixed (r=neil)
[Checkin: Comment 11]

1) this approval request has no risk assessment or statement why it's needed on branch
2) the approval request was not made by the developer of the patch

For those reasons, clearing the request.
Attachment #421008 - Flags: approval-seamonkey2.0.3?
(In reply to comment #12)

> 1) this approval request has no risk assessment or statement why it's needed on
> branch

I assumed:
*No/Low risk to add a missing image (and related css).
*Needed per http://mxr.mozilla.org/comm-1.9.1/search?string=icon48.png&case=on&find=%2Fwizard.css

> 2) the approval request was not made by the developer of the patch

But I may be mistaken or maybe Philip just doesn't care about this case?

Comment 14

7 years ago
(In reply to comment #13)
> *Needed per
> http://mxr.mozilla.org/comm-1.9.1/search?string=icon48.png&case=on&find=%2Fwizard.css

This only refers to the branded wizard, and comment #0 states that we are not using that one right now.
(Assignee)

Comment 15

7 years ago
1. Toolkit UI uses branded wizards.
2. This patch turns on branding for the one case where the suite has a wizard.
3. Since we don't do separate branded/unbranded builds like Firefox/Minefield we don't need branded wizards, certainly not on 1.9.1 branch!

The only reason I did this patch is for parity with toolkit going forward.
(I understand, sorry.)
(Assignee)

Updated

4 years ago
Duplicate of this bug: 467759
You need to log in before you can comment on or make changes to this bug.