Closed Bug 1367864 Opened 4 years ago Closed 4 years ago

Clean up bookmark-item graphics logo usage in SeaMonkey themes


(SeaMonkey :: Themes, enhancement)

Not set


(seamonkey2.53 fixed)

Tracking Status
seamonkey2.53 --- fixed


(Reporter: frg, Assigned: frg)



(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #1367542 +++

We now use bookmark-item.png for defaultFavicon.png and defaultFavicon.svg. This is more of a hack. We need a proper svg. There is also a bookmark-item.gif still used in some places which needs to be removed.
Attached patch 1367864-bookmarkItem.patch (obsolete) — Splinter Review
Richard provided some converted svgs. They work fine. I am not setting review yet. I might get real vector ones from another Richard soon. 

Anyone more familiar with svgs. Can we just replace the pngs generally with them?
(In reply to Frank-Rainer Grahl from comment #1)
> Anyone more familiar with svgs. Can we just replace the pngs generally with
> them?

Now yes. Complex icon could use more memory. FX with it's simple monochrome icons improved the startup time against the bitmap icons.
Comment on attachment 8871400 [details] [diff] [review]

Gettting a vector based icon takes more time so use the current patch to remove the hack.
Attachment #8871400 - Flags: review?(
Comment on attachment 8871400 [details] [diff] [review]

> .treecell-panel
> {
>- list-style-image: url("chrome://communicator/skin/bookmarks/bookmark-item.gif");
>+ list-style-image: url("chrome://communicator/skin/bookmarks/bookmark-item.png");
> }

Did you test this /before/ or /after/ changing the manifest from PNG to SVG?

>-% override chrome://mozapps/skin/places/defaultFavicon.svg        chrome://communicator/skin/bookmarks/bookmark-item.png
>+% override chrome://mozapps/skin/places/defaultFavicon.svg        chrome://communicator/skin/bookmarks/bookmark-item.svg

Looks like it should be
>+ list-style-image: url("chrome://communicator/skin/bookmarks/bookmark-item.svg");
or what am I missing?
Comment on attachment 8871400 [details] [diff] [review]

Drat. sorry older version of the patch. Need to extract a new one from my tree.
Attachment #8871400 - Flags: review?(
We have a bookmark-item.gif and a bookmark-item.png. First patch was an older version and only took care of the gif. This one also replaces the png. Tested and, as Richard mentioned, svg now works fine everywhere.
Attachment #8871400 - Attachment is obsolete: true
Attachment #8878773 - Flags: review?(iann_bugzilla)
Comment on attachment 8878773 [details] [diff] [review]

Switching reviewers as just discussed.
Attachment #8878773 - Flags: review?(iann_bugzilla) → review?(
Comment on attachment 8878773 [details] [diff] [review]

LGTM, I didn't find any other occurrences of bookmark-item.png or bookmark-item.gif.

>+++ b/suite/themes/classic/communicator/bookmarks/bookmark-item.svg
>+<svg height="16" width="16" xmlns="" xmlns:xlink="">
>+<image xlink:href="data:image/png;base64, ...
>+OvTPwhz+5giWNtcPheMCpetMnNIAAAAASUVORK5CYII=" width="16" height="16"/></svg>

I was wondering if height="16" and width="16" have to be listed twice here or if either of them may be redundant, but the inner specifications are for the <image> bitmap as such and the outer for the <svg> as a whole, thus probably makes it more efficient to have them explicitly stated rather than implied (= ok as is).
Attachment #8878773 - Flags: review?( → review+
Target SeaMonkey 2.53
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.53
Just came across this landing by accident, any idea what to do about AKA, all the copies of this across the localizations.
Hmm good question. compare-locales will likely not look in the help files. 

Its non fatal and means just the icon will not be displayed.

IanN is there a standard procedure for this? Should we just make sure all images in the help files are located in suite\locales\en-US\chrome\common\help\images? There sure will be some further png to svg conversions required.

Flags: needinfo?(iann_bugzilla)
This has always been a problem with the Help content, not much we can do about it.
Also, not all locales actually translate Help, so it's up to them to watch it or not.
Usually we would check the locales impacted, that are still shipped for SM, and flag it up to them. This could be done by posting in l10n groups and logging bugs.
Flags: needinfo?(iann_bugzilla)
You need to log in before you can comment on or make changes to this bug.