If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Look for favicon.ico in Seamonkey by default if website defines no favicon.

RESOLVED FIXED in seamonkey2.48

Status

SeaMonkey
Bookmarks & History
--
minor
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: frg, Assigned: frg)

Tracking

Trunk
seamonkey2.48

SeaMonkey Tracking Flags

(seamonkey2.48 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

a year ago
The preference browser.chrome.favicons is set to false by default. This results in many websites not showing a favicon. Tb and FF have this on.

Also the text is imho wrong. The setting does more than looking for icons. It manages and makes them usable in bookmarks and history too. 

My personal oppinion is that we maybe should set browser.chrome.site_icons and browser.chrome.favicons to true and drop them from the prefs pane. They can still be changed in about:config.
(Assignee)

Comment 1

a year ago
Created attachment 8769470 [details] [diff] [review]
1285798-favicons.patch

Turning on the pref and changed the wording. Agressively implied that it's not a normal behaviour or out of scope. 

Needs a help change if accepted too. Or we just drop both prefs in the ui.
Attachment #8769470 - Flags: review?(philip.chee)
(Assignee)

Comment 2

a year ago
Patch includes some whitespace changes in the adjacent area of the change.
(Assignee)

Comment 3

a year ago
Created attachment 8769471 [details] [diff] [review]
1285798-favicons.patch

Previous patch missed the xul part.
Attachment #8769470 - Attachment is obsolete: true
Attachment #8769470 - Flags: review?(philip.chee)
Attachment #8769471 - Flags: review?(philip.chee)

Comment 4

a year ago
Comment on attachment 8769471 [details] [diff] [review]
1285798-favicons.patch

1. Please Please Please whitespace changes in a separate patch

2.
> "Enhanced search for website icons when the page doesn't define one"
I think "search" gives the wrong impression. How about:
"Check for favicons as well when looking for website icons to display"

3. 
> -pref("browser.chrome.favicons", false);
> +pref("browser.chrome.favicons", true);
I would say no. Maybe post a message to seamonkey-members mailing list to see if we can have a consensus on thi.
Attachment #8769471 - Flags: review?(philip.chee)
Attachment #8769471 - Flags: review?(iann_bugzilla)
Attachment #8769471 - Flags: review-

Comment 5

a year ago
> > -pref("browser.chrome.favicons", false);
> > +pref("browser.chrome.favicons", true);
> I would say no. Maybe post a message to seamonkey-members mailing list to
> see if we can have a consensus on this.
Flags: needinfo?(neil)

Comment 6

a year ago
(In reply to Philip Chee from comment #4)
> Comment on attachment 8769471 [details] [diff] [review]
> 1285798-favicons.patch

> > "Enhanced search for website icons when the page doesn't define one"
> I think "search" gives the wrong impression. How about:
> "Check for favicons as well when looking for website icons to display"
What is the difference between "website icons" and "favicons"?

> > -pref("browser.chrome.favicons", false);
> > +pref("browser.chrome.favicons", true);
> I would say no. Maybe post a message to seamonkey-members mailing list to
> see if we can have a consensus on thi.
Might depend on the answer to the above, but also does it slow anything down?
Flags: needinfo?(philip.chee)

Comment 7

a year ago
Comment on attachment 8769471 [details] [diff] [review]
1285798-favicons.patch

>+++ b/suite/locales/en-US/chrome/common/pref/pref-content.dtd
>-<!ENTITY useFavIcons.label                      "Aggressively look for website icons when the page doesn&apos;t define one">
>-<!ENTITY useFavIcons.accesskey                  "A">
>+<!ENTITY useFavIcons2.label                     "Enhanced search for website icons when the page doesn&apos;t define one">
>+<!ENTITY useFavIcons2.accesskey                 "E">
Perhaps "Analyse website for suitable icon when the page doesn&apos;t define one"?
On balance I think it would be fine to have it true as default.
Attachment #8769471 - Flags: review?(iann_bugzilla) → review+

Comment 8

a year ago
Comment on attachment 8769471 [details] [diff] [review]
1285798-favicons.patch

(In reply to Ian Neal from comment #7)
> >+<!ENTITY useFavIcons2.accesskey                 "E">
> Perhaps "Analyse website for suitable icon when the page doesn&apos;t define one"?
* Scrounge around the website for a suitable icon when the page doesn't define one.
* If the page doesn't define a website icon, try to find a suitable icon from/ on/ in the website.

> On balance I think it would be fine to have it true as default.
I defer to IanN on this one.
Flags: needinfo?(philip.chee)
Attachment #8769471 - Flags: review- → review+
(Assignee)

Comment 9

a year ago
Great. I really think its best that the option is turned on. We know how to do it anyway but for end users its imho the better default. 

Let me look into the favicon code and see what the option really does. maybe I find a suitable text for it there. Stay tuned.
(Assignee)

Updated

a year ago
Summary: Use favicon-service by default in Seamonkey → Look for favicon.ico in Seamonkey by default if website defines no favicon.
(Assignee)

Comment 10

a year ago
How about: 

Try to use the server favicon when the page doesn't define an icon.

This is consistent with the help which actually has it right.

Comment 11

a year ago
(In reply to Frank-Rainer Grahl from comment #10)
> How about: 
> 
> Try to use the server favicon when the page doesn't define an icon.
> 
> This is consistent with the help which actually has it right.

Seems reasonable, maybe server's rather than server.

Either way, yes.
(Assignee)

Comment 12

a year ago
Created attachment 8792216 [details] [diff] [review]
1285798-favicons-V2.patch

New patch with string changed. Access key adjusted and tested. r+ from IanN and Philip Chee carried forward.

Needinfo cleared.
Attachment #8769471 - Attachment is obsolete: true
Flags: needinfo?(neil)
Attachment #8792216 - Flags: review+
(Assignee)

Comment 13

a year ago
Closed until further notice.

https://hg.mozilla.org/comm-central/rev/e295e24c8575
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-seamonkey2.48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.48
You need to log in before you can comment on or make changes to this bug.