February 2005 - Development - lists.drupal.org

[drupal-devel] cvs log messages
by no-reply＠zind.net 07 Feb '05

07 Feb '05

File: /modules/spam/spam.module 1.41 Date: February 7, 2005 - 05:02 User: jeremy uncomment 'bypass spam filter' (commented out for debugging) ---------------------------------------------------------------------- File: /modules/spam/CHANGELOG 1.38 /modules/spam/spam.module 1.40 Date: February 7, 2005 - 05:01 User: jeremy o fix _spam_rating function to properly look at most interesting words (it had been looking at the _least_ interesting words / this could have a dramatic affect on the Bayesian filter, need to watch closely) o cleanup spam_comment_actions and spam_node_actions o add new ops for _spam hook: - comment_spam - comment_notspam - content_spam - content_notspam ---------------------------------------------------------------------- File: /sandbox/crunchywelch/event/event.css 1.1 /sandbox/crunchywelch/event/event.module 1.1 /sandbox/crunchywelch/event/event.theme 1.1 Date: February 7, 2005 - 00:08 User: crunchywelch initial commit of civicspace event.module ---------------------------------------------------------------------- File: /sandbox/junyor/import/README 1.5 Date: February 6, 2005 - 23:45 User: junyor Updated with slightly more thorough instructions ---------------------------------------------------------------------- File: /sandbox/junyor/import/import.php 1.7 Date: February 6, 2005 - 23:45 User: junyor Fixed small problem with comment usernames ---------------------------------------------------------------------- File: /modules/taxonomy_block/taxonomy_block.mysql 1.1.2.2 DRUPAL-4-5 Date: February 6, 2005 - 20:53 User: crunchywelch fix escape characters ---------------------------------------------------------------------- File: /modules/taxonomy_block/taxonomy_block.mysql 1.2 Date: February 6, 2005 - 20:52 User: crunchywelch fix escaped characters ---------------------------------------------------------------------- File: /modules/project/project.inc 1.46 /modules/project/project.module 1.181 Date: February 6, 2005 - 19:59 User: dries - Began integrating the project module and CVS module. ---------------------------------------------------------------------- File: /modules/event/po/da.po 1.1.2.1 DRUPAL-4-5 Date: February 6, 2005 - 19:50 User: wulff Initial release ---------------------------------------------------------------------- File: /modules/image/po/da.po 1.1.2.1 DRUPAL-4-5 Date: February 6, 2005 - 19:48 User: wulff Initial release ---------------------------------------------------------------------- File: /translations/da/modules/event.po 1.1.2.1 DRUPAL-4-5 /translations/da/modules/image.po 1.1.2.1 DRUPAL-4-5 Date: February 6, 2005 - 19:32 User: wulff Initial release ---------------------------------------------------------------------- File: /translations/da/README 1.1.2.2 DRUPAL-4-5 Date: February 6, 2005 - 19:31 User: wulff Added wordlist ---------------------------------------------------------------------- File: /modules/remindme/remindme.module 1.21.2.1 DRUPAL-4-5 Date: February 6, 2005 - 19:24 User: killes backport from head. ---------------------------------------------------------------------- File: /modules/flexinode/flexinode.module 1.45 Date: February 6, 2005 - 18:35 User: killes bugfix: SQL error on search page. ---------------------------------------------------------------------- File: /modules/flexinode/flexinode.module 1.36.2.4 DRUPAL-4-5 Date: February 6, 2005 - 18:20 User: killes bugfix: The search query generated invalid SQL. ---------------------------------------------------------------------- File: /modules/cvslog/cvs.module 1.44 /modules/cvslog/README.txt 1.3 Date: February 6, 2005 - 17:15 User: dries - Further integegration with user accounts. - Added a 'status'-page to check the consistency of the cvs module related SQL tables (useful during the transition period). - Improved input checking of integegrated 'request cvs account' form. ---------------------------------------------------------------------- File: /modules/spam/CHANGELOG 1.37 /modules/spam/spam.module 1.39 Date: February 6, 2005 - 16:55 User: jeremy - fix spam_check() to return TRUE or FALSE, and to only call the callback if defined ---------------------------------------------------------------------- File: /modules/spam/CHANGELOG 1.10.2.21 DRUPAL-4-5 /modules/spam/spam.module 1.9.2.26 DRUPAL-4-5 Date: February 6, 2005 - 16:48 User: jeremy sync with CVS version (new _spam hook for use by external modules) ---------------------------------------------------------------------- File: /modules/spam/spam.module 1.38 Date: February 6, 2005 - 16:37 User: jeremy uncomment "bypass spam filter" permission (debugging artifact) ---------------------------------------------------------------------- File: /modules/spam/CHANGELOG 1.36 /modules/spam/spam.module 1.37 Date: February 6, 2005 - 16:35 User: jeremy o replace spam_get_rating() and is_spam() with single spam_check() function o add support for _spam hook, allows other modules to add to spam settings page, and to begin to perform spam filtering for custom content types. supported hooks (see comments for spam_invoke_spam_hook): - filter_settings - url_count_settings - action_settings - url_count - check ---------------------------------------------------------------------- File: /modules/translation/README.en.txt 1.2.2.1 DRUPAL-4-5 /modules/translation/translation.module 1.3.2.3 DRUPAL-4-5 /modules/translation/translation.pgsql 1.1.2.1 DRUPAL-4-5 Date: February 6, 2005 - 15:27 User: axel Fix #16835 (patch by Jarno Elonen), fix of settings screen, begin work on administrative screen. ---------------------------------------------------------------------- File: /modules/project/comment.inc 1.41 /modules/project/issue.inc 1.90 /modules/project/project.module 1.180 Date: February 6, 2005 - 12:32 User: dries - Fixed PHP5 bugs in the project module. - Fixed RSS feed of issues and removed dead code. ---------------------------------------------------------------------- File: /modules/project/issue.inc 1.89 Date: February 6, 2005 - 12:17 User: dries - Fixed the channel.language attribute in RSS feeds. ---------------------------------------------------------------------- File: /modules/project/release.inc 1.44 Date: February 6, 2005 - 12:05 User: dries - Fixed broken drupal_goto()s (bug #6330). ---------------------------------------------------------------------- File: /modules/project/project.module 1.179 Date: February 6, 2005 - 11:40 User: dries - Removed dead code (access permissions are checked at the _menu level). ---------------------------------------------------------------------- File: /modules/book.module 1.284 Date: February 6, 2005 - 09:37 User: dries - Patch #16462 by Moshe: book pages slighty disobey the node_access system because they always return a value for book_access('update'). This simple patch causes us to use node_access when the user has a chance of being able to update the current page. ---------------------------------------------------------------------- File: /modules/spam/CHANGELOG 1.35 Date: February 6, 2005 - 06:51 User: jeremy replace spam_get_rating() and is_spam() with single spam_check() function ---------------------------------------------------------------------- File: /modules/spam/CHANGELOG 1.34 /modules/spam/spam.module 1.36 Date: February 6, 2005 - 05:45 User: jeremy - spam.module o fix url in notification mail messages to point to url for editing node or comment (an adding $base_url to comment url) o split out some api logic, adding two functions that can be called from other modules, documented in code: - spam_get_rating() - is_spam() o add support for _spam_url_count hook, allowing modules to define how many urls are permitted for their content type ----------------------------------------------------------------------

1 0

[drupal-devel] [feature] Don't reset existing password on request, prevent DoS password reset abuse
by brlcad 07 Feb '05

07 Feb '05

Project: Drupal Version: 4.5.2 Component: user system Category: feature requests Priority: critical Assigned to: Anonymous Reported by: brlcad Updated by: brlcad Status: patch Attachment: http://drupal.org/files/issues/pass_alt.diff (6.12 KB) This patch adds a field to the user table for storing an alternate password. When a password request is made, the alternate password is set instead of clobbering the existing password. This allows the user to discard the regenerated password e-mail, preserving the existing password. This also prevents abuse whereby any anonymous user can repeatedly reset user passwords potentially entirely blocking access to a site, e.g. a curl shell script that repeatedly posts a password reset request for all accounts. Iff the alternate password is used for authentication, the alternate password becomes the main password. If the main password is set (e.g. admin user form), the alternate password is unset. The patch was made against DRUPAL-4-5-2. Cheers! Sean brlcad -- View: http://drupal.org/node/16909 Edit: http://drupal.org/project/comments/add/16909

2 2

[drupal-devel] [bug] Signature "interrupts" typing comments due to existence in text field
by andremolnar 06 Feb '05

06 Feb '05

Project: Drupal Version: cvs Component: comment.module Category: bug reports Priority: minor Assigned to: Anonymous Reported by: irwin Updated by: andremolnar Status: patch "Another option (hehe) would be to make two tiny modules with signature-code in them, and remove all the signature code from user and comment.module. " +1 to any approach that removes code from user.module. I've realized lately that user.module does too many things that might be better done as seperate modules that extend the core user module. In other words anything that makes user.module more modular is a great idea! andre andremolnar Previous comments: ------------------------------------------------------------------------ February 4, 2005 - 04:47 : irwin In the Drupal comments/ signature system, there is a usability problem with the signature. The main problem is that, when adding a comment, the signature appears in the same line as the textarea that you are writing the content of your post. When you begin typing, your new text actually appears AFTER the signature (at least in Konqueror 3.2). This makes it incredibly annoying for users because you have to go to the beginning of the textarea before you can type your data. A corrolary to this problem is that, when quoting (using the quote module), the signature is included by default in the quote, which should not be the default behaviour. It appears that the signature is being added to the actual body text of the comment, as opposed to being retrieved from the user's profile when the page is rendered. -- >From an engineering/design perspective, one possible way to fix this is instead of adding the signature text to the comment, instead retrieve the signature dynamically from the user's profile when the page is rendered. If you do this, not only do you avoid the above problems, but you also improve the flexibility of signatures (for example, changing it once will cause the signature to change in every comment you make). You can also use this design change to give the users a choice of when to display their signature (for example, by using a checkbox on the node that says, "Display your signature with this post"). -- Thank you. ------------------------------------------------------------------------ February 4, 2005 - 05:49 : irwin Attachment: http://drupal.org/files/issues/signature_load_on_render-20050204.patch (1.62 KB) Included: patch. Changes explanation: "theme_comment_form": the code to autoinsert the signature into the textarea is removed. "theme_comment_view": code added to insert signature at the end of the $comment->comment data structure. I encase the comment in [p class="signature"] [/p] (with the correct brackets of course) so that a CSS theme can change the appearance of signatures. Caveat: the sig still appears in the [div class="body"] of the comment theme. Added function _comment_get_signature($uid), which gets a signature, given a user ID. Caveat: this requires an additional query to the "users" table with every user comment. Tested with: - User without signature - User with signature - Anonymous poster - Previewing - Positng - Editing a signature: expected result is that all signatures are changed to reflect the new signature. Please let me know of any problems. ------------------------------------------------------------------------ February 4, 2005 - 05:50 : irwin Reducing severity to "minor" because that's what it is. ------------------------------------------------------------------------ February 4, 2005 - 09:15 : Bart Jansens The extra queries aren't necessary, the queries that are used to retrieve the comments already join the users table, just add u.signature to the select clause there. I'm not sure if adding the signature to the comment body is the best approach, if you just keep it in a different field, themes can decide where to place it. Note that usually admins will want to be able to use a different filter for signatures. Eg, allow bbcode in the comment body, but not in the signature. I am using a similar patch on my own site (although it really needs to be updated), it never made it into core because it is difficult or almost impossible to create an upgrade path. I hope this can be discussed again, or at least made into an option for new sites, i'm surprised there haven't been more complaints about this ;) ------------------------------------------------------------------------ February 4, 2005 - 15:40 : irwin I realised as I was going to bed last night that the extra query was extraneous. :) As for adding the comment to the comment body, the reason I would want to do that is for the signature to "appear" to be a part of the... well, comment body. That will avoid potential issues with some themes that aren't designed to handle signatures at all - for instance, a theme might put a border around the "body" part of the signature, and if it doesn't explicitly handle the signature, things can look very ugly and strange. But I can see what you mean about Input Filtering. Do you have suggestions on how to improve that? I had one idea, and that was to add the signature as an argument to the theme, but that would require theme modifications fir signatures to be displayed. If this patch is of interest I'll keep working on it and posting updates. ------------------------------------------------------------------------ February 5, 2005 - 07:19 : Steven Have you considered that not everyone might want retroactive signatures? It depends on the preference of the site owner I guess. ------------------------------------------------------------------------ February 5, 2005 - 07:20 : Steven By the way, the comments are currently fetched with a single query. You could easily extend it to get the signature with a JOIN instead of doing a query per comment. ------------------------------------------------------------------------ February 5, 2005 - 10:19 : Bèr Kessels "It depends on the preference of the site owner I guess. " Please, no. Not a config optin for this. Choose the best default and optimise that. We must get /less/ options instead of more. Another option (hehe) would be to make two tiny modules with signature-code in them, and remove all the signature code from user and comment.module. one module would be: signature_static.module. That one would add signatures to nodes and comments like we do it now: concanate it to the $node->body on insert in the DB. signature_dynamic.module would concenate on output, and not store the data in the DB with every comment and node. Bèr -- View: http://drupal.org/node/16726 Edit: http://drupal.org/project/comments/add/16726

1 0

[drupal-devel] Speed of Drupal, memory usage, and all that stuff
by Gerhard Killesreiter 06 Feb '05

06 Feb '05

Hi there, those of you who have been following Drupal's development for some time will know that I've always been interested in making Drupal faster. Some other people have expressed interest in making Drupal use less memory, especially on the admin/modules page where currently all present modules are included. I've recently experimented a bit and came up with a solution that achieves both goals. You can look at it in the speed-drupal directory in my sandbox. There is also an issue with the title "Make Drupal faster" where I described my results. I reduced the amount of lines of code that gets included and parsed by PHP on each page call by moving most of a module's code into .inc files and only leaving small wrappers for the functions in the .module file. The result is fully functional and achieves both goals. I still don't like it much because having both a wrapper and a function in a .inc file isn't really nice. That is why I thought about another approach: On calling admin/modules we should not include the modules, but include a small .functions file. This file should (for each module) tell us which functions are defined in which file. By this approach it would be possible to split each file up in a number of small files and still let Drupal know where the functions are defined. The array of functionname->file associations will be stored and if a function is called through a wrapper such as module_invoke Drupal would be able to include the neccessary file and execute the function. This would of course require that module authors call all external functions through module_invoke. A second step of optimisation would be to define the operation of each hook that is implemented. For example a module might only implement the 'delete' operation of the _nodeapi hook. Then it would not be neccessary to include any file if the 'view' operation of that hook is invoked. This is very important for the _help hook which has a lot of code[1]. As a closing remark I need to state that the whole code parsing issue is of little concern if you run your own server and can install a PHP cache. Those caches keep the generated byte code in memory (either virtual or in files) and thus make all this effort pretty much unneccessary. Since I do now run my own server, I have little personal use for either of the outlined approaches and will only work on it if I see good chances to get it into Drupal 4.7. Discussion is welcome. Cheers, Gerhard [1] Actually I think we could get rid of the help hook for most help messages by using drupal_set_message with a help status (drupal_set_message('I am your helpfull message', 'help')).

3 5

RE: [drupal-devel] invite others
by Boerland Bert 06 Feb '05

06 Feb '05

-----Original Message----- From: drupal-devel-bounces(a)drupal.org > Should we invite a few other people on info(a)drupal.org to help respond > to e-mails? If so, any candidates? May I suggest an auto reply on info AT drupal.org with a remark that standard questions that can be found in the FAQ or forum will not be answered? I am willing to be on this list with an specific email account but there needs to be some kind of understanding on how who is answering what questions.

1 0

[drupal-devel] [feature] improve theme_box to make it better themeable and consistent with blocks
by Bèr Kessels 06 Feb '05

06 Feb '05

Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: Bèr Kessels Reported by: Bèr Kessels Updated by: Bèr Kessels Status: patch The reason why we would need IDS on each box, would be because of issues like Form groups should be collapsible [1]. so unless we introduce other theme_ or apis for such cases, the boxes will need more and better information. Besides that, with this patch you will be able to use: .box {} /*for any box*/ .meaningfull-class {} /*Developers can and should use classes from general lists*/ #modulename-idname {} /*if you want advanced power*/ But you are (1) not enfirced to use them, and (2) able to define general classes. This is still future, and cannot be enforced with code. But what we need is a meaningfull list of classes, a developer can choose his from. Examples would be: .left2col .right2col /* boxes that will show up as two columns in the main area*/ .important .top /* boxes used to explain what that page does, for example the aggregator top-box */ .help /* boxes that will render as helptext, for example search help */ etc. but again, thats for future, first we need the ability to "überhaupt" set these classes. Bèr [1] http://drupal.org/node/16204 Bèr Kessels Previous comments: ------------------------------------------------------------------------ January 8, 2005 - 22:37 : Bèr Kessels Attachment: http://drupal.org/files/issues/theme_box.patch (1.24 KB) This patch changes the theme_box function. Currently we lak the ability to style blocks different, since we cannot add classes or IDS. I fixed the theme_box to follow the system we use for sideblocks. The id and classes should be provided by arguments. I have only one problem: backwards compatibility. To output valid XHTML all calls of this function must add a unique ID, I documented how this must be done, thats not the problem. The problem, is that we do not do this atm. So untill all calls are fixed, I do: <?php theme_box($title, $content, $class = NULL, $id = NULL) ?> and not <?php theme_box($title, $content, $class, $id) ?> which would be correct, since only the later will force developers to use and ID, resulting in always validating XHMTML. I will try to generate a grep list with places where theme box is called and where it should be fixed. Bèr ------------------------------------------------------------------------ January 8, 2005 - 23:25 : tangent XHTML does not require a div to have an ID attribute. Why would an ID be required in this case? ------------------------------------------------------------------------ January 8, 2005 - 23:40 : Bèr Kessels XHTML does not require a div to have an ID attribute. Why would an ID be required in this case? Because i do not wat to add an if($id) { } or even an $id ? 'id= etc. That would be loads of ifs that are not needed: its much better to enforce everyone to add an ID. Also, if we make ids optional, a lot of lazy developers will not add it, resulting in a lot of brokenish themes. I perfer to have always an ID, and put tit in the coding guidelines, then to have only sometimes an id. And the last reason is, that with the current useability discussions going on, there are people starting to look at javascripts in their themes. We should be ahead of that, and simply give all elements an ID. So that themers can add their javascript to do fancy stuff to parts of the pages. (like hide and show them using cards and or accordeons) ------------------------------------------------------------------------ January 8, 2005 - 23:46 : tangent I see where you're going but I don't think that we should necessarily require *all* blocks to have an id. If a module developer doesn't want his block to have an id and prefers to have a set of blocks with the same or different classes I see nothing wrong with that. Requiring an ID seems a bit heavy handed. This is really another issue but how will user definable modules be handled? I assume the module would just use an incremented ID? ------------------------------------------------------------------------ January 9, 2005 - 02:10 : chrisada "if we make ids optional, a lot of lazy developers will not add it " As one who theme Drupal quite a lot, I am absolutely with Bèr on this point. The deveoper might not need the id to be there, but there will be peoples who want them. Right now there is just too many things in Drupal that is impossible to style due to lack of CSS id. (classes we have plenty) ------------------------------------------------------------------------ January 12, 2005 - 15:01 : Bèr Kessels I cannot see the issues with "requiring" an ID. Really: is that so hard to do? A module might have one, or two, sometimes three blocks. Why cant you add two letters to your code for a unique ID? Its all about consistency. I already know a huge lot of places in the code where the *block theme function iteself* should have been used, but is not. This is annoying, not for developers, they don't care. But for both users, who see inconsistent interfaces, and for themers who go like: Hey: I am sure I had all titles of all pages set to bleu, why is that one red? --aah. f*c, its an h3 that is not in a block, grrr. Same will go for blocks ids. I do not know why, but the general html-code and interfaces of contributions are plain bad, in drupal. Core is nice, but a lot of contributions are hacks-upon-hacks, to keep up with the changing drupal-core versions. We should enforce developers to at least keep consistancy. ------------------------------------------------------------------------ January 25, 2005 - 22:03 : Bèr Kessels Attachment: http://drupal.org/files/issues/theme_box_0.patch (1.22 KB) This one has all parameters required. ------------------------------------------------------------------------ January 25, 2005 - 22:53 : drumm The html outputting could be cleaned up a little, using single quotes where possible and not putting variables inside strings. Also, I think this sohuld be renamed to theme_page_section() and it should not necessarily actually draw a box. Whether something has a literal box or not should be up to the themer on a case-by-case basis. I see this themeable function as omething to break pages up into areas and not add boxes. Otherwise, I like where this is going and would like to see the code changed to call this properly. For now I won't give a whole +1, but it deserves at least +0.5. ------------------------------------------------------------------------ February 6, 2005 - 12:37 : adrinux Personally I find the 'every element with a class and id' style of drupal slightly annoying. It seems to me there is great emphasis on keeping the php code clean and effecient, but when it comes to xhtml a 'kitchen sink' include everything that a theme designer might possibly need and somewhat bloated approach prevails. To me it all adds clutter that makes designing drupal themes harder not easier, especially for newcomers faced with a huge array of classes and ID's and not knowing quite what to use, or where they come from. Documenting them helps but not a lot. That said I'm for anything that lets me over-ride xhtml and add my own in phptemplate :) I'm not conviced that enforced ID's on all boxes is a good idea... -- View: http://drupal.org/node/15332 Edit: http://drupal.org/project/comments/add/15332

1 0

[drupal-devel] invite others
by Dries Buytaert 06 Feb '05

06 Feb '05

Should we invite a few other people on info(a)drupal.org to help respond to e-mails? If so, any candidates? -- Dries Buytaert :: http://www.buytaert.net/

3 4

[drupal-devel] [task] Put revisions in their own table
by killes 06 Feb '05

06 Feb '05

Project: Drupal Version: cvs Component: node system Category: tasks Priority: normal Assigned to: killes(a)www.drop.org Reported by: killes(a)www.drop.org Updated by: killes(a)www.drop.org Status: patch Attachment: http://drupal.org/files/issues/revisions_16.patch (52.49 KB) Updated to apply to cvs again. killes(a)www.drop.org Previous comments: ------------------------------------------------------------------------ May 5, 2004 - 17:25 : killes(a)www.drop.org Currently all node revisions are stored in a serialized field in node.table and retrieved for _each_ page view although they are rarely needed. However, we have agreed that serializing data is bad and that we should try to keep the memory foot print pf Drupal small. Therefore I propose to create a separate revisions table which would be in principle identical to the node table, only that it could have several old copies of the same node. Extra data added by other modules could be added in a serialized field unless we find a better solution. ------------------------------------------------------------------------ May 5, 2004 - 18:06 : jhriggs I too think the serialized approach is less than desirable, but here's an alternative. This would likely take some considerable rework in core and contrib, but the following is how we handle similar types of situations in our databases at work. It is more elegant that a separate table, and avoids the (almost exact) duplication of a table. Instead of separate tables, keep all revisions of nodes in the node table as follows: * add field: active (0/1 or Y/N) * add field: revision * every revision of a node is stored in the node table; however, only one revision can be active at any given time * nid can no longer be unique -- primary/unique key becomes (nid, active) * any time a node is loaded, updated (without revision), etc., the active version is used. Thoughts? ------------------------------------------------------------------------ May 5, 2004 - 18:57 : killes(a)www.drop.org I am not opposed to your scheme, but I want to stress the following: * Duplicating a table's structure is not bad (IMHO) as long as the content is different. * having two tables will allow us to have a rather small node table. This is (maybe) a performance gain. ------------------------------------------------------------------------ May 5, 2004 - 19:37 : jhriggs I don't necessarily think that duplicating a table's structure is _bad_. It just seems to be wasteful and a pain to maintain. (Every change to the node table is made twice...easy to do, but also easy to miss perhaps.) As for performance, as long as nid and the active indicator are indexed, there shouldn't be any performance loss. Also, archiving an old version when making a new revision will be much simpler: just change the active indicator rather than copying an entire node to another table (and ensuring everything gets copied...again a potential maintainance issue). To be honest, I would just like to see the serialized data go away, regarless of what approach is taken. ------------------------------------------------------------------------ July 30, 2004 - 20:49 : Nick Nassar Attachment: http://drupal.org/files/issues/Drupal-Improved_Revision_Schema_07-30-2004.p… (10.47 KB) I'm interested in using Drupal for a large scale wiki-type project. In order to do this, I need revisions to be in their own table. Attached is a patch to do just that. Most of the changes are pretty self explanatory. Spreading out node data across two tables meant that I had to add database functions to do locking/transactions. Without this, race conditions in which the database becomes corrupted are possible. ------------------------------------------------------------------------ July 30, 2004 - 20:54 : Nick Nassar Oh yeah... The patch is a diff against Drupal CVS ------------------------------------------------------------------------ July 31, 2004 - 01:00 : Anonymous Gerhard speaking. Nick, thanks a lot for your nice patch! It saves me a great deal of labour. I looked through it and immediately liked it. You not only put the old revisions into a new table but also the current one. Do you have an estimate how much more expensive the additional join is? Besides a few minor coding style issues I found a major one: Just a few hours before you uploaded your patch JonBob's node access patch hit core. That means your patch won't apply anymore as all the queries you change have been changed. Can I bug you to update your patch? ------------------------------------------------------------------------ July 31, 2004 - 02:11 : Anonymous Also I think that your upgrade path loses existing revisions. ------------------------------------------------------------------------ July 31, 2004 - 03:39 : drumm I think this is the proper way to do things. No columns are duplicated, there is no serialized data, and only the fields that are logically revised are stored. Nothing jumped out at me as a way to have my node modules be able to keep a table of revisions of additional fields. I'm guessing this could be done within the confines of _insert and _update. Assuming the upgrade path works and modules can extend it I give it a +1. ------------------------------------------------------------------------ July 31, 2004 - 15:40 : Nick Nassar It figures that just as I finish a big patch, another patch comes along and breaks it. Oh well, it should be a pretty easy to fix. I'll work on it. Fixing the upgrade path to keep revisions should be fairly painless. I found another issue that needs to be fixed before this patch gets merged. There format of a node needs to be stored for each revision. Otherwise, for modules that store a format for the nodes, such as page and book, if you write one revision in PHP and the next in HTML, the PHP revision will be displayed as HTML. This is part of a larger issue of how node modules should store revisions of additional fields. I think each module that wants to do this should create another table with (nid, revid) as the primary key. Just as when they want to add fields to a node they create another table with nid as the primary key. As far as performance goes, for sites that make heavy use of revisions, an extra join on primary keys is going to be a lot faster than grabbing all of the revisions from that database everytime. We would need to run benchmarks to determine is the overall difference in speed is for an average site is a gain or a loss. I'm guessing it's very minor either way. ------------------------------------------------------------------------ August 23, 2004 - 14:55 : Nick Nassar Attachment: http://drupal.org/files/issues/Drupal-Improved_Revision_Schema_08-23-2004.p… (10.92 KB) Here's an updated patch against CVS that puts revisions in their own table, provides an upgrade path, and fixes the format related bugs in the last patch. Hopefully, this can make it into CVS as soon as the freeze is over. ------------------------------------------------------------------------ August 23, 2004 - 15:10 : moshe weitzman Interesting patch ... drumm's question is still outstanding. how do modules store revisions of their fields? Are they expected to manage this on their own? Thats not how it works today. As an aside, i am seeing profile_ fields in my node.revisions column. One could argue that those need not be saved. They pertain to the node author, not to the node itself. ------------------------------------------------------------------------ August 23, 2004 - 17:14 : Nick Nassar Having modules be responsible for storing revisions of their own fields is a side-effect of storing revision data in tables. There's really no way around it. However, revisions generally don't make sense for node types that don't have PHP/HTML content, such as polls. I think it's going to be a pretty rare scenario for a new node type to want another field to change per-revision, so it's a pretty good trade-off. Storing fields that shouldn't be part of revisions, such as the profile_ fields, is a side-effect of storing revisions as serialized objects. Applying this patch will free up that wasted space. :) ------------------------------------------------------------------------ August 23, 2004 - 18:20 : Anonymous There should be a hook that let's the module choose whether it supports history. This way a module author can prevent the user from doing something that may break his module or just cause undefined behavior. If the module doesn't support history then don't let the user/admin choose to add history to nodes of that type. Craig ------------------------------------------------------------------------ August 23, 2004 - 20:23 : Nick Nassar I agree, there should be an API change to make specifying support for revisions easier. In the interests of keeping patches small and keeping to one change per patch, I think the API change should be a separate issue. A sort of ad-hoc API to decide whether or not a module supports revisions by default already exists. Instead of having a hook, modules set the default value of the "Create new revision" field in the edit form. The admin can change this option in admin/node/configure/defaults. This patch doesn't change that. Revisions are broken for node types that have their own database structure, like polls, even when storing them as serialized objects. This patch doesn't change that, either. ------------------------------------------------------------------------ October 26, 2004 - 03:35 : moshe weitzman I'm guessing that someone is going to have to demonstrate that this patch performs as well as current drupal before it gets comitted. i think this patch is a few benchmarks from being comitted. ------------------------------------------------------------------------ October 27, 2004 - 02:04 : Nick Nassar Attachment: http://drupal.org/files/issues/Drupal-Improved_Revision_Schema_10-26-2004.p… (11 KB) I ran some really unscientific benchmarks, and it looks like this patch has a negligible affect on performance. I used apache bench and the database from theregular.org, which doesn't contain any revisions (worst case scenario for this patch) and contains several hundred nodes. Both the patched and unpatched versions hovered between 2.36 and 2.38 requests per second. The command I used was: ab -n50 -C 'PHPSESSID=b01a9f92880ef215b0ed6f1314a5eba2' http://192.168.0.100/ An updated patch that should apply to CVS is attached. ------------------------------------------------------------------------ October 27, 2004 - 02:05 : Nick Nassar I ran some really unscientific benchmarks, and it looks like this patch has a negligible affect on performance. I used apache bench and the database from theregular.org, which doesn't contain any revisions (worst case scenario for this patch) and contains several hundred nodes. Both the patched and unpatched versions hovered between 2.36 and 2.38 requests per second. The command I used was: ab -n50 -C 'PHPSESSID=b01a9f92880ef215b0ed6f1314a5eba2' http://192.168.0.100/ An updated patch that should apply to CVS is attached. ------------------------------------------------------------------------ October 27, 2004 - 02:05 : Nick Nassar Attachment: http://drupal.org/files/issues/Drupal-Improved_Revision_Schema_10-26-2004.p… (11 KB) I ran some really unscientific benchmarks, and it looks like this patch has a negligible affect on performance. I used apache bench and the database from theregular.org, which doesn't contain any revisions (worst case scenario for this patch) and contains several hundred nodes. Both the patched and unpatched versions hovered between 2.36 and 2.38 requests per second. The command I used was: ab -n50 -C 'PHPSESSID=b01a9f92880ef215b0ed6f1314a5eba2' http://192.168.0.100/ An updated patch that should apply to CVS is attached. ------------------------------------------------------------------------ November 15, 2004 - 06:05 : elias1884 please overthink the revision system default workflow as well. don't look at the revision system as an isolated system but as a part of the whole workflow system! if you combine revisions with the moderation queue the most logic default workflow would be like that: auth user creates node (revision #0) admin approves the node (status = 1, moderation = 0) => node publicly available auth user finds typo and changes node (revision #1, status = 0, moderation = 1) ------- what happens at that point at the moment is, that the node is not accessible anymore at all until the new revision is approved by admin. of course the new revision should not go online until reviewed and approved, this is absolutely correct, but there is no reason to not take the old revision offline, since it was already approved and should therefore be online until the new revision is approved. it is not practical if a node disappears only because the author corrected a typo. ------- admin approves the node (status = 1, moderation = 0) eventhough I first thought a plain boolean active field would not be capable of providing that functionality if finally came to the conclusion, that it can. The only thing to do is to not set that bit, when a new revision is created, but when it is approved (in case moderation is activated under default workflow). Every revision should have its own moderation, status and active field and on approval they are set like this (status=1, moderation=0, active=Y). When you wanna rollback to an old revision, you can chose between all revisions that already have the moderation bit set back to 0 again and the published to 1. There should be an extra permission for rollback! another concern that I have about the default workflow is, that users can't see the content, they have just created, when moderation is enabled. Eventhough, there is a big fat "submission accepted" presented after submissions, unexperienced users tend to question the information those stupid tincans give them, if they can't find their content afterwards. Many users are really lazy bastards and they don't even read the status messages. The best feedback about whether his story was submitted successfully or not of course is, if he can find the story somewhere on the site, maybe with a status message on top of it, mentioning, that the content is currently not publicly available since it has not been approved yet. there should be a my content section under my account, like somebody is trying to do with the workspace module I guess. so my suggestion is to make (status=0, moderation=1) still available for the creator under a my content section somewhere! ------------------------------------------------------------------------ November 24, 2004 - 05:21 : Nick Nassar I agree. The current workflow for moderation queues and revisions needs to change, but this patch isn't the place for it. The patch is already too big, and it only does the backend stuff. Instead of adding more to this patch and making it take even longer to get into core, would you mind creating a new issue for your UI suggestions, so the those changes can be added as a separate patch? Thanks, Nick ------------------------------------------------------------------------ December 11, 2004 - 13:26 : Dries This patch is _much_ needed so I'd love to see someone revive it. In order for this patch to be accepted, the following needs to be done: Update this patch to CVS HEAD. Rename revid to vid. Rename node_rev to node_revisions. Rename node_rev.changed to node_revisions.timestamp. Rename $rnode to $revision. Fix the coding style to match Drupal's: proper spacing, single quotes where possible, proper variable names. Benchmark this patch with a large database with enough revisions. I'd be happy to benchmark this on my local copy of the drupal.org database. The book.log field should probably move to the node_revisions table. This can be done in a separate patch. Investigate whether transactions are well-supported. ------------------------------------------------------------------------ December 13, 2004 - 01:25 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/Drupal-Improved_Revision_Schema_10-26-2004-r… (11.02 KB) I've worked a bit on the patch (coding style issues as mentioned by Dries). One thing I noticed is that the patch uses REPLACE. IIRC this needs to be chagned to "UPDATE, if fail INSERT" for pgsql compatibility. Nick, are you still interested in working on that patch? I'd like to know how it works on your site and work on getting it into core. ------------------------------------------------------------------------ December 13, 2004 - 13:33 : Dries Gerhard: your patch does not apply. ------------------------------------------------------------------------ December 13, 2004 - 18:10 : killes(a)www.drop.org Yes, I know, that was the same version as I mailed to you earlier. ------------------------------------------------------------------------ December 13, 2004 - 22:02 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions.patch (52.96 KB) Ok, upüdated the patch to cvs. ------------------------------------------------------------------------ December 14, 2004 - 09:58 : Dries Some more comments: db_begin_transaction() and db_end_transaction() do not belong in database.inc, but in database.mysql.inc and database.pgsql.inc respectively. The node module calls node_revisionsision_list() which is not defined. (Fxed that on my local copy.) Do db_begin_transaction() and db_end_transaction() deprecate Jeremy's table locking patch? The upgrade path assigns the wrong user ID to each revision. The upgrade path assigns the wrong date to each revision (that or a node's revision page shows the wrong usernames/dates). The coding style needs a bit of work, but we can worry about that later. ------------------------------------------------------------------------ December 14, 2004 - 18:34 : Nick Nassar If you need any help getting those things fixed, just let me know. ------------------------------------------------------------------------ December 14, 2004 - 18:50 : Nick Nassar How this relates to Jeremy's node locking patch: There was lots of discussion, and node locking was decided against because from an end user point of view you never want a node to be locked. He's now advocating for a much simpler patch that warns users if their changes will overwrite someone elses. That patch still has a race condition, which might be fixed using db_begin_transaction(). http://drupal.org/node/6025 ------------------------------------------------------------------------ December 14, 2004 - 23:26 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_0.patch (55.96 KB) Here is an updated patch that tries to address Dries concerns. ------------------------------------------------------------------------ December 15, 2004 - 09:32 : Dries Attachment: http://drupal.org/files/issues/revisions-bug.png (76.06 KB) It didn't fix the aforementioned bugs. See attached screenshot. ------------------------------------------------------------------------ January 6, 2005 - 21:15 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_1.patch (51.77 KB) Ok, here is a new version. Dries and myself worked hart at it, so please have a look. what is still missing - database upgrades for the core modules with an own table - contrib modules need an upgrade too. - do we need nid and vid in both the node and the node_revisions table? - the amount of sql queries means a good stress testing for large databases. ------------------------------------------------------------------------ January 19, 2005 - 22:43 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_2.patch (49.49 KB) Here is an updated patch. We discussed to keep the current title in node module and also in the revisiosn table. This is content duplication but will save many joins as many queries only need the title of a node. Discussion is welcome. ------------------------------------------------------------------------ January 20, 2005 - 00:33 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_3.patch (29.93 KB) I've implemented the aforementioned solution. This makes the patch much smaller. The patch now also removes taxonomy_node_has_term() which wasn't used anywhere. I'd really apprciate if some people could test drive the patch. It will be another huge improvement for 4.6. ------------------------------------------------------------------------ January 20, 2005 - 01:05 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_4.patch (30 KB) Another revision. Steven didn't like my literal $node->vid in queries. ------------------------------------------------------------------------ January 20, 2005 - 02:10 : killes(a)www.drop.org - database upgrades for the core modules with an own table - contrib modules need an upgrade too. - do we need nid and vid in both the node and the node_revisions table? - the amount of sql queries means a good stress testing for large databases. These issues are still open, btw. Especially the first one needs to be tackled. ------------------------------------------------------------------------ January 25, 2005 - 21:11 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_5.patch (51.13 KB) Here is a patch that has the database tables updated for forum, book, and page module. ------------------------------------------------------------------------ January 29, 2005 - 23:55 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_6.patch (49.18 KB) Yet another update to keep it working with head. The patch now also removes the table definitons for the page table. ------------------------------------------------------------------------ January 29, 2005 - 23:57 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_7.patch (55.69 KB) Sorry, that was the old version, this is the right one. ------------------------------------------------------------------------ January 31, 2005 - 20:55 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_8.patch (55.71 KB) Updated once more. ------------------------------------------------------------------------ January 31, 2005 - 21:52 : Dries Anyone to help review/test this? ------------------------------------------------------------------------ January 31, 2005 - 22:22 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_9.patch (49.29 KB) Updated again, the update functions occurred twice. Thanks Bart. ------------------------------------------------------------------------ February 2, 2005 - 01:27 : killes(a)www.drop.org Don't know if the db I am using is corrupted or what. I still do have some didficulties. The latest patch is attached. ------------------------------------------------------------------------ February 2, 2005 - 01:27 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_10.patch (49.67 KB) I am probably slowly going mad ... ------------------------------------------------------------------------ February 2, 2005 - 02:54 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_11.patch (48.95 KB) The update issue still needs investigating. This patch is updated for cvs. ------------------------------------------------------------------------ February 2, 2005 - 21:20 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_12.patch (49.83 KB) Ok, here is a new version. I've solved my troubles with book.module. There are still some issues with forum module. Possibly due to inconsistent database. ------------------------------------------------------------------------ February 2, 2005 - 22:31 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_13.patch (49.83 KB) Turns out the drupal.org database had indeed some quirks. Please run this query in your oldest db and tell me the result: select nid,type from node where type like '%/%'; If you get a non-zero result we might need to add another security update. The patch could use still more testing, though. ------------------------------------------------------------------------ February 3, 2005 - 02:16 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_14.patch (49 KB) Ok, we are getting somewhere. At a first glance the update is working. There is a problem remaining: the revisions tab will be shown whether the node has revisions or not. Not sure we can/need to fix this. People with a drupal.org account can log in at http://killes.drupaldevs.org/revision/ and poke around. Your permissions will be the same as on drupal.org. Feel free to vreak everything but don't forget to file complaints here. (Note: this is only a pruned version of the drupal.org database with all project nodes and nodes with nids > 7000 dropped). ------------------------------------------------------------------------ February 3, 2005 - 05:19 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/revisions_15.patch (52.39 KB) There was some error in node_save and also the patches to the database.inc files got lost... ------------------------------------------------------------------------ February 3, 2005 - 08:07 : robertDouglass Submitting book pages doesn't work on your test site. It puts the entire content of the preview inside the body textarea. I wrote a sentence in the body and the log, and pressing preview put several lines of HTML containing both sentences in the body textarea on the preview page, plus the book page wouldn't submit. -R ------------------------------------------------------------------------ February 3, 2005 - 08:50 : Junyor 0 results here. I started using Drupal with version 4.4, though. ------------------------------------------------------------------------ February 4, 2005 - 00:56 : killes(a)www.drop.org @Junyor: Thanks, that's a good sign. Maybe somebody else has an older db to try. @robertDouglass: The first effect you describe is due to drupaldevs running on PHP 5. I am unsure why the second thing does not work. In node_save() the node object has a nid although there is none in the form. Very strange. I've enabled display of db queries on the testsite. ------------------------------------------------------------------------ February 4, 2005 - 20:17 : dmjossel No results here on the query: select nid,type from node where type like '%/%'; On a database that was put in place prior to Drupal 4 and is now running on 4.5.2. ------------------------------------------------------------------------ February 4, 2005 - 21:44 : killes(a)www.drop.org @dmjossel: thanks. @all. The strange problem I reported was apparently php 5 related. After applying Steven's php 5 patch it went away. One error is remaining: If I create a new forum topic it will be shown as part of the book on preview. Hmm, that was due to a db that got corrupted during testing so that is fixed too. Please poke around at the test site and look if you find more errors. ------------------------------------------------------------------------ February 5, 2005 - 08:16 : Steven By the way, I just remembered that Drupal.org has some blogs lingering on in the database even though blog.module is not enabled. Perhaps this is causing troubles? ------------------------------------------------------------------------ February 5, 2005 - 12:22 : Anonymous I can't see why it would. Drupal.org will need extra updates for images and project nodes because those have their own tables. GK. -- View: http://drupal.org/node/7582 Edit: http://drupal.org/project/comments/add/7582

1 0

[drupal-devel] [feature] improve theme_box to make it better themeable and consistent with blocks
by adrinux 06 Feb '05

06 Feb '05

Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: Bèr Kessels Reported by: Bèr Kessels Updated by: adrinux Status: patch Personally I find the 'every element with a class and id' style of drupal slightly annoying. It seems to me there is great emphasis on keeping the php code clean and effecient, but when it comes to xhtml a 'kitchen sink' include everything that a theme designer might possibly need and somewhat bloated approach prevails. To me it all adds clutter that makes designing drupal themes harder not easier, especially for newcomers faced with a huge array of classes and ID's and not knowing quite what to use, or where they come from. Documenting them helps but not a lot. That said I'm for anything that lets me over-ride xhtml and add my own in phptemplate :) I'm not conviced that enforced ID's on all boxes is a good idea... adrinux Previous comments: ------------------------------------------------------------------------ January 8, 2005 - 21:37 : Bèr Kessels Attachment: http://drupal.org/files/issues/theme_box.patch (1.24 KB) This patch changes the theme_box function. Currently we lak the ability to style blocks different, since we cannot add classes or IDS. I fixed the theme_box to follow the system we use for sideblocks. The id and classes should be provided by arguments. I have only one problem: backwards compatibility. To output valid XHTML all calls of this function must add a unique ID, I documented how this must be done, thats not the problem. The problem, is that we do not do this atm. So untill all calls are fixed, I do: <?php theme_box($title, $content, $class = NULL, $id = NULL) ?> and not <?php theme_box($title, $content, $class, $id) ?> which would be correct, since only the later will force developers to use and ID, resulting in always validating XHMTML. I will try to generate a grep list with places where theme box is called and where it should be fixed. Bèr ------------------------------------------------------------------------ January 8, 2005 - 22:25 : tangent XHTML does not require a div to have an ID attribute. Why would an ID be required in this case? ------------------------------------------------------------------------ January 8, 2005 - 22:40 : Bèr Kessels XHTML does not require a div to have an ID attribute. Why would an ID be required in this case? Because i do not wat to add an if($id) { } or even an $id ? 'id= etc. That would be loads of ifs that are not needed: its much better to enforce everyone to add an ID. Also, if we make ids optional, a lot of lazy developers will not add it, resulting in a lot of brokenish themes. I perfer to have always an ID, and put tit in the coding guidelines, then to have only sometimes an id. And the last reason is, that with the current useability discussions going on, there are people starting to look at javascripts in their themes. We should be ahead of that, and simply give all elements an ID. So that themers can add their javascript to do fancy stuff to parts of the pages. (like hide and show them using cards and or accordeons) ------------------------------------------------------------------------ January 8, 2005 - 22:46 : tangent I see where you're going but I don't think that we should necessarily require *all* blocks to have an id. If a module developer doesn't want his block to have an id and prefers to have a set of blocks with the same or different classes I see nothing wrong with that. Requiring an ID seems a bit heavy handed. This is really another issue but how will user definable modules be handled? I assume the module would just use an incremented ID? ------------------------------------------------------------------------ January 9, 2005 - 01:10 : chrisada "if we make ids optional, a lot of lazy developers will not add it " As one who theme Drupal quite a lot, I am absolutely with Bèr on this point. The deveoper might not need the id to be there, but there will be peoples who want them. Right now there is just too many things in Drupal that is impossible to style due to lack of CSS id. (classes we have plenty) ------------------------------------------------------------------------ January 12, 2005 - 14:01 : Bèr Kessels I cannot see the issues with "requiring" an ID. Really: is that so hard to do? A module might have one, or two, sometimes three blocks. Why cant you add two letters to your code for a unique ID? Its all about consistency. I already know a huge lot of places in the code where the *block theme function iteself* should have been used, but is not. This is annoying, not for developers, they don't care. But for both users, who see inconsistent interfaces, and for themers who go like: Hey: I am sure I had all titles of all pages set to bleu, why is that one red? --aah. f*c, its an h3 that is not in a block, grrr. Same will go for blocks ids. I do not know why, but the general html-code and interfaces of contributions are plain bad, in drupal. Core is nice, but a lot of contributions are hacks-upon-hacks, to keep up with the changing drupal-core versions. We should enforce developers to at least keep consistancy. ------------------------------------------------------------------------ January 25, 2005 - 21:03 : Bèr Kessels Attachment: http://drupal.org/files/issues/theme_box_0.patch (1.22 KB) This one has all parameters required. ------------------------------------------------------------------------ January 25, 2005 - 21:53 : drumm The html outputting could be cleaned up a little, using single quotes where possible and not putting variables inside strings. Also, I think this sohuld be renamed to theme_page_section() and it should not necessarily actually draw a box. Whether something has a literal box or not should be up to the themer on a case-by-case basis. I see this themeable function as omething to break pages up into areas and not add boxes. Otherwise, I like where this is going and would like to see the code changed to call this properly. For now I won't give a whole +1, but it deserves at least +0.5. -- View: http://drupal.org/node/15332 Edit: http://drupal.org/project/comments/add/15332

1 0

[drupal-devel] [feature] Menu fast edit node links
by Dries 06 Feb '05

06 Feb '05

Project: Drupal Version: cvs Component: menu system Category: feature requests Priority: minor Assigned to: mathias Reported by: mathias Updated by: Dries Status: patch I'm going to put this on hold until the collapsbile form groups [1] settled. I encourage you to participate in the design and discussion so that it meets your requirements. The same should be done for other parts of the node edit form, so let's try to come up with a generic solution that can be used through an API that shields you from all Javascript details. [1] http://drupal.org/node/16204 Dries Previous comments: ------------------------------------------------------------------------ July 9, 2004 - 19:15 : mathias Attachment: http://drupal.org/files/issues/menu_fast_edit.patch (1.18 KB) When developers are creating custom menus, the process of adding content becomes twofold: 1. Create content 2. Navigate to administer > menus > add menu item and add the latest node entry This patch adds a link hook to the menu module. Specifically if a node does not have a menu entry the 'add menu item' link appears. If a menu item exists, the 'edit menu item' and 'delete menu item' are displayed thereby speeding up the publishing workflow. Note: This feature is only present when the menu module is enabled. Status: Waiting for JonBobs blessing ... ------------------------------------------------------------------------ July 10, 2004 - 19:21 : JonBob This is interesting. I do have a couple reservations. This adds a bunch of queries to node listing pages, since it has to check for every node whether that node has a menu item set. I also wonder how much time this saves the user in reality. It saves navigating to the menu admin screen, sure, but provides little other help. It seems that if we are to provide a link to the menu item add page, we should at least be pre-filling in the path and title fields for the user. The provided method is little better than having the menu admin page open in another browser tab. Maybe a node link isn't the right interface for this, anyway? Possibly a tab, or a hook_nodeapi('form admin') injection? If done right, this could finally replace that aging "link title" field in page.module nodes. ------------------------------------------------------------------------ January 24, 2005 - 23:23 : mathias Attachment: http://drupal.org/files/issues/menu_quick_link.patch (5.91 KB) This new patch gives users the ability to define menu items on any node creation/editing form. Screenshot here: http://asitis.org/tmp/menu_quick_link_form.png [2] It does this through nodeapi (as suggested by Jonbob), which means that any node has access to these fields unless specified otherwise via the new menu configuration settings. The functionality lives in menu.module, and users with 'administer menus' permission are allowed to edit the menu item fields inside the node form. One more note. I'm also proposing the addition of a misc/drupal.js [3] file to Drupal core (* gasp *). Currently it includes javascript functions to hide regions of a page. When a link is clicked on for a collapsed region, the region unfolds revealing the additional parts of a page. In this case the menu item fields are displayed after clicking the 'Menu item settings' link. I think it helps make best use of screen real estate, but we'll leave that verdict up to the usability experts. Let's remove the js file if it's the only thing stopping this patch from being submitted. I thought it might be worth exploring, especially for form pre and form post regions of the node form. [2] http://asitis.org/tmp/menu_quick_link_form.png [3] http://asitis.org/tmp/drupal.js ------------------------------------------------------------------------ January 24, 2005 - 23:34 : mathias Woops. Setting status to 'patch'. ------------------------------------------------------------------------ January 25, 2005 - 15:17 : Bèr Kessels +1 on the feature, I use menu_otf all the time. -1 for the javascript. Not that i am against it, ion contrary. I think we should not start implementing JS on a per-case basis, but we should introduce a javascript system that can do your accordeon-magick on any element. So it should wait IMO. ------------------------------------------------------------------------ January 25, 2005 - 17:07 : Dries I agree with Ber: +1 for the feature, -1 for the Javascript. This would (or should) be easy to implement/integrate as soon the new node forms are done. Both the new node forms, and this feature are crucial. ------------------------------------------------------------------------ January 27, 2005 - 22:39 : mathias Attachment: http://drupal.org/files/issues/menu_quick_link_0.patch (4.12 KB) Okay, here's the latest patch, footloose and javascript free. ------------------------------------------------------------------------ February 1, 2005 - 10:04 : robertDouglass +1 to the feature, with or without javascript. The Menu item title:* is showing that it is required, but it can't be required as that would force users to create menu items (and it doesn't validate to be required). The patch does, however, open up a big can of permissions worms, and lacks configuration. I would like to see a checkbox on the Workflow (/admin/node/configure/types/{type}) page so that I can hide this on a per-type basis. Then, I think we need some way of limiting this by role, by user, and by menu. I would at least want an "excluded menus" list where I could, say turn off the main Navigation menu so that people don't go and trash it. Very cool feature in a very important direction! I sincerely hope this makes it into the code freeze. ------------------------------------------------------------------------ February 3, 2005 - 00:37 : Anonymous I removed the required mark next to menu item form field. Thanks for spotting that. I don't completely understand what you mean by this patch 'opening up a big can of permissions worms'. User's with 'administer menus' permission are simply given an easier way to add menu items while creating content. I think it's better to keep it simple to begin with. http://asitis.org/tmp/menu_quick_link_form.png [4] There actually is a configuration interface (settings/menu). Your menu cache probably needed to be cleared after applying the patch :-) On that page you can choose which node types acquire the quick menu item form field as well as define the default menu item. I disagree with putting the menu item form visibility settings on the admin/node/configure/types/{type} page. Those settings are for the default state of the node instance, not what's visible on the form field. For example, I wouldn't want to see a list of all 'form pre' and 'form post' fields on that page to select from, but it could be useful as a standard interface elsewhere. [4] http://asitis.org/tmp/menu_quick_link_form.png ------------------------------------------------------------------------ February 3, 2005 - 00:40 : mathias Last comment by 'learning to login' mathias. -- View: http://drupal.org/node/9178 Edit: http://drupal.org/project/comments/add/9178

1 0