On Tue, 28 Apr 2009 16:27:00 +0100
Daniel Silverstone <[email protected]> wrote:
> Index: render/favicon.c
> ===================================================================
> --- /dev/null 2008-11-24 10:09:14.252162000 +0000
> +++ render/favicon.c 2009-04-28 16:01:49.412080078 +0100
> +static char *html_get_icon_ref(struct content *c, xmlNode *html);
> +static void html_favicon_callback(content_msg msg, struct content
> *icon,
> + intptr_t p1, intptr_t p2, union content_msg_data
> data); +static unsigned long hash(char *str);
These functions should start with favicon_ (see NetSurf source code
standards document.)
> +unsigned long hash(char *str)
> +{
> + if (str == NULL)
> + return 0;
> + unsigned long hash = 5381;
> + int c;
> + while ((c = (unsigned char) *str++))
> + hash = ((hash << 5) + hash) + c; /* hash * 33 + c */
> + return hash;
> +}
Do we need another hash function in NetSurf? I've lost count of them!
Are all the others static to their modules?
> +/**
> + * retrieve 1 url reference to 1 favicon
> + * \param html xml node of html element
> + * \return pointer to url; NULL for no icon
> + */
> +char *html_get_icon_ref(struct content *c, xmlNode *html)
> +{
> + xmlNode *node;
> + char *rel, *type, *href, *url, *suf, *url2;
> + url2 = NULL;
> + url_func_result res;
> + int score, hiscore;
> + hiscore = 0;
> + /* hashed values - saves calculating them afresh every time
> */
> + #define HHICON 0x7c98572e
> + /* icon */
> + #define HHSHORTCUTICON 0xfcbccdca
> + /* shortcut icon */
> + #define HHAPPLETOUCHICON 0x024c6ddd
> + /* apple-touch-icon */
> + #define HHIMAGEPNG 0x7382417c
> + /* image/png */
> + #define HHIMAGEGIF 0x73821a8d
> + /* image/gif */
> + #define HHIMAGEVNDMICROSOFTICON 0xdae02bba
> + /* image.vnd.microsoft.icon */
> + #define HHIMAGEJPEG 0xe3c72f5d
> + /* image/jpeg */
> + #define HHIMAGEJPG 0x73822838
> + /* image/jpg */
> + #define HHIMAGEICO 0x73822252
> + /* image/ico */
> + #define HHIMAGEICON 0xe3c66d00
> + /* image/icon */
> + #define HHIMAGEXICON 0x0e3e78e5
> + /* image/x-icon */
> + #define HHTEXTICO 0x17e966a2
> + /* text/icon */
> + #define HHAPPLICATIONICO 0x087b6fb4
> + /*application/icon*/
> + #define HHSUFICO 0x0b887ec0
> + /* ico */
> + #define HHSUFPNG 0x0b889dea
> + /* png */
> + #define HHSUFGIF 0x0b8876fb
> + /* gif */
> + #define HHSUFJPG 0x0b888486
> + /* jpg */
> + #define HHSUFJPEG 0x7c99198b
> + /* jpeg */
I think a typedefed enum would be better for these.
> + if ((href = (char *) xmlGetProp(node,
> + (const xmlChar *) "href"))
> == NULL)
> + continue;
> + suf = strrchr(href, '.');
> + if (suf != NULL) {
> + suf ++;
> + switch(hash(suf)) {
> + case HHSUFICO:
> + score += 10;
> + break;
> + case HHSUFPNG:
> + score += 10;
> + break;
> + case HHSUFGIF:
> + score += 10;
> + break;
> + case HHSUFJPG:
> + score += 7;
> + break;
> + case HHSUFJPEG:
> + score += 7;
> + break;
> + }
> + }
Some of this parsing and scoring code would be nice to have commented;
what is being parsed and how/why, and what the logic is for the scoring.
> +/**
> + * Callback for fetchcache() for linked favicon
> + */
> +
> +void html_favicon_callback(content_msg msg, struct content *icon,
> + intptr_t p1, intptr_t p2, union content_msg_data
> data) +{
> + struct content *c = (struct content *) p1;
> + unsigned int i = p2;
> +
> + switch (msg) {
> + case CONTENT_MSG_LOADING:
Incorrectly-nested case.
> + /* check that the favicon is really a
> correct image
> + * type */
> +
> + if ((icon->type == CONTENT_ICO) ||
> + (icon->type == CONTENT_PNG)
> ||
> + (icon->type == CONTENT_GIF))
> {
> +
> + /* may need call to content_open() */
You may? Why might you? :)
> Index: render/favicon.h
> ===================================================================
> --- /dev/null 2008-11-24 10:09:14.252162000 +0000
> +++ render/favicon.h 2009-04-28 16:01:49.412080078 +0100
> @@ -0,0 +1,25 @@
> +/*
> + * Copyright 2007 James Bursa <[email protected]>
What part of this file is copyrighted to James?
I'll do the changed files once I've beaten my client to stop it
insanely wrapping the diff.
B.
