Closed Bug 1279206 Opened 4 years ago Closed 4 years ago

Password icons

Categories

(SeaMonkey :: Themes, defect)

defect
Not set

Tracking

(seamonkey2.46 unaffected, seamonkey2.47 fixed)

RESOLVED FIXED
Tracking Status
seamonkey2.46 --- unaffected
seamonkey2.47 --- fixed

People

(Reporter: Paolo, Assigned: frg)

References

Details

Attachments

(1 file, 2 obsolete files)

As part of the popup notifications redesign, we are converting the password icons from PNG to SVG. The new icons are specified in the front-end theme, and the old "key" icons are being removed from Toolkit.

These references will no longer be valid:

http://mxr.mozilla.org/comm-central/search?string=chrome%3A//mozapps/skin/passwordmgr/

To solve this, the old PNG icons or the new SVG ones could be moved to the SeaMonkey theme.
Attached patch 1279206-PasswordIcons.patch (obsolete) — Splinter Review
Paolo, thanks for letting us know about the change.

Philip, I took the Windows and OSX png icons but not the linux ones. key.png and key-16@2x.png seem to be unused in the themes. Shopuld we keep them?
Assignee: nobody → frgrahl
Status: NEW → ASSIGNED
Attachment #8763344 - Flags: review?(philip.chee)
OS: Unspecified → All
Hardware: Unspecified → All
Version: unspecified → Trunk
Comment on attachment 8763344 [details] [diff] [review]
1279206-PasswordIcons.patch

> Philip, I took the Windows and OSX png icons but not the linux ones.
Yes. I think we use the windows images for linux.

> key.png
> and key-16@2x.png seem to be unused in the themes. Shopuld we keep them?
We should keep key-16@2x.png assuming we will support HiDPI screens in the future. key.png is small enough that it doesn't make a difference and we don't package it anyway.

> diff --git a/suite/themes/classic/communicator/passwordmgr/key-16.png
Please put the PNGs in /suite/themes/classic/communicator/icons/ instead.
(and /suite/themes/classic/mac/communicator/icons/)

> diff --git a/suite/themes/modern/communicator/passwordmgr/key-16.png
All the changes in modern are unnecessary. Please revert.
Attachment #8763344 - Flags: review?(philip.chee) → review-
Attached patch 1279206-PasswordIcons-V2.patch (obsolete) — Splinter Review
V2 but with modern still in. Why isn't modern not needing the changes?
Attachment #8763344 - Attachment is obsolete: true
Attachment #8763984 - Flags: review?(philip.chee)
> V2 but with modern still in. Why isn't modern not needing the changes?
Modern already has key-16.png, key-64.png, and key.png. I don't see why you need additional copies of these three files.
You are right. Sorry I missed it. I overlooked that the png files were not fetched from the toolkit directory. 

Modern and classic are now a bit inconsistent. Modern is mozapps/passwordmgr and classic icons. Should they be moved?
Attachment #8763984 - Attachment is obsolete: true
Attachment #8763984 - Flags: review?(philip.chee)
Attachment #8764366 - Flags: review?(philip.chee)
> Modern and classic are now a bit inconsistent. Modern is mozapps/passwordmgr
> and classic icons. Should they be moved?

No. Modern is (or should be built) as a third party theme. Hence Modern controls /global/ /mozapps/ and all other toolkit theme locations.
Comment on attachment 8764366 [details] [diff] [review]
1279206-PasswordIcons-V3.patch

Thanks!
Attachment #8764366 - Flags: review?(philip.chee) → review+
Done

https://hg.mozilla.org/comm-central/rev/b82327300d15
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.