[drupal-devel] [feature] favicon patch for themes
Issue status update for http://drupal.org/node/20809 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: nysus Reported by: nysus Updated by: nysus Status: patch Attachment: http://drupal.org/files/issues/favicon.patch (4.57 KB) Here is a patch that allows a user to manually set the path to the "favicon.ico" file. It makes a single change to the user interface: it adds a text field to the "Logo image setting" section of the "themes/settings" page. This is especially useful for sites sharing a single code base as you no longer need to create a new theme just so you can have a distinct favicon.ico. nysus
+1 to this concept, though it seems that it should be ok for themes to provide their own... Therefore if you specify a custom favicon in the theme admin section, you override the default, but otherwise the default is used. The pseudo-code in the theme would look like: if {drupal-favicon} output link to drupal-favicon else output link to theme-favicon end if -1 for putting the favicon in drupal_get_html_head. I also think that you should be able to upload the favicon like you can a logo in the theme admin section and that this should be a global theme setting instead of the current "enter a relative path for the favicon". Chris On 4/18/05, nysus <drupal-devel@drupal.org> wrote:
Issue status update for http://drupal.org/node/20809
Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: nysus Reported by: nysus Updated by: nysus Status: patch Attachment: http://drupal.org/files/issues/favicon.patch (4.57 KB)
Here is a patch that allows a user to manually set the path to the "favicon.ico" file. It makes a single change to the user interface: it adds a text field to the "Logo image setting" section of the "themes/settings" page.
This is especially useful for sites sharing a single code base as you no longer need to create a new theme just so you can have a distinct favicon.ico.
nysus
Chris Messina wrote:
-1 for putting the favicon in drupal_get_html_head.
Not sure what you mean here. I didn't change the drupal_get_html_head() function. Perhaps you mean for NOT putting it there?
I also think that you should be able to upload the favicon like you can a logo in the theme admin section and that this should be a global theme setting instead of the current "enter a relative path for the favicon".
I purposefully left it out until I got feedback on the smaller change first. I'm worried it would get shot down for adding too much clutter.
Chris
On 4/18/05, nysus <drupal-devel@drupal.org> wrote:
Issue status update for http://drupal.org/node/20809
Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: nysus Reported by: nysus Updated by: nysus Status: patch Attachment: http://drupal.org/files/issues/favicon.patch (4.57 KB)
Here is a patch that allows a user to manually set the path to the "favicon.ico" file. It makes a single change to the user interface: it adds a text field to the "Logo image setting" section of the "themes/settings" page.
This is especially useful for sites sharing a single code base as you no longer need to create a new theme just so you can have a distinct favicon.ico.
nysus
Chris Messina wrote:
+1 to this concept, though it seems that it should be ok for themes to provide their own... Therefore if you specify a custom favicon in the theme admin section, you override the default, but otherwise the default is used. The pseudo-code in the theme would look like:
if {drupal-favicon} output link to drupal-favicon else output link to theme-favicon end if
This wouldn't work very well. Sites are pretty much limited to one favicon. That's because the browser stores the icon in its cache until it get deleted. So, for most users, changing the theme won't have any affect on the favicon. Also, I'm just not sure I agree that each theme should have its own favicon. Why would each theme need its own icon? What purpose would that serve? I think this would confuse users who expect there to be only one icon per site.
On 4/18/05, Steve Dondley <s@dondley.com> wrote:
Also, I'm just not sure I agree that each theme should have its own favicon. Why would each theme need its own icon? What purpose would that serve? I think this would confuse users who expect there to be only one icon per site.
If your run multiple, totally different sites from the same code base an icon in each theme's directory and a seperate theme per site gives you an individual icon per site. I think, though, it would be better to have the favicon.ico in the sites/example.com directory.
Chris Messina wrote:
+1 to this concept, though it seems that it should be ok for themes to provide their own... Therefore if you specify a custom favicon in the theme admin section, you override the default, but otherwise the default is used. The pseudo-code in the theme would look like:
if {drupal-favicon} output link to drupal-favicon else output link to theme-favicon end if
-1 for putting the favicon in drupal_get_html_head.
I also think that you should be able to upload the favicon like you can a logo in the theme admin section and that this should be a global theme setting instead of the current "enter a relative path for the favicon".
A big +1... a little more formally: - We leave the default favicon.ico in root. - Allow themes to put a favicon.ico in their folder. - Provide an admin option to allow you to change the favicon, like we do now for the logo. This value defaults to the theme favicon if there is one, the drupal favicon otherwise. We designate which favicon to use through a <link rel="shortcut icon" /> tag in the head. Steven Wittens
Issue status update for http://drupal.org/node/20809 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: nysus Reported by: nysus Updated by: Steven Status: patch There is a lot of mailing list discussion about this patch. One more note though: the <link /> tags are missing the trailing slash for XHTML. Steven Previous comments: ------------------------------------------------------------------------ April 19, 2005 - 01:42 : nysus Attachment: http://drupal.org/files/issues/favicon.patch (4.57 KB) Here is a patch that allows a user to manually set the path to the "favicon.ico" file. It makes a single change to the user interface: it adds a text field to the "Logo image setting" section of the "themes/settings" page. This is especially useful for sites sharing a single code base as you no longer need to create a new theme just so you can have a distinct favicon.ico.
Issue status update for http://drupal.org/node/20809 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: nysus Reported by: nysus Updated by: nysus Status: patch Attachment: http://drupal.org/files/issues/favicon2.patch (6.01 KB) Here is a revision to the patch based on the feedback received on the discussion list. You can now override the default favicon the same you way you can override the default site logo. There is one difference: When a logo is uploaded, it is checked to determine whether it is a valid image file using PHP's getimagesize function. Since this function does not work with .ico files, no validation of the uploaded file is performed. Advice on how (or if) validation needs to be done---or any other suggestions and corrections---is welcome. nysus Previous comments: ------------------------------------------------------------------------ April 18, 2005 - 19:42 : nysus Attachment: http://drupal.org/files/issues/favicon.patch (4.57 KB) Here is a patch that allows a user to manually set the path to the "favicon.ico" file. It makes a single change to the user interface: it adds a text field to the "Logo image setting" section of the "themes/settings" page. This is especially useful for sites sharing a single code base as you no longer need to create a new theme just so you can have a distinct favicon.ico. ------------------------------------------------------------------------ April 23, 2005 - 00:28 : Steven There is a lot of mailing list discussion about this patch. One more note though: the <link /> tags are missing the trailing slash for XHTML.
Issue status update for http://drupal.org/node/20809 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: nysus Reported by: nysus Updated by: Steven Status: patch There is still a slash missing in bluemarine. This patch appears to have per-theme favicons, but the setting to control this is in fact defined by the theme engine, not the template (through the list of support theme features). I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory. But I'm not sure if we really need this: it does make it very obvious to people editing a theme that there is an icon file they can change. On the other hand, few people will use a theme's default favicon. It's more useful as a customization, and the UI option is good enough for that. Some text suggestions: "favorite icon" is not clear. I think the phrase "shortcut icon" is better (that's the standard name) if we accompany it with more text. Something like: "Check this box to use the default shortcut icon or 'favicon'. This icon is displayed in the address bar and bookmarks in most browsers." "The path to the icon file you would like to use as your custom shortcut icon." (note: i replaced "image file" with "icon file" because for IE, favicons need to be .ico files) 'Shortcut icon settings' Steven Previous comments: ------------------------------------------------------------------------ April 19, 2005 - 01:42 : nysus Attachment: http://drupal.org/files/issues/favicon.patch (4.57 KB) Here is a patch that allows a user to manually set the path to the "favicon.ico" file. It makes a single change to the user interface: it adds a text field to the "Logo image setting" section of the "themes/settings" page. This is especially useful for sites sharing a single code base as you no longer need to create a new theme just so you can have a distinct favicon.ico. ------------------------------------------------------------------------ April 23, 2005 - 06:28 : Steven There is a lot of mailing list discussion about this patch. One more note though: the <link /> tags are missing the trailing slash for XHTML. ------------------------------------------------------------------------ April 23, 2005 - 17:06 : nysus Attachment: http://drupal.org/files/issues/favicon2.patch (6.01 KB) Here is a revision to the patch based on the feedback received on the discussion list. You can now override the default favicon the same you way you can override the default site logo. There is one difference: When a logo is uploaded, it is checked to determine whether it is a valid image file using PHP's getimagesize function. Since this function does not work with .ico files, no validation of the uploaded file is performed. Advice on how (or if) validation needs to be done---or any other suggestions and corrections---is welcome.
Issue status update for http://drupal.org/node/20809 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: nysus Reported by: nysus Updated by: nysus Status: patch
I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory.
This wouldn't work very well for sites sharing the same code base. Using your suggestion, you would have to create a custom theme just to have a customized favorite icon.
But I'm not sure if we really need this...
Can you please clarify to what "this" refers to? It's unclear to me what your suggestion is. I'll incorporate your other suggestions and corrections into the next revision. nysus Previous comments: ------------------------------------------------------------------------ April 18, 2005 - 19:42 : nysus Attachment: http://drupal.org/files/issues/favicon.patch (4.57 KB) Here is a patch that allows a user to manually set the path to the "favicon.ico" file. It makes a single change to the user interface: it adds a text field to the "Logo image setting" section of the "themes/settings" page. This is especially useful for sites sharing a single code base as you no longer need to create a new theme just so you can have a distinct favicon.ico. ------------------------------------------------------------------------ April 23, 2005 - 00:28 : Steven There is a lot of mailing list discussion about this patch. One more note though: the <link /> tags are missing the trailing slash for XHTML. ------------------------------------------------------------------------ April 23, 2005 - 11:06 : nysus Attachment: http://drupal.org/files/issues/favicon2.patch (6.01 KB) Here is a revision to the patch based on the feedback received on the discussion list. You can now override the default favicon the same you way you can override the default site logo. There is one difference: When a logo is uploaded, it is checked to determine whether it is a valid image file using PHP's getimagesize function. Since this function does not work with .ico files, no validation of the uploaded file is performed. Advice on how (or if) validation needs to be done---or any other suggestions and corrections---is welcome. ------------------------------------------------------------------------ April 23, 2005 - 13:45 : Steven There is still a slash missing in bluemarine. This patch appears to have per-theme favicons, but the setting to control this is in fact defined by the theme engine, not the template (through the list of support theme features). I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory. But I'm not sure if we really need this: it does make it very obvious to people editing a theme that there is an icon file they can change. On the other hand, few people will use a theme's default favicon. It's more useful as a customization, and the UI option is good enough for that. Some text suggestions: "favorite icon" is not clear. I think the phrase "shortcut icon" is better (that's the standard name) if we accompany it with more text. Something like: "Check this box to use the default shortcut icon or 'favicon'. This icon is displayed in the address bar and bookmarks in most browsers." "The path to the icon file you would like to use as your custom shortcut icon." (note: i replaced "image file" with "icon file" because for IE, favicons need to be .ico files) 'Shortcut icon settings'
Issue status update for http://drupal.org/node/20809 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: nysus Reported by: nysus Updated by: kbahey Status: patch The patch as it stands is only for xtemplate. What about phptemplate? Will it work with phptemplate? +1 on the concept though. I like a more generic solution, like what you proposed, that allows the favico to be added from an admin interface just like the logo. As for the comment:
I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory.
This wouldn't work very well for sites sharing the same code base. Using your suggestion, you would have to create a custom theme just to have a customized favorite icon.
If the theme is installed under sites/sitename/themes then its favico will only show for the given site. kbahey Previous comments: ------------------------------------------------------------------------ April 18, 2005 - 18:42 : nysus Attachment: http://drupal.org/files/issues/favicon.patch (4.57 KB) Here is a patch that allows a user to manually set the path to the "favicon.ico" file. It makes a single change to the user interface: it adds a text field to the "Logo image setting" section of the "themes/settings" page. This is especially useful for sites sharing a single code base as you no longer need to create a new theme just so you can have a distinct favicon.ico. ------------------------------------------------------------------------ April 22, 2005 - 23:28 : Steven There is a lot of mailing list discussion about this patch. One more note though: the <link /> tags are missing the trailing slash for XHTML. ------------------------------------------------------------------------ April 23, 2005 - 10:06 : nysus Attachment: http://drupal.org/files/issues/favicon2.patch (6.01 KB) Here is a revision to the patch based on the feedback received on the discussion list. You can now override the default favicon the same you way you can override the default site logo. There is one difference: When a logo is uploaded, it is checked to determine whether it is a valid image file using PHP's getimagesize function. Since this function does not work with .ico files, no validation of the uploaded file is performed. Advice on how (or if) validation needs to be done---or any other suggestions and corrections---is welcome. ------------------------------------------------------------------------ April 23, 2005 - 12:45 : Steven There is still a slash missing in bluemarine. This patch appears to have per-theme favicons, but the setting to control this is in fact defined by the theme engine, not the template (through the list of support theme features). I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory. But I'm not sure if we really need this: it does make it very obvious to people editing a theme that there is an icon file they can change. On the other hand, few people will use a theme's default favicon. It's more useful as a customization, and the UI option is good enough for that. Some text suggestions: "favorite icon" is not clear. I think the phrase "shortcut icon" is better (that's the standard name) if we accompany it with more text. Something like: "Check this box to use the default shortcut icon or 'favicon'. This icon is displayed in the address bar and bookmarks in most browsers." "The path to the icon file you would like to use as your custom shortcut icon." (note: i replaced "image file" with "icon file" because for IE, favicons need to be .ico files) 'Shortcut icon settings' ------------------------------------------------------------------------ April 23, 2005 - 13:07 : nysus
I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory.
This wouldn't work very well for sites sharing the same code base. Using your suggestion, you would have to create a custom theme just to have a customized favorite icon.
But I'm not sure if we really need this...
Can you please clarify to what "this" refers to? It's unclear to me what your suggestion is. I'll incorporate your other suggestions and corrections into the next revision.
Issue status update for http://drupal.org/node/20809 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: nysus Reported by: nysus Updated by: Steven Status: patch What I meant was to have the favicon default to themes/themename/favicon.ico if it exists, and use favicon.ico otherwise. I would still allow you to override the favicon regardless over the admin interface. But I'm not sure if default per-theme icons are useful anymore (that's what I meant with 'this'). Steven Previous comments: ------------------------------------------------------------------------ April 19, 2005 - 01:42 : nysus Attachment: http://drupal.org/files/issues/favicon.patch (4.57 KB) Here is a patch that allows a user to manually set the path to the "favicon.ico" file. It makes a single change to the user interface: it adds a text field to the "Logo image setting" section of the "themes/settings" page. This is especially useful for sites sharing a single code base as you no longer need to create a new theme just so you can have a distinct favicon.ico. ------------------------------------------------------------------------ April 23, 2005 - 06:28 : Steven There is a lot of mailing list discussion about this patch. One more note though: the <link /> tags are missing the trailing slash for XHTML. ------------------------------------------------------------------------ April 23, 2005 - 17:06 : nysus Attachment: http://drupal.org/files/issues/favicon2.patch (6.01 KB) Here is a revision to the patch based on the feedback received on the discussion list. You can now override the default favicon the same you way you can override the default site logo. There is one difference: When a logo is uploaded, it is checked to determine whether it is a valid image file using PHP's getimagesize function. Since this function does not work with .ico files, no validation of the uploaded file is performed. Advice on how (or if) validation needs to be done---or any other suggestions and corrections---is welcome. ------------------------------------------------------------------------ April 23, 2005 - 19:45 : Steven There is still a slash missing in bluemarine. This patch appears to have per-theme favicons, but the setting to control this is in fact defined by the theme engine, not the template (through the list of support theme features). I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory. But I'm not sure if we really need this: it does make it very obvious to people editing a theme that there is an icon file they can change. On the other hand, few people will use a theme's default favicon. It's more useful as a customization, and the UI option is good enough for that. Some text suggestions: "favorite icon" is not clear. I think the phrase "shortcut icon" is better (that's the standard name) if we accompany it with more text. Something like: "Check this box to use the default shortcut icon or 'favicon'. This icon is displayed in the address bar and bookmarks in most browsers." "The path to the icon file you would like to use as your custom shortcut icon." (note: i replaced "image file" with "icon file" because for IE, favicons need to be .ico files) 'Shortcut icon settings' ------------------------------------------------------------------------ April 23, 2005 - 20:07 : nysus
I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory.
This wouldn't work very well for sites sharing the same code base. Using your suggestion, you would have to create a custom theme just to have a customized favorite icon.
But I'm not sure if we really need this...
Can you please clarify to what "this" refers to? It's unclear to me what your suggestion is. I'll incorporate your other suggestions and corrections into the next revision. ------------------------------------------------------------------------ April 23, 2005 - 20:23 : kbahey The patch as it stands is only for xtemplate. What about phptemplate? Will it work with phptemplate? +1 on the concept though. I like a more generic solution, like what you proposed, that allows the favico to be added from an admin interface just like the logo. As for the comment:
I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory.
This wouldn't work very well for sites sharing the same code base. Using your suggestion, you would have to create a custom theme just to have a customized favorite icon.
If the theme is installed under sites/sitename/themes then its favico will only show for the given site.
Issue status update for http://drupal.org/node/20809 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: nysus Reported by: nysus Updated by: nysus Status: patch
The patch as it stands is only for xtemplate. What about phptemplate? Will it work with phptemplate?
xtemplate is the only templating engine in the Drupal core. This patch will not work with phptemplate until the maintainer of phptemplate engine incorporates favicon functionality into the phptemplate engine.
If the theme is installed under sites/sitename/themes then its favico will only show for the given site.
Yes, I understand this, but it's a real hassle to have to create a new theme directory just to facilitate the creation of a favicon. I feel strongly that a customized favicon is a common enough task that it should be built into the Drupal user interface and should be made as easy as possible. nysus Previous comments: ------------------------------------------------------------------------ April 18, 2005 - 19:42 : nysus Attachment: http://drupal.org/files/issues/favicon.patch (4.57 KB) Here is a patch that allows a user to manually set the path to the "favicon.ico" file. It makes a single change to the user interface: it adds a text field to the "Logo image setting" section of the "themes/settings" page. This is especially useful for sites sharing a single code base as you no longer need to create a new theme just so you can have a distinct favicon.ico. ------------------------------------------------------------------------ April 23, 2005 - 00:28 : Steven There is a lot of mailing list discussion about this patch. One more note though: the <link /> tags are missing the trailing slash for XHTML. ------------------------------------------------------------------------ April 23, 2005 - 11:06 : nysus Attachment: http://drupal.org/files/issues/favicon2.patch (6.01 KB) Here is a revision to the patch based on the feedback received on the discussion list. You can now override the default favicon the same you way you can override the default site logo. There is one difference: When a logo is uploaded, it is checked to determine whether it is a valid image file using PHP's getimagesize function. Since this function does not work with .ico files, no validation of the uploaded file is performed. Advice on how (or if) validation needs to be done---or any other suggestions and corrections---is welcome. ------------------------------------------------------------------------ April 23, 2005 - 13:45 : Steven There is still a slash missing in bluemarine. This patch appears to have per-theme favicons, but the setting to control this is in fact defined by the theme engine, not the template (through the list of support theme features). I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory. But I'm not sure if we really need this: it does make it very obvious to people editing a theme that there is an icon file they can change. On the other hand, few people will use a theme's default favicon. It's more useful as a customization, and the UI option is good enough for that. Some text suggestions: "favorite icon" is not clear. I think the phrase "shortcut icon" is better (that's the standard name) if we accompany it with more text. Something like: "Check this box to use the default shortcut icon or 'favicon'. This icon is displayed in the address bar and bookmarks in most browsers." "The path to the icon file you would like to use as your custom shortcut icon." (note: i replaced "image file" with "icon file" because for IE, favicons need to be .ico files) 'Shortcut icon settings' ------------------------------------------------------------------------ April 23, 2005 - 14:07 : nysus
I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory.
This wouldn't work very well for sites sharing the same code base. Using your suggestion, you would have to create a custom theme just to have a customized favorite icon.
But I'm not sure if we really need this...
Can you please clarify to what "this" refers to? It's unclear to me what your suggestion is. I'll incorporate your other suggestions and corrections into the next revision. ------------------------------------------------------------------------ April 23, 2005 - 14:23 : kbahey The patch as it stands is only for xtemplate. What about phptemplate? Will it work with phptemplate? +1 on the concept though. I like a more generic solution, like what you proposed, that allows the favico to be added from an admin interface just like the logo. As for the comment:
I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory.
This wouldn't work very well for sites sharing the same code base. Using your suggestion, you would have to create a custom theme just to have a customized favorite icon.
If the theme is installed under sites/sitename/themes then its favico will only show for the given site. ------------------------------------------------------------------------ April 23, 2005 - 14:41 : Steven What I meant was to have the favicon default to themes/themename/favicon.ico if it exists, and use favicon.ico otherwise. I would still allow you to override the favicon regardless over the admin interface. But I'm not sure if default per-theme icons are useful anymore (that's what I meant with 'this').
Issue status update for http://drupal.org/node/20809 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: nysus Reported by: nysus Updated by: nysus Status: patch Attachment: http://drupal.org/files/issues/favicon3.patch (6.59 KB) Here is a revision to the patch that incorporates some suggestions that have been made here. It also fixes a bug in the previous version of the patch. nysus Previous comments: ------------------------------------------------------------------------ April 18, 2005 - 19:42 : nysus Attachment: http://drupal.org/files/issues/favicon.patch (4.57 KB) Here is a patch that allows a user to manually set the path to the "favicon.ico" file. It makes a single change to the user interface: it adds a text field to the "Logo image setting" section of the "themes/settings" page. This is especially useful for sites sharing a single code base as you no longer need to create a new theme just so you can have a distinct favicon.ico. ------------------------------------------------------------------------ April 23, 2005 - 00:28 : Steven There is a lot of mailing list discussion about this patch. One more note though: the <link /> tags are missing the trailing slash for XHTML. ------------------------------------------------------------------------ April 23, 2005 - 11:06 : nysus Attachment: http://drupal.org/files/issues/favicon2.patch (6.01 KB) Here is a revision to the patch based on the feedback received on the discussion list. You can now override the default favicon the same you way you can override the default site logo. There is one difference: When a logo is uploaded, it is checked to determine whether it is a valid image file using PHP's getimagesize function. Since this function does not work with .ico files, no validation of the uploaded file is performed. Advice on how (or if) validation needs to be done---or any other suggestions and corrections---is welcome. ------------------------------------------------------------------------ April 23, 2005 - 13:45 : Steven There is still a slash missing in bluemarine. This patch appears to have per-theme favicons, but the setting to control this is in fact defined by the theme engine, not the template (through the list of support theme features). I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory. But I'm not sure if we really need this: it does make it very obvious to people editing a theme that there is an icon file they can change. On the other hand, few people will use a theme's default favicon. It's more useful as a customization, and the UI option is good enough for that. Some text suggestions: "favorite icon" is not clear. I think the phrase "shortcut icon" is better (that's the standard name) if we accompany it with more text. Something like: "Check this box to use the default shortcut icon or 'favicon'. This icon is displayed in the address bar and bookmarks in most browsers." "The path to the icon file you would like to use as your custom shortcut icon." (note: i replaced "image file" with "icon file" because for IE, favicons need to be .ico files) 'Shortcut icon settings' ------------------------------------------------------------------------ April 23, 2005 - 14:07 : nysus
I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory.
This wouldn't work very well for sites sharing the same code base. Using your suggestion, you would have to create a custom theme just to have a customized favorite icon.
But I'm not sure if we really need this...
Can you please clarify to what "this" refers to? It's unclear to me what your suggestion is. I'll incorporate your other suggestions and corrections into the next revision. ------------------------------------------------------------------------ April 23, 2005 - 14:23 : kbahey The patch as it stands is only for xtemplate. What about phptemplate? Will it work with phptemplate? +1 on the concept though. I like a more generic solution, like what you proposed, that allows the favico to be added from an admin interface just like the logo. As for the comment:
I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory.
This wouldn't work very well for sites sharing the same code base. Using your suggestion, you would have to create a custom theme just to have a customized favorite icon.
If the theme is installed under sites/sitename/themes then its favico will only show for the given site. ------------------------------------------------------------------------ April 23, 2005 - 14:41 : Steven What I meant was to have the favicon default to themes/themename/favicon.ico if it exists, and use favicon.ico otherwise. I would still allow you to override the favicon regardless over the admin interface. But I'm not sure if default per-theme icons are useful anymore (that's what I meant with 'this'). ------------------------------------------------------------------------ April 23, 2005 - 14:52 : nysus
The patch as it stands is only for xtemplate. What about phptemplate? Will it work with phptemplate?
xtemplate is the only templating engine in the Drupal core. This patch will not work with phptemplate until the maintainer of phptemplate engine incorporates favicon functionality into the phptemplate engine.
If the theme is installed under sites/sitename/themes then its favico will only show for the given site.
Yes, I understand this, but it's a real hassle to have to create a new theme directory just to facilitate the creation of a favicon. I feel strongly that a customized favicon is a common enough task that it should be built into the Drupal user interface and should be made as easy as possible.
Issue status update for http://drupal.org/node/20809 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: nysus Reported by: nysus Updated by: Junyor Status: patch Why does this have to be part of the theme at all? I think others mentioned on the mailing list discussion that favicons are site-specific, not theme-specific. Thus, they are candidates for inclusion in drupal/sites/ rather than as part of a theme. Then, the LINK element can be included with drupal_set_head (or whatever it's called). I like having the setting in the admin settings, similar to the site logo setting. And don't forget: favicons can be any image type these days. They won't necessarily by .ico files. So, +1 for better customization of favicons and -1 for the implementation. Junyor Previous comments: ------------------------------------------------------------------------ April 19, 2005 - 00:42 : nysus Attachment: http://drupal.org/files/issues/favicon.patch (4.57 KB) Here is a patch that allows a user to manually set the path to the "favicon.ico" file. It makes a single change to the user interface: it adds a text field to the "Logo image setting" section of the "themes/settings" page. This is especially useful for sites sharing a single code base as you no longer need to create a new theme just so you can have a distinct favicon.ico. ------------------------------------------------------------------------ April 23, 2005 - 05:28 : Steven There is a lot of mailing list discussion about this patch. One more note though: the <link /> tags are missing the trailing slash for XHTML. ------------------------------------------------------------------------ April 23, 2005 - 16:06 : nysus Attachment: http://drupal.org/files/issues/favicon2.patch (6.01 KB) Here is a revision to the patch based on the feedback received on the discussion list. You can now override the default favicon the same you way you can override the default site logo. There is one difference: When a logo is uploaded, it is checked to determine whether it is a valid image file using PHP's getimagesize function. Since this function does not work with .ico files, no validation of the uploaded file is performed. Advice on how (or if) validation needs to be done---or any other suggestions and corrections---is welcome. ------------------------------------------------------------------------ April 23, 2005 - 18:45 : Steven There is still a slash missing in bluemarine. This patch appears to have per-theme favicons, but the setting to control this is in fact defined by the theme engine, not the template (through the list of support theme features). I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory. But I'm not sure if we really need this: it does make it very obvious to people editing a theme that there is an icon file they can change. On the other hand, few people will use a theme's default favicon. It's more useful as a customization, and the UI option is good enough for that. Some text suggestions: "favorite icon" is not clear. I think the phrase "shortcut icon" is better (that's the standard name) if we accompany it with more text. Something like: "Check this box to use the default shortcut icon or 'favicon'. This icon is displayed in the address bar and bookmarks in most browsers." "The path to the icon file you would like to use as your custom shortcut icon." (note: i replaced "image file" with "icon file" because for IE, favicons need to be .ico files) 'Shortcut icon settings' ------------------------------------------------------------------------ April 23, 2005 - 19:07 : nysus
I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory.
This wouldn't work very well for sites sharing the same code base. Using your suggestion, you would have to create a custom theme just to have a customized favorite icon.
But I'm not sure if we really need this...
Can you please clarify to what "this" refers to? It's unclear to me what your suggestion is. I'll incorporate your other suggestions and corrections into the next revision. ------------------------------------------------------------------------ April 23, 2005 - 19:23 : kbahey The patch as it stands is only for xtemplate. What about phptemplate? Will it work with phptemplate? +1 on the concept though. I like a more generic solution, like what you proposed, that allows the favico to be added from an admin interface just like the logo. As for the comment:
I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory.
This wouldn't work very well for sites sharing the same code base. Using your suggestion, you would have to create a custom theme just to have a customized favorite icon.
If the theme is installed under sites/sitename/themes then its favico will only show for the given site. ------------------------------------------------------------------------ April 23, 2005 - 19:41 : Steven What I meant was to have the favicon default to themes/themename/favicon.ico if it exists, and use favicon.ico otherwise. I would still allow you to override the favicon regardless over the admin interface. But I'm not sure if default per-theme icons are useful anymore (that's what I meant with 'this'). ------------------------------------------------------------------------ April 23, 2005 - 19:52 : nysus
The patch as it stands is only for xtemplate. What about phptemplate? Will it work with phptemplate?
xtemplate is the only templating engine in the Drupal core. This patch will not work with phptemplate until the maintainer of phptemplate engine incorporates favicon functionality into the phptemplate engine.
If the theme is installed under sites/sitename/themes then its favico will only show for the given site.
Yes, I understand this, but it's a real hassle to have to create a new theme directory just to facilitate the creation of a favicon. I feel strongly that a customized favicon is a common enough task that it should be built into the Drupal user interface and should be made as easy as possible. ------------------------------------------------------------------------ April 24, 2005 - 20:47 : nysus Attachment: http://drupal.org/files/issues/favicon3.patch (6.59 KB) Here is a revision to the patch that incorporates some suggestions that have been made here. It also fixes a bug in the previous version of the patch.
Issue status update for http://drupal.org/node/20809 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: nysus Reported by: nysus Updated by: nysus Status: patch
Why does this have to be part of the theme at all? I think others mentioned on the mailing list discussion that favicons are site-specific, not theme-specific.
I was actually the one who brought this point up. However, upon further reflection about the suggestion, some users may want to change the icon to match the color scheme of the different themes. Plus, since this is what was requested, this is what I delivered.
And don't forget: favicons can be any image type these days. They won't necessarily by .ico files.
This is true, at least for FF and maybe for recent verision of IE, too. However, users may still try to upload .ico files. And, right now, the PHP function called to process images for the logo file will not work for .ico files. But, I'm not sure what your point is, here. This patch will work with any type of image file, including .ico. nysus Previous comments: ------------------------------------------------------------------------ April 18, 2005 - 19:42 : nysus Attachment: http://drupal.org/files/issues/favicon.patch (4.57 KB) Here is a patch that allows a user to manually set the path to the "favicon.ico" file. It makes a single change to the user interface: it adds a text field to the "Logo image setting" section of the "themes/settings" page. This is especially useful for sites sharing a single code base as you no longer need to create a new theme just so you can have a distinct favicon.ico. ------------------------------------------------------------------------ April 23, 2005 - 00:28 : Steven There is a lot of mailing list discussion about this patch. One more note though: the <link /> tags are missing the trailing slash for XHTML. ------------------------------------------------------------------------ April 23, 2005 - 11:06 : nysus Attachment: http://drupal.org/files/issues/favicon2.patch (6.01 KB) Here is a revision to the patch based on the feedback received on the discussion list. You can now override the default favicon the same you way you can override the default site logo. There is one difference: When a logo is uploaded, it is checked to determine whether it is a valid image file using PHP's getimagesize function. Since this function does not work with .ico files, no validation of the uploaded file is performed. Advice on how (or if) validation needs to be done---or any other suggestions and corrections---is welcome. ------------------------------------------------------------------------ April 23, 2005 - 13:45 : Steven There is still a slash missing in bluemarine. This patch appears to have per-theme favicons, but the setting to control this is in fact defined by the theme engine, not the template (through the list of support theme features). I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory. But I'm not sure if we really need this: it does make it very obvious to people editing a theme that there is an icon file they can change. On the other hand, few people will use a theme's default favicon. It's more useful as a customization, and the UI option is good enough for that. Some text suggestions: "favorite icon" is not clear. I think the phrase "shortcut icon" is better (that's the standard name) if we accompany it with more text. Something like: "Check this box to use the default shortcut icon or 'favicon'. This icon is displayed in the address bar and bookmarks in most browsers." "The path to the icon file you would like to use as your custom shortcut icon." (note: i replaced "image file" with "icon file" because for IE, favicons need to be .ico files) 'Shortcut icon settings' ------------------------------------------------------------------------ April 23, 2005 - 14:07 : nysus
I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory.
This wouldn't work very well for sites sharing the same code base. Using your suggestion, you would have to create a custom theme just to have a customized favorite icon.
But I'm not sure if we really need this...
Can you please clarify to what "this" refers to? It's unclear to me what your suggestion is. I'll incorporate your other suggestions and corrections into the next revision. ------------------------------------------------------------------------ April 23, 2005 - 14:23 : kbahey The patch as it stands is only for xtemplate. What about phptemplate? Will it work with phptemplate? +1 on the concept though. I like a more generic solution, like what you proposed, that allows the favico to be added from an admin interface just like the logo. As for the comment:
I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory.
This wouldn't work very well for sites sharing the same code base. Using your suggestion, you would have to create a custom theme just to have a customized favorite icon.
If the theme is installed under sites/sitename/themes then its favico will only show for the given site. ------------------------------------------------------------------------ April 23, 2005 - 14:41 : Steven What I meant was to have the favicon default to themes/themename/favicon.ico if it exists, and use favicon.ico otherwise. I would still allow you to override the favicon regardless over the admin interface. But I'm not sure if default per-theme icons are useful anymore (that's what I meant with 'this'). ------------------------------------------------------------------------ April 23, 2005 - 14:52 : nysus
The patch as it stands is only for xtemplate. What about phptemplate? Will it work with phptemplate?
xtemplate is the only templating engine in the Drupal core. This patch will not work with phptemplate until the maintainer of phptemplate engine incorporates favicon functionality into the phptemplate engine.
If the theme is installed under sites/sitename/themes then its favico will only show for the given site.
Yes, I understand this, but it's a real hassle to have to create a new theme directory just to facilitate the creation of a favicon. I feel strongly that a customized favicon is a common enough task that it should be built into the Drupal user interface and should be made as easy as possible. ------------------------------------------------------------------------ April 24, 2005 - 15:47 : nysus Attachment: http://drupal.org/files/issues/favicon3.patch (6.59 KB) Here is a revision to the patch that incorporates some suggestions that have been made here. It also fixes a bug in the previous version of the patch. ------------------------------------------------------------------------ April 25, 2005 - 18:04 : Junyor Why does this have to be part of the theme at all? I think others mentioned on the mailing list discussion that favicons are site-specific, not theme-specific. Thus, they are candidates for inclusion in drupal/sites/ rather than as part of a theme. Then, the LINK element can be included with drupal_set_head (or whatever it's called). I like having the setting in the admin settings, similar to the site logo setting. And don't forget: favicons can be any image type these days. They won't necessarily by .ico files. So, +1 for better customization of favicons and -1 for the implementation.
Issue status update for http://drupal.org/node/20809 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: nysus Reported by: nysus Updated by: Steven Status: patch I committed a modified patch to HEAD: - The favicon should be implemented as a Theme Engine option. PHPTemplate already picked up shortcut icons if there was a favicon.ico in the theme directory. - The favicon should use drupal_set_html_head() for adding itself. That way, it can be implemented like a proper theme engine feature which can be enabled and disabled. Now it should do all the things mentioned before: per theme, user-configurable, optional favicons. I also updated the contrib theme engines for this. Steven Previous comments: ------------------------------------------------------------------------ April 19, 2005 - 01:42 : nysus Attachment: http://drupal.org/files/issues/favicon.patch (4.57 KB) Here is a patch that allows a user to manually set the path to the "favicon.ico" file. It makes a single change to the user interface: it adds a text field to the "Logo image setting" section of the "themes/settings" page. This is especially useful for sites sharing a single code base as you no longer need to create a new theme just so you can have a distinct favicon.ico. ------------------------------------------------------------------------ April 23, 2005 - 06:28 : Steven There is a lot of mailing list discussion about this patch. One more note though: the <link /> tags are missing the trailing slash for XHTML. ------------------------------------------------------------------------ April 23, 2005 - 17:06 : nysus Attachment: http://drupal.org/files/issues/favicon2.patch (6.01 KB) Here is a revision to the patch based on the feedback received on the discussion list. You can now override the default favicon the same you way you can override the default site logo. There is one difference: When a logo is uploaded, it is checked to determine whether it is a valid image file using PHP's getimagesize function. Since this function does not work with .ico files, no validation of the uploaded file is performed. Advice on how (or if) validation needs to be done---or any other suggestions and corrections---is welcome. ------------------------------------------------------------------------ April 23, 2005 - 19:45 : Steven There is still a slash missing in bluemarine. This patch appears to have per-theme favicons, but the setting to control this is in fact defined by the theme engine, not the template (through the list of support theme features). I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory. But I'm not sure if we really need this: it does make it very obvious to people editing a theme that there is an icon file they can change. On the other hand, few people will use a theme's default favicon. It's more useful as a customization, and the UI option is good enough for that. Some text suggestions: "favorite icon" is not clear. I think the phrase "shortcut icon" is better (that's the standard name) if we accompany it with more text. Something like: "Check this box to use the default shortcut icon or 'favicon'. This icon is displayed in the address bar and bookmarks in most browsers." "The path to the icon file you would like to use as your custom shortcut icon." (note: i replaced "image file" with "icon file" because for IE, favicons need to be .ico files) 'Shortcut icon settings' ------------------------------------------------------------------------ April 23, 2005 - 20:07 : nysus
I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory.
This wouldn't work very well for sites sharing the same code base. Using your suggestion, you would have to create a custom theme just to have a customized favorite icon.
But I'm not sure if we really need this...
Can you please clarify to what "this" refers to? It's unclear to me what your suggestion is. I'll incorporate your other suggestions and corrections into the next revision. ------------------------------------------------------------------------ April 23, 2005 - 20:23 : kbahey The patch as it stands is only for xtemplate. What about phptemplate? Will it work with phptemplate? +1 on the concept though. I like a more generic solution, like what you proposed, that allows the favico to be added from an admin interface just like the logo. As for the comment:
I was more thinking of simply checking for the presence of a favicon.ico in the theme's own directory.
This wouldn't work very well for sites sharing the same code base. Using your suggestion, you would have to create a custom theme just to have a customized favorite icon.
If the theme is installed under sites/sitename/themes then its favico will only show for the given site. ------------------------------------------------------------------------ April 23, 2005 - 20:41 : Steven What I meant was to have the favicon default to themes/themename/favicon.ico if it exists, and use favicon.ico otherwise. I would still allow you to override the favicon regardless over the admin interface. But I'm not sure if default per-theme icons are useful anymore (that's what I meant with 'this'). ------------------------------------------------------------------------ April 23, 2005 - 20:52 : nysus
The patch as it stands is only for xtemplate. What about phptemplate? Will it work with phptemplate?
xtemplate is the only templating engine in the Drupal core. This patch will not work with phptemplate until the maintainer of phptemplate engine incorporates favicon functionality into the phptemplate engine.
If the theme is installed under sites/sitename/themes then its favico will only show for the given site.
Yes, I understand this, but it's a real hassle to have to create a new theme directory just to facilitate the creation of a favicon. I feel strongly that a customized favicon is a common enough task that it should be built into the Drupal user interface and should be made as easy as possible. ------------------------------------------------------------------------ April 24, 2005 - 21:47 : nysus Attachment: http://drupal.org/files/issues/favicon3.patch (6.59 KB) Here is a revision to the patch that incorporates some suggestions that have been made here. It also fixes a bug in the previous version of the patch. ------------------------------------------------------------------------ April 26, 2005 - 00:04 : Junyor Why does this have to be part of the theme at all? I think others mentioned on the mailing list discussion that favicons are site-specific, not theme-specific. Thus, they are candidates for inclusion in drupal/sites/ rather than as part of a theme. Then, the LINK element can be included with drupal_set_head (or whatever it's called). I like having the setting in the admin settings, similar to the site logo setting. And don't forget: favicons can be any image type these days. They won't necessarily by .ico files. So, +1 for better customization of favicons and -1 for the implementation. ------------------------------------------------------------------------ April 26, 2005 - 00:19 : nysus
Why does this have to be part of the theme at all? I think others mentioned on the mailing list discussion that favicons are site-specific, not theme-specific.
I was actually the one who brought this point up. However, upon further reflection about the suggestion, some users may want to change the icon to match the color scheme of the different themes. Plus, since this is what was requested, this is what I delivered.
And don't forget: favicons can be any image type these days. They won't necessarily by .ico files.
This is true, at least for FF and maybe for recent verision of IE, too. However, users may still try to upload .ico files. And, right now, the PHP function called to process images for the logo file will not work for .ico files. But, I'm not sure what your point is, here. This patch will work with any type of image file, including .ico.
participants (9)
-
Chris Messina -
Earl Dunovant -
Junyor -
kbahey
-
nysus
-
Steve Dondley -
Steven
-
Steven Wittens -
Tim Altman