Author: vince Date: Sun Sep 26 18:27:50 2010 New Revision: 10849 URL: http://source.netsurf-browser.org?rev=10849&view=rev Log: most of the core review done Modified: branches/jmb/treeview-redux/desktop/cookies.c branches/jmb/treeview-redux/desktop/tree.c branches/jmb/treeview-redux/desktop/tree.h branches/jmb/treeview-redux/desktop/tree_url_node.c Modified: branches/jmb/treeview-redux/desktop/cookies.c URL: http://source.netsurf-browser.org/branches/jmb/treeview-redux/desktop/coo... ============================================================================== --- branches/jmb/treeview-redux/desktop/cookies.c (original) +++ branches/jmb/treeview-redux/desktop/cookies.c Sun Sep 26 18:27:50 2010 @@ -140,36 +140,6 @@ return NODE_CALLBACK_HANDLED; } -/** - * Helper to update the text of a node if it has changed. - * - * \param element The node element to update. - * \param text The text to update the element with. The ownership of - * this string is taken by this function and must not be - * referred to after the function exits. - */ -static bool update_element_text(struct node_element *element, char *text) -{ - const char *node_text; /* existing node text */ - - if (text == NULL) - return false; - - if (element == NULL) { - free(text); - return false; - } - - node_text = tree_node_element_get_text(element); - - if ((node_text == NULL) || (strcmp(node_text, text) != 0)) { - tree_update_node_element(cookies_tree, element, text, NULL); - } else { - /* text does not need changing, free it */ - free(text); - } - return true; -} /** * Updates a tree entry for a cookie. @@ -191,7 +161,8 @@ /* update the value text */ element = tree_node_find_element(node, TREE_ELEMENT_VALUE, NULL); - update_element_text(element, + tree_update_element_text(cookies_tree, + element, messages_get_buff("TreeValue", data->value != NULL ? data->value : @@ -202,14 +173,16 @@ if ((data->comment != NULL) && (strcmp(data->comment, "") != 0)) { element = tree_node_find_element(node, TREE_ELEMENT_COMMENT, NULL); - update_element_text(element, + tree_update_element_text(cookies_tree, + element, messages_get_buff("TreeComment", data->comment)); } /* update domain text */ element = tree_node_find_element(node, TREE_ELEMENT_DOMAIN, element); - update_element_text(element, + tree_update_element_text(cookies_tree, + element, messages_get_buff("TreeDomain", data->domain, data->domain_from_set ? @@ -218,7 +191,8 @@ /* update path text */ element = tree_node_find_element(node, TREE_ELEMENT_PATH, element); - update_element_text(element, + tree_update_element_text(cookies_tree, + element, messages_get_buff("TreePath", data->path, data->path_from_set ? messages_get("TreeHeaders") : @@ -226,7 +200,8 @@ /* update expiry text */ element = tree_node_find_element(node, TREE_ELEMENT_EXPIRES, element); - update_element_text(element, + tree_update_element_text(cookies_tree, + element, messages_get_buff("TreeExpires", (data->expires > 0) ? (data->expires == 1) @@ -236,7 +211,8 @@ /* update last used text */ element = tree_node_find_element(node, TREE_ELEMENT_LAST_USED, element); - update_element_text(element, + tree_update_element_text(cookies_tree, + element, messages_get_buff("TreeLastUsed", (data->last_used > 0) ? ctime(&data->last_used) : @@ -244,7 +220,8 @@ /* update secure text */ element = tree_node_find_element(node, TREE_ELEMENT_SECURE, element); - update_element_text(element, + tree_update_element_text(cookies_tree, + element, messages_get_buff("TreeSecure", data->secure ? messages_get("Yes") : @@ -253,13 +230,15 @@ /* update version text */ element = tree_node_find_element(node, TREE_ELEMENT_VERSION, element); snprintf(buffer, sizeof(buffer), "TreeVersion%i", data->version); - update_element_text(element, + tree_update_element_text(cookies_tree, + element, messages_get_buff("TreeVersion", messages_get(buffer))); /* update persistant text */ element = tree_node_find_element(node, TREE_ELEMENT_PERSISTENT, element); - update_element_text(element, + tree_update_element_text(cookies_tree, + element, messages_get_buff("TreePersistent", data->no_destroy ? messages_get("Yes") : Modified: branches/jmb/treeview-redux/desktop/tree.c URL: http://source.netsurf-browser.org/branches/jmb/treeview-redux/desktop/tre... ============================================================================== --- branches/jmb/treeview-redux/desktop/tree.c (original) +++ branches/jmb/treeview-redux/desktop/tree.c Sun Sep 26 18:27:50 2010 @@ -76,49 +76,49 @@ struct tree; struct node_element_box { - int x; /* X offset from origin */ - int y; /* Y offset from origin */ - int width; /* Element width */ - int height; /* Element height */ + int x; /**< X offset from origin */ + int y; /**< Y offset from origin */ + int width; /**< Element width */ + int height; /**< Element height */ }; struct node_element { - struct node *parent; /* Parent node */ - node_element_type type; /* Element type */ - struct node_element_box box; /* Element bounding box */ - const char *text; /* Text for the element */ - void *bitmap; /* Bitmap for the element */ - struct node_element *next; /* Next node element */ - unsigned int flag; /* Client specified flag for data + struct node *parent; /**< Parent node */ + node_element_type type; /**< Element type */ + struct node_element_box box; /**< Element bounding box */ + const char *text; /**< Text for the element */ + void *bitmap; /**< Bitmap for the element */ + struct node_element *next; /**< Next node element */ + unsigned int flag; /**< Client specified flag for data being represented */ - bool editable; /* Whether the node text can be + bool editable; /**< Whether the node text can be * modified, editable text is deleted * without noticing the tree user */ }; struct node { - bool selected; /* Whether the node is selected */ - bool expanded; /* Whether the node is expanded */ - bool folder; /* Whether the node is a folder */ - bool retain_in_memory; /* Whether the node remains + bool selected; /**< Whether the node is selected */ + bool expanded; /**< Whether the node is expanded */ + bool folder; /**< Whether the node is a folder */ + bool retain_in_memory; /**< Whether the node remains in memory after deletion */ - bool deleted; /* Whether the node is currently + bool deleted; /**< Whether the node is currently deleted */ - bool processing; /* Internal flag used when moving */ - struct node_element_box box; /* Bounding box of all elements */ - struct node_element data; /* Data to display */ - struct node *parent; /* Parent entry (NULL for root) */ - struct node *child; /* First child */ - struct node *last_child; /* Last child */ - struct node *previous; /* Previous child of the parent */ - struct node *next; /* Next child of the parent */ - - /* Sorting function for the node (for folder nodes only) */ + bool processing; /**< Internal flag used when moving */ + struct node_element_box box; /**< Bounding box of all elements */ + struct node_element data; /**< Data to display */ + struct node *parent; /**< Parent entry (NULL for root) */ + struct node *child; /**< First child */ + struct node *last_child; /**< Last child */ + struct node *previous; /**< Previous child of the parent */ + struct node *next; /**< Next child of the parent */ + + /** Sorting function for the node (for folder nodes only) */ int (*sort) (struct node *, struct node *); - /* Gets called for each deleted node_element and on node launch */ + /** Gets called for each deleted node_element and on node launch */ tree_node_user_callback user_callback; - /* User data to be passed to delete_callback */ + /** User data to be passed to delete_callback */ void *callback_data; }; @@ -273,7 +273,7 @@ { struct node *node; - assert(title); + assert(title != NULL); node = calloc(sizeof(struct node), 1); if (node == NULL) { @@ -386,10 +386,10 @@ struct node *parent; bool sort = false; - assert(link); - assert(node); - - if ((!link->folder) || (before)) { + assert(link != NULL); + assert(node != NULL); + + if ((link->folder == 0) || (before)) { parent = node->parent = link->parent; if (parent->sort) sort = true; @@ -629,6 +629,38 @@ tree_update_node_element(tree, &(node->data), NULL, icon); } +/** + * Update the text of a node element if it has changed. + * + * \param element The node element to update. + * \param text The text to update the element with. The ownership of + * this string is taken by this function and must not be + * referred to after the function exits. + */ +bool tree_update_element_text(struct tree *tree, + struct node_element *element, char *text) +{ + const char *node_text; /* existing node text */ + + if (text == NULL) + return false; + + if (element == NULL) { + free(text); + return false; + } + + node_text = tree_node_element_get_text(element); + + if ((node_text == NULL) || (strcmp(node_text, text) != 0)) { + tree_update_node_element(tree, element, text, NULL); + } else { + /* text does not need changing, free it */ + free(text); + } + return true; +} + /** * Updates [all siblings and descendants of] a node to an expansion state. @@ -643,10 +675,7 @@ void tree_set_node_expanded(struct tree *tree, struct node *node, bool expanded, bool folder, bool leaf) { - bool change = tree_set_node_expanded_internal(tree, node, expanded, - folder, leaf); - - if (change) + if (tree_set_node_expanded_internal(tree, node, expanded, folder, leaf)) tree_handle_node_changed(tree, node, false, true); } @@ -781,7 +810,7 @@ child = node->child; node->child = NULL; - while (child) { + while (child != NULL) { tree_sort_insert(node, child); child = child->next; } @@ -808,7 +837,7 @@ /** - * Inserts a node into the correct place according to the parents sort function + * Inserts a node into the correct place according to the parent's sort function * * \param parent the node whose child node 'node' becomes * \param node the node to be inserted @@ -822,7 +851,8 @@ assert(parent->sort != NULL); after = parent->last_child; - while (after && parent->sort(node, after) == -1) + while ((after != NULL) && + (parent->sort(node, after) == -1)) after = after->previous; if (after != NULL) { @@ -831,12 +861,12 @@ node->next = after->next; node->previous = after; after->next = node; - } - else { + } else { node->previous = NULL; node->next = parent->child; - if (parent->child) + if (parent->child != NULL) { parent->child->previous = node; + } parent->child = node; } @@ -919,11 +949,10 @@ void tree_update_node_element(struct tree *tree, struct node_element *element, const char *text, void *bitmap) { - node_callback_resp response; struct node_msg_data msg_data; - assert(element); + assert(element != NULL); if (tree != NULL && element == tree->editing) tree_stop_edit(tree, false); @@ -996,14 +1025,14 @@ /** - * Returns true if the tree is currently being edited + * Returns whether the current tree is being edited at this time * * \param tree the tree to be checked * \return true if the tree is currently being edited */ bool tree_is_edited(struct tree *tree) { - return tree->editing == NULL ? false:true; + return tree->editing == NULL ? false : true; } @@ -1534,10 +1563,10 @@ false); if (mouse & (BROWSER_MOUSE_DRAG_1 | BROWSER_MOUSE_DRAG_2)) { - /* TODO: the tree window has to scroll the tree when - mouse reaches border while dragging this isn't - solved for the browser window too. - */ + /** @todo the tree window has to scroll the tree when + * mouse reaches border while dragging this isn't + * solved for the browser window too. + */ tree->drag = TREE_SELECT_DRAG; } return true; @@ -1762,7 +1791,7 @@ { int width, height, tree_height; - assert(node); + assert(node != NULL); width = node->box.width; height = node->box.height; @@ -1900,7 +1929,7 @@ struct node_element *element; int width, height; - assert(node); + assert(node != NULL); width = node->box.width; height = node->box.height; @@ -2522,8 +2551,9 @@ const char *icon_url = NULL; int len; hlcache_handle *c; - - /* TODO: something like bitmap_from_disc is needed here */ + nserror err; + + /** @todo something like bitmap_from_disc is needed here */ if (!strncmp(name, "file://", 7)) { icon_url = name; @@ -2555,12 +2585,17 @@ } /* Fetch the icon */ - hlcache_handle_retrieve(icon_url, 0, 0, 0, + err = hlcache_handle_retrieve(icon_url, 0, 0, 0, tree_icon_callback, 0, 0, 0, &c); + /* If we built the URL here, free it */ if (url != NULL) free(url); + + if (err != NSERROR_OK) { + return NULL; + } return c; } Modified: branches/jmb/treeview-redux/desktop/tree.h URL: http://source.netsurf-browser.org/branches/jmb/treeview-redux/desktop/tre... ============================================================================== --- branches/jmb/treeview-redux/desktop/tree.h (original) +++ branches/jmb/treeview-redux/desktop/tree.h Sun Sep 26 18:27:50 2010 @@ -149,6 +149,7 @@ bool tree_node_is_folder(struct node *node); void tree_update_node_element(struct tree *tree, struct node_element *element, const char *text, void *bitmap); +bool tree_update_element_text(struct tree *tree, struct node_element *element, char *text); const char *tree_node_element_get_text(struct node_element *element); struct node *tree_get_root(struct tree *tree); bool tree_is_edited(struct tree *tree); Modified: branches/jmb/treeview-redux/desktop/tree_url_node.c URL: http://source.netsurf-browser.org/branches/jmb/treeview-redux/desktop/tre... ============================================================================== --- branches/jmb/treeview-redux/desktop/tree_url_node.c (original) +++ branches/jmb/treeview-redux/desktop/tree_url_node.c Sun Sep 26 18:27:50 2010 @@ -231,37 +231,6 @@ return node; } -/** - * Helper to update the text of a node if it has changed. - * - * \param element The node element to update. - * \param text The text to update the element with. The ownership of - * this string is taken by this function and must not be - * referred to after the function exits. - */ -static bool update_element_text(struct tree *tree, - struct node_element *element, char *text) -{ - const char *node_text; /* existing node text */ - - if (text == NULL) - return false; - - if (element == NULL) { - free(text); - return false; - } - - node_text = tree_node_element_get_text(element); - - if ((node_text == NULL) || (strcmp(node_text, text) != 0)) { - tree_update_node_element(tree, element, text, NULL); - } else { - /* text does not need changing, free it */ - free(text); - } - return true; -} /** * Updates the node details for a URL node. @@ -321,19 +290,19 @@ /* update last visit text */ element = tree_node_find_element(node, TREE_ELEMENT_LAST_VISIT, element); - update_element_text(tree, - element, - messages_get_buff("TreeLast", - (data->last_visit > 0) ? - ctime((time_t *)&data->last_visit) : - messages_get("TreeUnknown"))); + tree_update_element_text(tree, + element, + messages_get_buff("TreeLast", + (data->last_visit > 0) ? + ctime((time_t *)&data->last_visit) : + messages_get("TreeUnknown"))); /* update number of visits text */ element = tree_node_find_element(node, TREE_ELEMENT_VISITS, element); - update_element_text(tree, - element, - messages_get_buff("TreeVisits", data->visits)); + tree_update_element_text(tree, + element, + messages_get_buff("TreeVisits", data->visits)); /* update thumbnail */ @@ -495,15 +464,17 @@ */ static xmlNode *tree_url_find_xml_element(xmlNode *node, const char *name) { - xmlNode *n; - if (!node) - return 0; - for (n = node->children; - n && !(n->type == XML_ELEMENT_NODE && - strcmp((const char *) n->name, name) == 0); - n = n->next) + xmlNode *xmlnode; + if (node == NULL) + return NULL; + + for (xmlnode = node->children; + xmlnode && !(xmlnode->type == XML_ELEMENT_NODE && + strcmp((const char *) xmlnode->name, name) == 0); + xmlnode = xmlnode->next) ; - return n; + + return xmlnode; } /** @@ -513,26 +484,26 @@ * \param directory directory to add this entry to */ static void tree_url_load_entry(xmlNode *li, struct tree *tree, - struct node *directory, tree_node_user_callback callback, - void *callback_data) + struct node *directory, tree_node_user_callback callback, + void *callback_data) { char *url = NULL, *url1 = NULL; char *title = NULL; struct node *entry; - xmlNode *n; + xmlNode *xmlnode; const struct url_data *data; url_func_result res; - for (n = li->children; n; n = n->next) { + for (xmlnode = li->children; xmlnode; xmlnode = xmlnode->next) { /* The li must contain an "a" element */ - if (n->type == XML_ELEMENT_NODE && - strcmp((const char *) n->name, "a") == 0) { - url1 = (char *) xmlGetProp(n, (const xmlChar *) "href"); - title = (char *) xmlNodeGetContent(n); - } - } - - if (!url1 || !title) { + if (xmlnode->type == XML_ELEMENT_NODE && + strcmp((const char *)xmlnode->name, "a") == 0) { + url1 = (char *)xmlGetProp(xmlnode, (const xmlChar *) "href"); + title = (char *)xmlNodeGetContent(xmlnode); + } + } + + if ((url1 == NULL) || (title == NULL)) { warn_user("TreeLoadError", "(Missing <a> in <li> or " "memory exhausted.)"); return; @@ -583,9 +554,9 @@ if (entry == NULL) { /** \todo why isn't this fatal? */ warn_user("NoMemory", 0); - } - else + } else { tree_update_URL_node(tree, entry, url, data, false); + } xmlFree(title); @@ -599,43 +570,44 @@ * \param directory directory to add this directory to */ static void tree_url_load_directory(xmlNode *ul, struct tree *tree, - struct node *directory, tree_node_user_callback callback, - void *callback_data) + struct node *directory, tree_node_user_callback callback, + void *callback_data) { char *title; struct node *dir; - xmlNode *n; - - assert(ul); - assert(directory); - - for (n = ul->children; n; n = n->next) { + xmlNode *xmlnode; + + assert(ul != NULL); + assert(directory != NULL); + + for (xmlnode = ul->children; xmlnode; xmlnode = xmlnode->next) { /* The ul may contain entries as a li, or directories as * an h4 followed by a ul. Non-element nodes may be present * (eg. text, comments), and are ignored. */ - if (n->type != XML_ELEMENT_NODE) + if (xmlnode->type != XML_ELEMENT_NODE) continue; - if (strcmp((const char *) n->name, "li") == 0) { + if (strcmp((const char *)xmlnode->name, "li") == 0) { /* entry */ - tree_url_load_entry(n, tree, directory, callback, + tree_url_load_entry(xmlnode, tree, directory, callback, callback_data); - } else if (strcmp((const char *) n->name, "h4") == 0) { + } else if (strcmp((const char *)xmlnode->name, "h4") == 0) { /* directory */ - title = (char *) xmlNodeGetContent(n); + title = (char *) xmlNodeGetContent(xmlnode ); if (!title) { warn_user("TreeLoadError", "(Empty <h4> " "or memory exhausted.)"); return; } - for (n = n->next; - n && n->type != XML_ELEMENT_NODE; - n = n->next) + for (xmlnode = xmlnode->next; + xmlnode && xmlnode->type != XML_ELEMENT_NODE; + xmlnode = xmlnode->next) ; - if (!n || strcmp((const char *) n->name, "ul") != 0) { + if ((xmlnode == NULL) || + strcmp((const char *)xmlnode->name, "ul") != 0) { /* next element isn't expected ul */ free(title); warn_user("TreeLoadError", "(Expected " @@ -657,7 +629,7 @@ if (folder_icon != NULL) tree_set_node_icon(tree, dir, folder_icon); - tree_url_load_directory(n, tree, dir, callback, + tree_url_load_directory(xmlnode, tree, dir, callback, callback_data); } } @@ -728,7 +700,7 @@ const char *text; li = xmlNewChild(node, NULL, (const xmlChar *) "li", NULL); - if (!li) + if (li == NULL) return false; @@ -737,14 +709,15 @@ return false; a = xmlNewTextChild(li, NULL, (const xmlChar *) "a", (const xmlChar *) text); - if (!a) + if (a == NULL) return false; text = tree_url_node_get_url(entry); if (text == NULL) return false; + href = xmlNewProp(a, (const xmlChar *) "href", (const xmlChar *) text); - if (!href) + if (href == NULL) return false; return true; } @@ -762,8 +735,8 @@ xmlNode *ul, *h4; const char *text; - ul = xmlNewChild(node, NULL, (const xmlChar *) "ul", NULL); - if (!ul) + ul = xmlNewChild(node, NULL, (const xmlChar *)"ul", NULL); + if (ul == NULL) return false; for (child = tree_node_get_child(directory); child; @@ -783,7 +756,7 @@ h4 = xmlNewTextChild(ul, NULL, (const xmlChar *) "h4", (const xmlChar *) text); - if (!h4) + if (h4 == NULL) return false; if (!tree_url_save_directory(child, ul)) @@ -814,7 +787,8 @@ xmlNode *html, *head, *title, *body; /* Unfortunately the Browse Hotlist format is invalid HTML, - * so this is a lie. */ + * so this is a lie. + */ doc = htmlNewDoc( (const xmlChar *) "http://www.w3.org/TR/html4/strict.dtd", (const xmlChar *) "-//W3C//DTD HTML 4.01//EN");