420732 - Add load_toolbar_icons pref to content panel

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, defect)

Description

[Robert Kaiser]

We should add setting the browser.chrome.load_toolbar_icons pref to the icons box of the new content pref panel.

Comment 1

[Robert Kaiser]

I'll also add the string changes for consistency of the panel with help wrt bug 423281 here, as I need to touch this locale file anyway in this bug.

Comment 2

[Robert Kaiser]

Attachment: patch v1: add toolbar icons pref

Here's a patch that adds this pref to the content panel. The texts on the radio are not set in stone, but those are the most descriptive variants I could think of.
This patch also makes the panel consistent in using "website" in the other texts (just as help already does).

Comment 3

[[e-mail]]

Comment on attachment 309838 [details] [diff] [review]
patch v1: add toolbar icons pref

>+      <radiogroup id="loadToolbarIcons"
class="indent" perhaps? What do you think?

Comment 4

[Robert Kaiser]

(In reply to comment #3)
> (From update of attachment 309838 [details] [diff] [review])
> >+      <radiogroup id="loadToolbarIcons"
> class="indent" perhaps? What do you think?

Looks fine to me, added locally.

Comment 5

[Ian Neal]

Comment on attachment 309838 [details] [diff] [review]
patch v1: add toolbar icons pref

>Index: mozilla/suite/common/pref/pref-content.xul
>===================================================================
>@@ -74,14 +77,29 @@
>                 label="&useSiteIcons.label;"
>                 accesskey="&useSiteIcons.accesskey;"
>                 preference="browser.chrome.site_icons"/>
>       <checkbox id="useFavIcons"
>                 label="&useFavIcons.label;"
>                 accesskey="&useFavIcons.accesskey;"
>                 preference="browser.chrome.favicons"/>
>+
>+      <separator class="thin"/>
>+      <description>&tbIconsDescription.label;</description>
>+      <radiogroup id="loadToolbarIcons"
As Neil said, with indentation looks better.

>Index: mozilla/suite/locales/en-US/chrome/common/pref/pref-content.dtd
>===================================================================
>+<!ENTITY siteIcons.label                        "Website Icons">
Nit: inconsistent use of capitalisation on "icons".
>+<!ENTITY useSiteIcons.label                     "Show website icons">
> <!ENTITY useSiteIcons.accesskey                 "S">
>-<!ENTITY useFavIcons.label                      "Aggressively look for Web Site Icons when the page does not define one">
>+<!ENTITY useFavIcons.label                      "Aggressively look for website icons when the page does not define one">

>Index: mozilla/suite/locales/en-US/chrome/common/help/cs_nav_prefs_appearance.xhtml
>===================================================================
>@@ -120,14 +120,28 @@
>+      <li>Display website icons in bookmarks menu and toolbar:
>+        <ul>
>+          <li><strong>Never show icons for bookmarks</strong>: Select this if
>+            you only want to see the default icons but not the website's own
>+            icon in the bookmarks menu or the personal toolbar.</li>
>+          <li><strong>Only when website was loaded recently</strong>: Select
>+            this to show the website's own icon only if the website has been
>+            recently loaded and the icon is currently in the browser's
>+            cache.</li>
>+          <li><strong>Always load website icons for bookmarks</strong>: Select
>+            this to always load website icons to be displayed in the bookmarks
>+            menu or personal toolbar, even if it's not in the cache.</li>
>+        </ul>
>+      </li>
Nit: use &apos; instead of '

Is the backend to this preference working, as no matter what the preference is set to I do not get any icons in my list of bookmarks.

r=me with the above changes and answer.

Comment 6

[Robert Kaiser]

(In reply to comment #5)
> Is the backend to this preference working, as no matter what the preference is
> set to I do not get any icons in my list of bookmarks.

I tried all three settings in my builds and see it working fine - easiest to see with the personal toolbar, of course. I think the whole feature has been reworked in FF there and is more reliable there (and missing this pref), though.
The middle option, which is only using cached icons, is a bit unreliable and loses images quite often.
Of course, it only loads icons when the whole feature of website icons is on, and it only uses favicons when the favicon option is on. Site like www.mozilla.org, MozillaZine or Bugzilla load their icons nicely even without the aggressive favicon option, though, and also show them in the personal toolbar and bookmarks.

Comment 7

[Robert Kaiser]

Oh, forgot to note that I just checked in this patch.