476740 - Favicon not limited to 16x16 in Bookmarks sidebar and Bookmark Manager

Status: RESOLVED FIXED

(SeaMonkey :: Bookmarks & History, defect)

Description

[Jens Hatlak (:InvisibleSmiley)]

flyguy reported a problem in mozilla.support.seamonkey (thread: "favicon" displays 2 inches wide!) where he saw a website icon (favicon) displayed wider that the normal 16 pixels in the Bookmarks sidebar. I can confirm the problem with the following two URLs and also see it in the Bookmark Manager:

1. <http://share.findmespot.com/shared/faces/images/spot-icon-big.ico>: 255x222 pixels, obviously sent with wrong content type; save to disk and test.
2. <http://acid3.acidtests.org/favicon.ico>: 150x150 pixels.

As far as I can see the favicon is displayed correctly in the location bar because a CSS rule from navigator.css (#page-proxy-favicon) sets it to 16x16 pixels:

<http://mxr.mozilla.org/comm-central/source/suite/themes/classic/navigator/navigator.css#342>

The DOM Inspector is not really helpful for checking anything below treechildren so I hope someone with more insight can help identify what exactly needs to be added where to fix the Bookmarks sidebar and Bookmark Manager.

Comment 1

[Jens Hatlak (:InvisibleSmiley)]

Seems we are missing this rule:
<http://mxr.mozilla.org/mozilla-central/source/browser/themes/winstripe/browser/places/places.css#23>
I verified using the Stylish extension (userChrome.css should do, too) that the following rule fixes both the Bookmarks sidebar and the Bookmark Manager:

#bookmarks-view treechildren::-moz-tree-image {
   padding-right: 2px;
   margin: 0px 2px;
   width: 16px;
   height: 16px;
}

This could be added (without #bookmarks-view) to bookmarks.css of both themes. I'd be happy to come up with a patch but I'm still wondering what the following rule is intended for:
<http://mxr.mozilla.org/comm-central/source/suite/themes/classic/communicator/bookmarks/bookmarks.css#76>

Comment 2

[Jens Hatlak (:InvisibleSmiley)]

Attachment: proposed patch

Added "Name" to only refer to that column (called "title" in FF) and matching rule to not break separators (also like FF does it).

The other rule (bookmarks.css#76) is obviously for the Personal Toolbar, thus unrelated.

Comment 3

[[e-mail]]

Comment on attachment 360583 [details] [diff] [review]
proposed patch

>   -moz-margin-end: 2px;
>+  padding-right: 2px;
>+  margin: 0px 2px;
This doesn't seem to make sense. Just set the size, please?

>+  width: 0;
>+  height: 0;
Please write 0px here as that's what we use everywhere else.

Comment 4

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch v2 [Checkin: Comment 5]

Nits addressed (assuming -moz-margin-end should stay since it has been there before).

Comment 5

[Serge Gautherie (:sgautherie)]

Comment on attachment 360595 [details] [diff] [review]
patch v2
[Checkin: Comment 5]


http://hg.mozilla.org/comm-central/rev/223c017c5a3d