June 2005 - Development - lists.drupal.org

[drupal-devel] [bug] don't show blocks if they have no content
by Robin Monks 13 Jun '05

13 Jun '05

Issue status update for http://drupal.org/node/7897 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Robin Monks Reported by: ax Updated by: Robin Monks Status: patch Attachment: http://drupal.org/files/issues/no.block.display.on.no.content_0.patch (3.16 KB) Chx recommended moving the returns into their relative ifs. This also works on HEAD. Robin Robin Monks Previous comments: ------------------------------------------------------------------------ May 19, 2004 - 18:06 : ax Attachment: http://drupal.org/files/issues/no.blocks.without.content.patch (5.06 KB) blog, forum, and statistics.module show their blocks even if they have no contents. this looks quite dull, eg. +----------------------+ | Forum topics | | | | Active forum topics: | | | | New forum topics: | | | | more | +----------------------+ +----------------------+ | Popular content | | | | Today's: | | | | All time: | | | | Last viewed: | | | +----------------------+ and isn't what the rest of the drupal blocks do (not showing up if there is no content). this patch fixes this. additional improvements: * removed the "more" links from blog and forum.module and replaced them with block titles linking to the respective module page. rationales: + "more" means there are more blog entries/forum topics than those linked from the blocks. this, however, wasn't always the case. + it is common user expectation that the title of a "content excerpt" links to the complete content. examples: node titles on the homepage, links in admin tables, tracker, etc. + it saves screen estate * fixed the forum link description ('Read the latest forum topics.' > 'Go to the forums.') * added a link from the 'Popular content' block title to the 'Most popular content' page (statistics). this wasn't accessible before. please review and apply. ------------------------------------------------------------------------ May 20, 2004 - 14:23 : Dries I haven't tested the patch yet but it would be nice if more block titles were links (eg. the title of the archive block could link to the main archive page). ------------------------------------------------------------------------ May 31, 2004 - 22:24 : ax want me to make all block titles link to the respective module page? ------------------------------------------------------------------------ June 2, 2004 - 18:56 : Dries I'd like to discuss this at the very least. :-) ------------------------------------------------------------------------ June 2, 2004 - 19:47 : Boris Mann I don't want clickable block titles! This is a theming issue, and I don't think it should be in by default. What is wrong with the "more" link? Empty blocks not displaying content (i.e. block doesn't show at all) is OK for some blocks, not OK for others. This would ideally be a configuration option: "display even if empty" and a checkbox. I've always wanted "Active" and "New" forum topics a) either split into two blocks or b) made it configurable to display one or the other or both. ------------------------------------------------------------------------ June 2, 2004 - 19:51 : Goba Targets for block title links could be provided by modules, but it is after all the decision of modules to print out block titles using those links. ------------------------------------------------------------------------ June 3, 2004 - 11:26 : Kjartan Once upon a time blocks defind a link along with the subject and content. I think we should re-introduce it, and have it replace the more link that some blocks add. This way themes can choose how and where they put the link (title, text, icon, etc). ------------------------------------------------------------------------ June 3, 2004 - 12:53 : moshe weitzman we already pass the 'more' link separate from the content. this is about how the core themes should present that 'more' link ------------------------------------------------------------------------ June 3, 2004 - 12:58 : Kjartan >From my quick look at blocks in HEAD this is not true for the aggregator or blog blocks, they have specific div & CSS class around them yes, but they are not 100% controlable in the theme. ------------------------------------------------------------------------ June 3, 2004 - 17:18 : Boris Mann " Once upon a time blocks defind a link along with the subject and content. I think we should re-introduce it, and have it replace the more link that some blocks add. This way themes can choose how and where they put the link (title, text, icon, etc). " +10!! Yes, make more block stuff themeable. This is a great solution, since you could also add a "link" field to custom blocks that would display in the same way. Ideally, both the text (e.g. "more", "check this out", "extra stuff", etc. etc.) and the link would be configurable, and the theme would take care of how this is displayed. ------------------------------------------------------------------------ June 17, 2004 - 18:52 : Dries I'm marking this patch 'active' as we agreed to disagree with the suggested patch/approach. Please resubmit or split your patch in smaller chunks. Thanks. ------------------------------------------------------------------------ June 13, 2005 - 12:28 : Robin Monks Attachment: http://drupal.org/files/issues/no.block.display.on.no.content.patch (3 KB) This patch fixes the actual goal of the bug "don't show blocks if they have no content". This is basically ax's patch broken up and updated to HEAD. This patches forum.module and stats. It's been tested to work on CVS HEAD. I'll take responsibility for upcoming changes to this patch. Robin

1 0

[drupal-devel] [bug] code-style.pl script
by killes 13 Jun '05

13 Jun '05

Issue status update for http://drupal.org/node/167 Project: Drupal Version: cvs Component: other Category: bug reports Priority: normal Assigned to: Anonymous Reported by: barry(a)alted.co.uk Updated by: killes(a)www.drop.org -Status: active +Status: patch Attachment: http://drupal.org/files/issues/code-style.patch (669 bytes) Dunno if this causes elsewhere, but -- doesn't get reported anymore. killes(a)www.drop.org Previous comments: ------------------------------------------------------------------------ May 9, 2002 - 13:03 : barry(a)alted.co.uk the code-style.pl script can't handle code containing the increment ++ operator. ------------------------------------------------------------------------ May 11, 2002 - 13:51 : Anonymous Need a regular expression genius to fix this. I can't figure it out so any other takers? ------------------------------------------------------------------------ May 14, 2002 - 10:47 : barry(a)alted.co.uk I was running an older version of the script. ------------------------------------------------------------------------ July 6, 2004 - 00:33 : chx Well, I am using the latest version ($Id: code-style.pl,v 1.8 2003/12/28 10:40:17 dries Exp $) and the -- operator is not handled. So I have reopened this bug report.

1 0

[drupal-devel] [bug] don't show blocks if they have no content
by Robin Monks 13 Jun '05

13 Jun '05

Issue status update for http://drupal.org/node/7897 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal -Assigned to: ax +Assigned to: Robin Monks Reported by: ax Updated by: Robin Monks -Status: active +Status: patch Attachment: http://drupal.org/files/issues/no.block.display.on.no.content.patch (3 KB) This patch fixes the actual goal of the bug "don't show blocks if they have no content". This is basically ax's patch broken up and updated to HEAD. This patches forum.module and stats. It's been tested to work on CVS HEAD. I'll take responsibility for upcoming changes to this patch. Robin Robin Monks Previous comments: ------------------------------------------------------------------------ May 19, 2004 - 18:06 : ax Attachment: http://drupal.org/files/issues/no.blocks.without.content.patch (5.06 KB) blog, forum, and statistics.module show their blocks even if they have no contents. this looks quite dull, eg. +----------------------+ | Forum topics | | | | Active forum topics: | | | | New forum topics: | | | | more | +----------------------+ +----------------------+ | Popular content | | | | Today's: | | | | All time: | | | | Last viewed: | | | +----------------------+ and isn't what the rest of the drupal blocks do (not showing up if there is no content). this patch fixes this. additional improvements: * removed the "more" links from blog and forum.module and replaced them with block titles linking to the respective module page. rationales: + "more" means there are more blog entries/forum topics than those linked from the blocks. this, however, wasn't always the case. + it is common user expectation that the title of a "content excerpt" links to the complete content. examples: node titles on the homepage, links in admin tables, tracker, etc. + it saves screen estate * fixed the forum link description ('Read the latest forum topics.' > 'Go to the forums.') * added a link from the 'Popular content' block title to the 'Most popular content' page (statistics). this wasn't accessible before. please review and apply. ------------------------------------------------------------------------ May 20, 2004 - 14:23 : Dries I haven't tested the patch yet but it would be nice if more block titles were links (eg. the title of the archive block could link to the main archive page). ------------------------------------------------------------------------ May 31, 2004 - 22:24 : ax want me to make all block titles link to the respective module page? ------------------------------------------------------------------------ June 2, 2004 - 18:56 : Dries I'd like to discuss this at the very least. :-) ------------------------------------------------------------------------ June 2, 2004 - 19:47 : Boris Mann I don't want clickable block titles! This is a theming issue, and I don't think it should be in by default. What is wrong with the "more" link? Empty blocks not displaying content (i.e. block doesn't show at all) is OK for some blocks, not OK for others. This would ideally be a configuration option: "display even if empty" and a checkbox. I've always wanted "Active" and "New" forum topics a) either split into two blocks or b) made it configurable to display one or the other or both. ------------------------------------------------------------------------ June 2, 2004 - 19:51 : Goba Targets for block title links could be provided by modules, but it is after all the decision of modules to print out block titles using those links. ------------------------------------------------------------------------ June 3, 2004 - 11:26 : Kjartan Once upon a time blocks defind a link along with the subject and content. I think we should re-introduce it, and have it replace the more link that some blocks add. This way themes can choose how and where they put the link (title, text, icon, etc). ------------------------------------------------------------------------ June 3, 2004 - 12:53 : moshe weitzman we already pass the 'more' link separate from the content. this is about how the core themes should present that 'more' link ------------------------------------------------------------------------ June 3, 2004 - 12:58 : Kjartan >From my quick look at blocks in HEAD this is not true for the aggregator or blog blocks, they have specific div & CSS class around them yes, but they are not 100% controlable in the theme. ------------------------------------------------------------------------ June 3, 2004 - 17:18 : Boris Mann " Once upon a time blocks defind a link along with the subject and content. I think we should re-introduce it, and have it replace the more link that some blocks add. This way themes can choose how and where they put the link (title, text, icon, etc). " +10!! Yes, make more block stuff themeable. This is a great solution, since you could also add a "link" field to custom blocks that would display in the same way. Ideally, both the text (e.g. "more", "check this out", "extra stuff", etc. etc.) and the link would be configurable, and the theme would take care of how this is displayed. ------------------------------------------------------------------------ June 17, 2004 - 18:52 : Dries I'm marking this patch 'active' as we agreed to disagree with the suggested patch/approach. Please resubmit or split your patch in smaller chunks. Thanks.

1 0

[drupal-devel] [bug] Do not execute functions in include files
by gordon 13 Jun '05

13 Jun '05

Issue status update for http://drupal.org/node/18213 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: chx Reported by: killes(a)www.drop.org Updated by: gordon Status: patch +1, I agree that this needs to go into live sooner, or we are not going to be able to get it into the next release. This is going to require a good shake down to make sure that it is all working. gordon Previous comments: ------------------------------------------------------------------------ March 2, 2005 - 08:09 : killes(a)www.drop.org Drupal currently executes functions in include files. This is very annoying if you want to include files from external scripts such as update scripts or other scripts that want to use Drupal as a framework. Offending files are: database.inc: db_set_active(); session.inc: session_set_save_handler("sess_open", "sess_close", "sess_read", "sess_write", "sess_destroy", "sess_gc"); session_start(); common.inc: A bunch of stuff at the bottom. All this should be moved into a start.inc file. ------------------------------------------------------------------------ March 27, 2005 - 16:44 : adrian I haven't done this recently, but back in 4.4 era I attempted something similar to this for the install api. A _lot_ of modifications were required to be able to use (for instance) the theme api without needing to have a database connection active. ------------------------------------------------------------------------ March 27, 2005 - 16:55 : adrian It's definately not a simple problem. For instance.. the db connection needs to be started during bootstrap, and during normal execution. I think the better idea is to include a DRUPAL_INIT constant we can check before executing the function. (or a DRUPAL_NO_INIT .. i'm not picky). ------------------------------------------------------------------------ April 29, 2005 - 07:53 : killes(a)www.drop.org Here's an old patch by me that illustrates what I want to do. http://lists.drupal.org/archives/drupal-devel/2005-03/txtq23pAcHATw.txt Instead of putting the stuff into index.php, alternative file names need to be found. ------------------------------------------------------------------------ May 5, 2005 - 03:36 : chx Attachment: http://drupal.org/files/issues/boot.patch (6.83 KB) Well, I have written something to address the issue. Now you can include everything to your hearts content just include init.inc as well and do an appropriate drupal_bootstrap. Alas, ATM database and bootstrap is very interwined. This interdependency could be obliterated if we'd not use variable_get('dev_query') in database.*.inc. cvs diff -u -N -F^f against cvs.drupal.org as CVSROOT failed to include to include the two new files, so I'll include them as followups. ------------------------------------------------------------------------ May 5, 2005 - 03:37 : chx Attachment: http://drupal.org/files/issues/boot_0.patch (10.31 KB) Hm, settings.php changes were ommited. ------------------------------------------------------------------------ May 5, 2005 - 03:37 : chx Attachment: http://drupal.org/files/issues/init.inc (3.46 KB) ------------------------------------------------------------------------ May 5, 2005 - 03:38 : chx Attachment: http://drupal.org/files/issues/settings_ini_set.php (850 bytes) ------------------------------------------------------------------------ May 5, 2005 - 03:44 : chx Attachment: http://drupal.org/files/issues/boot_1.patch (14.49 KB) Following advice from darix I have now created one patch. Still, I'd like to know how could I do this with cvs. ------------------------------------------------------------------------ May 5, 2005 - 04:01 : chx Attachment: http://drupal.org/files/issues/boot_2.patch (14.48 KB) Steef found that I had $$ in settings.php. He also asked what's this good for. Several things, but most advantages come when you use only parts of Drupal. If you want use, say, only our forms functions then now you can include('common.inc'); without side effects. If you need the database layer, you do a include('init.inc') and call drupal_bootstrap('database') ------------------------------------------------------------------------ May 6, 2005 - 07:03 : chx Attachment: http://drupal.org/files/issues/boot_3.patch (17.98 KB) The database layer is now totally independent of other parts of Drupal. ------------------------------------------------------------------------ May 16, 2005 - 05:37 : chx A very stupid bug related to variable init is fixed and I am sync'd to HEAD, so timer_init('page') is called. ------------------------------------------------------------------------ May 17, 2005 - 07:12 : chx Attachment: http://drupal.org/files/issues/bootstrap_3.patch (12.36 KB) ------------------------------------------------------------------------ June 5, 2005 - 01:29 : chx Attachment: http://drupal.org/files/issues/bootstrap_4.patch (12.36 KB) Fixed drupal_bootstrap('database') -- $phase was 0 , and == FALSE fired. === FALSE is used. ------------------------------------------------------------------------ June 5, 2005 - 03:03 : nsk Some days before I was studying the Drupal source code and it caused me headaches that it executed code via include calls. I think it is better design to not execute anything automatically in includes. ------------------------------------------------------------------------ June 5, 2005 - 03:25 : chx Attachment: http://drupal.org/files/issues/bootstrap_5.patch (15.26 KB) Added phpdoc and moved drupal_unpack to init.inc thus now session handler can be used independently. ------------------------------------------------------------------------ June 5, 2005 - 19:13 : Dries Sorry but the quality of the PHPdoc is not up to standards. Please use proper punctuation, typography, line-wrapping, etc. It should also explain when/why to use this function. I can't help but think this is ugly code. drupal_bootstrap, for example, can be written more elegantly (not tested): function drupal_bootstrap($phase) { $phases = array('database', 'session', 'bootstrap', 'common'); foreach ($phases as $step) { _drupal_bootstrap($step); if ($phase == $step) { break; } } } instead of: function drupal_bootstrap($phase) { static $done = -1; $phase = array_search($phase, array('database', 'session', 'bootstrap', 'common')); if ($phase === FALSE) { return; } for (; $done <= $phase; $done++) { _drupal_bootstrap($done); } } ------------------------------------------------------------------------ June 6, 2005 - 02:34 : chx Attachment: http://drupal.org/files/issues/bootstrap_6.patch (15.56 KB) Some code simplification in drupal_bootstrap and better PHPdoc. (Thanks Dries and Steven.) ------------------------------------------------------------------------ June 7, 2005 - 01:19 : moshe weitzman i did a quick code review and this patch looks quite nice to me. note that it changes some of the same bits that are changed by my 'ban hosts' patch. i'm ok with committing this one first, and i will update accordingly. ------------------------------------------------------------------------ June 7, 2005 - 04:56 : Dries - Patch breaks update.php. Might want to double-check cron.php and xmlrpc.php. - The PHPdoc code wraps at variable width/length. Might want to tidy up that a little. ------------------------------------------------------------------------ June 8, 2005 - 06:25 : moshe weitzman in the big case statement in drupal_bootstrap(), perhaps use defined constants instead of 0, 1, 2 ... i think the 4 phase bootstrap is perfect. perhaps move the code in the drupal_bootstrap('all') section to common.inc under a new function called drupal_bootstrap_all(). that makes the code a bit tidier and keeps a few lines out of init.inc which is useful. saves a bit of parsing on requests that don't use the whole framework. you could do that for the other phases as well although the benefit is smaller. ------------------------------------------------------------------------ June 8, 2005 - 06:58 : chx Attachment: http://drupal.org/files/issues/bootstrap_7.patch (20.76 KB) We were already using named phases (not constants), but I have renamed them now as discussed with moshe. Moved the code for the last phase (now called 'full') to common.inc. Simplified index.php -- much simpler than in previous patch, a bit simpler than pre-patch. ------------------------------------------------------------------------ June 8, 2005 - 07:07 : chx Attachment: http://drupal.org/files/issues/bootstrap_8.patch (16.34 KB) Following Moshe's advice to see http://talks.php.net/show/acc_php/33 , I have added a few dots before the paths. ------------------------------------------------------------------------ June 8, 2005 - 07:34 : Bèr Kessels I love the idea of this. +1 for the last (#8) patch from me. I must mention i did not test it thoroughly, hust saw that it works here! The levelled include looks great to me. A minor thing , that I thnik would make this even better, is an additional level for feeds. Feeds need not load all the theme, blocks, and other stuff, only the nodes (items) in the feeds. For reference, my last statistics: Top 30 of 4679 Total URLs # Hits KBytes URL 1 22042 11.11% 2 0.00% /cron.php 2 9418 4.75% 14742 0.60% /favicon.ico 3 6917 3.49% 56598 2.29% /misc/drupal.css ... 13 3272 1.65% 31790 1.29% /node/feed So my main feeds are the 13th most referenced URL on my sites. (the 4-12 contain a lot of odd deeplinked images, and off course /node and / ) ------------------------------------------------------------------------ June 8, 2005 - 08:04 : chx Separating node/feed is not an easy thing, maybe impossible First off, you need most modules as nodeapi is called during this a theme call is possible. I do not see what can be omitted from 'full'. I think this could be a next patch, if we can (which I doubt) an intermediate between 'page cache' and 'full'. ------------------------------------------------------------------------ June 8, 2005 - 16:55 : Bèr Kessels I figured it would be hard. So lets ignore my post. I will think of a way to cache it better inside node.module and taxonomy.module. ------------------------------------------------------------------------ June 13, 2005 - 21:08 : killes(a)www.drop.org I read the patch and would like to see it get into core rather sooner than later. ------------------------------------------------------------------------ June 13, 2005 - 21:21 : stefan nagtegaal This would make things pretty simple.. When we look at the upcoming install-api, we only have todo: <?php drupal_bootstrap('database'); ?> to make database access possible and ready for usage.. I cannot see anything inside the code itself which looks not right, but: <?php + header('HTTP/1.0 403 Forbidden'); + print "Sorry, ". $_SERVER['REMOTE_ADDR']. " has been banned."; + exit(); ?> Shouldn't that be something like:

1 0

[drupal-devel] [bug] Do not execute functions in include files
by stefan nagtegaal 13 Jun '05

13 Jun '05

Issue status update for http://drupal.org/node/18213 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: chx Reported by: killes(a)www.drop.org Updated by: stefan nagtegaal Status: patch This would make things pretty simple.. When we look at the upcoming install-api, we only have todo: <?php drupal_bootstrap('database'); ?> to make database access possible and ready for usage.. I cannot see anything inside the code itself which looks not right, but: <?php + header('HTTP/1.0 403 Forbidden'); + print "Sorry, ". $_SERVER['REMOTE_ADDR']. " has been banned."; + exit(); ?> Shouldn't that be something like: stefan nagtegaal Previous comments: ------------------------------------------------------------------------ March 2, 2005 - 00:09 : killes(a)www.drop.org Drupal currently executes functions in include files. This is very annoying if you want to include files from external scripts such as update scripts or other scripts that want to use Drupal as a framework. Offending files are: database.inc: db_set_active(); session.inc: session_set_save_handler("sess_open", "sess_close", "sess_read", "sess_write", "sess_destroy", "sess_gc"); session_start(); common.inc: A bunch of stuff at the bottom. All this should be moved into a start.inc file. ------------------------------------------------------------------------ March 27, 2005 - 08:44 : adrian I haven't done this recently, but back in 4.4 era I attempted something similar to this for the install api. A _lot_ of modifications were required to be able to use (for instance) the theme api without needing to have a database connection active. ------------------------------------------------------------------------ March 27, 2005 - 08:55 : adrian It's definately not a simple problem. For instance.. the db connection needs to be started during bootstrap, and during normal execution. I think the better idea is to include a DRUPAL_INIT constant we can check before executing the function. (or a DRUPAL_NO_INIT .. i'm not picky). ------------------------------------------------------------------------ April 28, 2005 - 23:53 : killes(a)www.drop.org Here's an old patch by me that illustrates what I want to do. http://lists.drupal.org/archives/drupal-devel/2005-03/txtq23pAcHATw.txt Instead of putting the stuff into index.php, alternative file names need to be found. ------------------------------------------------------------------------ May 4, 2005 - 19:36 : chx Attachment: http://drupal.org/files/issues/boot.patch (6.83 KB) Well, I have written something to address the issue. Now you can include everything to your hearts content just include init.inc as well and do an appropriate drupal_bootstrap. Alas, ATM database and bootstrap is very interwined. This interdependency could be obliterated if we'd not use variable_get('dev_query') in database.*.inc. cvs diff -u -N -F^f against cvs.drupal.org as CVSROOT failed to include to include the two new files, so I'll include them as followups. ------------------------------------------------------------------------ May 4, 2005 - 19:37 : chx Attachment: http://drupal.org/files/issues/boot_0.patch (10.31 KB) Hm, settings.php changes were ommited. ------------------------------------------------------------------------ May 4, 2005 - 19:37 : chx Attachment: http://drupal.org/files/issues/init.inc (3.46 KB) ------------------------------------------------------------------------ May 4, 2005 - 19:38 : chx Attachment: http://drupal.org/files/issues/settings_ini_set.php (850 bytes) ------------------------------------------------------------------------ May 4, 2005 - 19:44 : chx Attachment: http://drupal.org/files/issues/boot_1.patch (14.49 KB) Following advice from darix I have now created one patch. Still, I'd like to know how could I do this with cvs. ------------------------------------------------------------------------ May 4, 2005 - 20:01 : chx Attachment: http://drupal.org/files/issues/boot_2.patch (14.48 KB) Steef found that I had $$ in settings.php. He also asked what's this good for. Several things, but most advantages come when you use only parts of Drupal. If you want use, say, only our forms functions then now you can include('common.inc'); without side effects. If you need the database layer, you do a include('init.inc') and call drupal_bootstrap('database') ------------------------------------------------------------------------ May 5, 2005 - 23:03 : chx Attachment: http://drupal.org/files/issues/boot_3.patch (17.98 KB) The database layer is now totally independent of other parts of Drupal. ------------------------------------------------------------------------ May 15, 2005 - 21:37 : chx A very stupid bug related to variable init is fixed and I am sync'd to HEAD, so timer_init('page') is called. ------------------------------------------------------------------------ May 16, 2005 - 23:12 : chx Attachment: http://drupal.org/files/issues/bootstrap_3.patch (12.36 KB) ------------------------------------------------------------------------ June 4, 2005 - 17:29 : chx Attachment: http://drupal.org/files/issues/bootstrap_4.patch (12.36 KB) Fixed drupal_bootstrap('database') -- $phase was 0 , and == FALSE fired. === FALSE is used. ------------------------------------------------------------------------ June 4, 2005 - 19:03 : nsk Some days before I was studying the Drupal source code and it caused me headaches that it executed code via include calls. I think it is better design to not execute anything automatically in includes. ------------------------------------------------------------------------ June 4, 2005 - 19:25 : chx Attachment: http://drupal.org/files/issues/bootstrap_5.patch (15.26 KB) Added phpdoc and moved drupal_unpack to init.inc thus now session handler can be used independently. ------------------------------------------------------------------------ June 5, 2005 - 11:13 : Dries Sorry but the quality of the PHPdoc is not up to standards. Please use proper punctuation, typography, line-wrapping, etc. It should also explain when/why to use this function. I can't help but think this is ugly code. drupal_bootstrap, for example, can be written more elegantly (not tested): function drupal_bootstrap($phase) { $phases = array('database', 'session', 'bootstrap', 'common'); foreach ($phases as $step) { _drupal_bootstrap($step); if ($phase == $step) { break; } } } instead of: function drupal_bootstrap($phase) { static $done = -1; $phase = array_search($phase, array('database', 'session', 'bootstrap', 'common')); if ($phase === FALSE) { return; } for (; $done <= $phase; $done++) { _drupal_bootstrap($done); } } ------------------------------------------------------------------------ June 5, 2005 - 18:34 : chx Attachment: http://drupal.org/files/issues/bootstrap_6.patch (15.56 KB) Some code simplification in drupal_bootstrap and better PHPdoc. (Thanks Dries and Steven.) ------------------------------------------------------------------------ June 6, 2005 - 17:19 : moshe weitzman i did a quick code review and this patch looks quite nice to me. note that it changes some of the same bits that are changed by my 'ban hosts' patch. i'm ok with committing this one first, and i will update accordingly. ------------------------------------------------------------------------ June 6, 2005 - 20:56 : Dries - Patch breaks update.php. Might want to double-check cron.php and xmlrpc.php. - The PHPdoc code wraps at variable width/length. Might want to tidy up that a little. ------------------------------------------------------------------------ June 7, 2005 - 22:25 : moshe weitzman in the big case statement in drupal_bootstrap(), perhaps use defined constants instead of 0, 1, 2 ... i think the 4 phase bootstrap is perfect. perhaps move the code in the drupal_bootstrap('all') section to common.inc under a new function called drupal_bootstrap_all(). that makes the code a bit tidier and keeps a few lines out of init.inc which is useful. saves a bit of parsing on requests that don't use the whole framework. you could do that for the other phases as well although the benefit is smaller. ------------------------------------------------------------------------ June 7, 2005 - 22:58 : chx Attachment: http://drupal.org/files/issues/bootstrap_7.patch (20.76 KB) We were already using named phases (not constants), but I have renamed them now as discussed with moshe. Moved the code for the last phase (now called 'full') to common.inc. Simplified index.php -- much simpler than in previous patch, a bit simpler than pre-patch. ------------------------------------------------------------------------ June 7, 2005 - 23:07 : chx Attachment: http://drupal.org/files/issues/bootstrap_8.patch (16.34 KB) Following Moshe's advice to see http://talks.php.net/show/acc_php/33 , I have added a few dots before the paths. ------------------------------------------------------------------------ June 7, 2005 - 23:34 : Bèr Kessels I love the idea of this. +1 for the last (#8) patch from me. I must mention i did not test it thoroughly, hust saw that it works here! The levelled include looks great to me. A minor thing , that I thnik would make this even better, is an additional level for feeds. Feeds need not load all the theme, blocks, and other stuff, only the nodes (items) in the feeds. For reference, my last statistics: Top 30 of 4679 Total URLs # Hits KBytes URL 1 22042 11.11% 2 0.00% /cron.php 2 9418 4.75% 14742 0.60% /favicon.ico 3 6917 3.49% 56598 2.29% /misc/drupal.css ... 13 3272 1.65% 31790 1.29% /node/feed So my main feeds are the 13th most referenced URL on my sites. (the 4-12 contain a lot of odd deeplinked images, and off course /node and / ) ------------------------------------------------------------------------ June 8, 2005 - 00:04 : chx Separating node/feed is not an easy thing, maybe impossible First off, you need most modules as nodeapi is called during this a theme call is possible. I do not see what can be omitted from 'full'. I think this could be a next patch, if we can (which I doubt) an intermediate between 'page cache' and 'full'. ------------------------------------------------------------------------ June 8, 2005 - 08:55 : Bèr Kessels I figured it would be hard. So lets ignore my post. I will think of a way to cache it better inside node.module and taxonomy.module. ------------------------------------------------------------------------ June 13, 2005 - 13:08 : killes(a)www.drop.org I read the patch and would like to see it get into core rather sooner than later.

1 0

[drupal-devel] [bug] Do not execute functions in include files
by killes 13 Jun '05

13 Jun '05

Issue status update for http://drupal.org/node/18213 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: chx Reported by: killes(a)www.drop.org Updated by: killes(a)www.drop.org Status: patch I read the patch and would like to see it get into core rather sooner than later. killes(a)www.drop.org Previous comments: ------------------------------------------------------------------------ March 2, 2005 - 00:09 : killes(a)www.drop.org Drupal currently executes functions in include files. This is very annoying if you want to include files from external scripts such as update scripts or other scripts that want to use Drupal as a framework. Offending files are: database.inc: db_set_active(); session.inc: session_set_save_handler("sess_open", "sess_close", "sess_read", "sess_write", "sess_destroy", "sess_gc"); session_start(); common.inc: A bunch of stuff at the bottom. All this should be moved into a start.inc file. ------------------------------------------------------------------------ March 27, 2005 - 08:44 : adrian I haven't done this recently, but back in 4.4 era I attempted something similar to this for the install api. A _lot_ of modifications were required to be able to use (for instance) the theme api without needing to have a database connection active. ------------------------------------------------------------------------ March 27, 2005 - 08:55 : adrian It's definately not a simple problem. For instance.. the db connection needs to be started during bootstrap, and during normal execution. I think the better idea is to include a DRUPAL_INIT constant we can check before executing the function. (or a DRUPAL_NO_INIT .. i'm not picky). ------------------------------------------------------------------------ April 28, 2005 - 23:53 : killes(a)www.drop.org Here's an old patch by me that illustrates what I want to do. http://lists.drupal.org/archives/drupal-devel/2005-03/txtq23pAcHATw.txt Instead of putting the stuff into index.php, alternative file names need to be found. ------------------------------------------------------------------------ May 4, 2005 - 19:36 : chx Attachment: http://drupal.org/files/issues/boot.patch (6.83 KB) Well, I have written something to address the issue. Now you can include everything to your hearts content just include init.inc as well and do an appropriate drupal_bootstrap. Alas, ATM database and bootstrap is very interwined. This interdependency could be obliterated if we'd not use variable_get('dev_query') in database.*.inc. cvs diff -u -N -F^f against cvs.drupal.org as CVSROOT failed to include to include the two new files, so I'll include them as followups. ------------------------------------------------------------------------ May 4, 2005 - 19:37 : chx Attachment: http://drupal.org/files/issues/boot_0.patch (10.31 KB) Hm, settings.php changes were ommited. ------------------------------------------------------------------------ May 4, 2005 - 19:37 : chx Attachment: http://drupal.org/files/issues/init.inc (3.46 KB) ------------------------------------------------------------------------ May 4, 2005 - 19:38 : chx Attachment: http://drupal.org/files/issues/settings_ini_set.php (850 bytes) ------------------------------------------------------------------------ May 4, 2005 - 19:44 : chx Attachment: http://drupal.org/files/issues/boot_1.patch (14.49 KB) Following advice from darix I have now created one patch. Still, I'd like to know how could I do this with cvs. ------------------------------------------------------------------------ May 4, 2005 - 20:01 : chx Attachment: http://drupal.org/files/issues/boot_2.patch (14.48 KB) Steef found that I had $$ in settings.php. He also asked what's this good for. Several things, but most advantages come when you use only parts of Drupal. If you want use, say, only our forms functions then now you can include('common.inc'); without side effects. If you need the database layer, you do a include('init.inc') and call drupal_bootstrap('database') ------------------------------------------------------------------------ May 5, 2005 - 23:03 : chx Attachment: http://drupal.org/files/issues/boot_3.patch (17.98 KB) The database layer is now totally independent of other parts of Drupal. ------------------------------------------------------------------------ May 15, 2005 - 21:37 : chx A very stupid bug related to variable init is fixed and I am sync'd to HEAD, so timer_init('page') is called. ------------------------------------------------------------------------ May 16, 2005 - 23:12 : chx Attachment: http://drupal.org/files/issues/bootstrap_3.patch (12.36 KB) ------------------------------------------------------------------------ June 4, 2005 - 17:29 : chx Attachment: http://drupal.org/files/issues/bootstrap_4.patch (12.36 KB) Fixed drupal_bootstrap('database') -- $phase was 0 , and == FALSE fired. === FALSE is used. ------------------------------------------------------------------------ June 4, 2005 - 19:03 : nsk Some days before I was studying the Drupal source code and it caused me headaches that it executed code via include calls. I think it is better design to not execute anything automatically in includes. ------------------------------------------------------------------------ June 4, 2005 - 19:25 : chx Attachment: http://drupal.org/files/issues/bootstrap_5.patch (15.26 KB) Added phpdoc and moved drupal_unpack to init.inc thus now session handler can be used independently. ------------------------------------------------------------------------ June 5, 2005 - 11:13 : Dries Sorry but the quality of the PHPdoc is not up to standards. Please use proper punctuation, typography, line-wrapping, etc. It should also explain when/why to use this function. I can't help but think this is ugly code. drupal_bootstrap, for example, can be written more elegantly (not tested): function drupal_bootstrap($phase) { $phases = array('database', 'session', 'bootstrap', 'common'); foreach ($phases as $step) { _drupal_bootstrap($step); if ($phase == $step) { break; } } } instead of: function drupal_bootstrap($phase) { static $done = -1; $phase = array_search($phase, array('database', 'session', 'bootstrap', 'common')); if ($phase === FALSE) { return; } for (; $done <= $phase; $done++) { _drupal_bootstrap($done); } } ------------------------------------------------------------------------ June 5, 2005 - 18:34 : chx Attachment: http://drupal.org/files/issues/bootstrap_6.patch (15.56 KB) Some code simplification in drupal_bootstrap and better PHPdoc. (Thanks Dries and Steven.) ------------------------------------------------------------------------ June 6, 2005 - 17:19 : moshe weitzman i did a quick code review and this patch looks quite nice to me. note that it changes some of the same bits that are changed by my 'ban hosts' patch. i'm ok with committing this one first, and i will update accordingly. ------------------------------------------------------------------------ June 6, 2005 - 20:56 : Dries - Patch breaks update.php. Might want to double-check cron.php and xmlrpc.php. - The PHPdoc code wraps at variable width/length. Might want to tidy up that a little. ------------------------------------------------------------------------ June 7, 2005 - 22:25 : moshe weitzman in the big case statement in drupal_bootstrap(), perhaps use defined constants instead of 0, 1, 2 ... i think the 4 phase bootstrap is perfect. perhaps move the code in the drupal_bootstrap('all') section to common.inc under a new function called drupal_bootstrap_all(). that makes the code a bit tidier and keeps a few lines out of init.inc which is useful. saves a bit of parsing on requests that don't use the whole framework. you could do that for the other phases as well although the benefit is smaller. ------------------------------------------------------------------------ June 7, 2005 - 22:58 : chx Attachment: http://drupal.org/files/issues/bootstrap_7.patch (20.76 KB) We were already using named phases (not constants), but I have renamed them now as discussed with moshe. Moved the code for the last phase (now called 'full') to common.inc. Simplified index.php -- much simpler than in previous patch, a bit simpler than pre-patch. ------------------------------------------------------------------------ June 7, 2005 - 23:07 : chx Attachment: http://drupal.org/files/issues/bootstrap_8.patch (16.34 KB) Following Moshe's advice to see http://talks.php.net/show/acc_php/33 , I have added a few dots before the paths. ------------------------------------------------------------------------ June 7, 2005 - 23:34 : Bèr Kessels I love the idea of this. +1 for the last (#8) patch from me. I must mention i did not test it thoroughly, hust saw that it works here! The levelled include looks great to me. A minor thing , that I thnik would make this even better, is an additional level for feeds. Feeds need not load all the theme, blocks, and other stuff, only the nodes (items) in the feeds. For reference, my last statistics: Top 30 of 4679 Total URLs # Hits KBytes URL 1 22042 11.11% 2 0.00% /cron.php 2 9418 4.75% 14742 0.60% /favicon.ico 3 6917 3.49% 56598 2.29% /misc/drupal.css ... 13 3272 1.65% 31790 1.29% /node/feed So my main feeds are the 13th most referenced URL on my sites. (the 4-12 contain a lot of odd deeplinked images, and off course /node and / ) ------------------------------------------------------------------------ June 8, 2005 - 00:04 : chx Separating node/feed is not an easy thing, maybe impossible First off, you need most modules as nodeapi is called during this a theme call is possible. I do not see what can be omitted from 'full'. I think this could be a next patch, if we can (which I doubt) an intermediate between 'page cache' and 'full'. ------------------------------------------------------------------------ June 8, 2005 - 08:55 : Bèr Kessels I figured it would be hard. So lets ignore my post. I will think of a way to cache it better inside node.module and taxonomy.module.

1 0

[drupal-devel] [feature] Forum Index "new" on unread comments
by pyromanfo 13 Jun '05

13 Jun '05

Issue status update for http://drupal.org/node/24885 Project: Drupal Version: 4.6.0 Component: forum.module Category: feature requests Priority: normal Assigned to: Anonymous Reported by: pyromanfo Updated by: pyromanfo Status: patch Attachment: http://drupal.org/files/issues/forum_index_unread_comments.patch (2.04 KB) This patch changes the "X new" in the forum index from being the number of new nodes to just "new" signifying new comments in this forum. It seemed to be much more useful for tracking forum discussions this way, as instead of just knowing about new topics you can see any activity in the forums at all. pyromanfo

1 0

[drupal-devel] [feature] Comment New Link Patch
by pyromanfo 13 Jun '05

13 Jun '05

Issue status update for http://drupal.org/node/24881 Project: Drupal Version: 4.6.0 Component: base system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: pyromanfo Updated by: pyromanfo Status: patch Attachment: http://drupal.org/files/issues/comment_new_link.patch (4.46 KB) This patch takes the user to the first unread comment of nodes with multiple pages of comments. Patches the forums, tracker, active forum topics and "x new comments" links on the front page. pyromanfo

1 0

[drupal-devel] [bug] Strange markup in book render functions
by puregin 13 Jun '05

13 Jun '05

Issue status update for http://drupal.org/node/1898 Project: Drupal Version: cvs Component: book.module Category: bug reports Priority: critical Assigned to: puregin Reported by: kika Updated by: puregin Status: patch Attachment: http://drupal.org/files/issues/xml-export-06.patch (616 bytes) I forgot to include a proper id attribute for book elements in the last patch. The attached patch corrects this, so all 'node container' elements have an id derived from the node id. Djun puregin Previous comments: ------------------------------------------------------------------------ June 9, 2003 - 15:06 : kika Snippet from book_render(): $output .= "<dt>". l($node->title, "node/view/$node->nid") ."</dt><dd>". book_body($node) ."<br /><br /></dd>"; Is using definition list here is really correct? This should be replaced with appropiate div's or even better, funnel through theme() Same goes for the function book_print(): $output .= "$node->title"; if ($node->body) { $output .= "<ul>". book_body($node) ."</ul>"; } Why wrap body inside <ul>? ------------------------------------------------------------------------ June 10, 2003 - 17:31 : al Yeah, it's all nasty. I patched this ages ago, but no one seemed interested. The patch now won't apply due to all the changes since then. I'm on this - expect a new patch to hit my sandbox shortly. ------------------------------------------------------------------------ June 10, 2003 - 23:03 : ax please remove the superfluous <ul>s and <li> [1], too. to solve the problem of invalid nested <hx> tags, maybe we should change <h$depth> to <h1 class="book-h$depth">? this way, we could use <h2> - <h6> in book pages. alternatively, we should only use <h5> and <h6> in book pages - we shouldn't have nestings deeper than 4 in printed books, then. additionally, i think it would be a good idea to add id's to all book headings so they can be jumped to via fragment identifiers (url#fragment-identifier). i often wish this would be possible when referencing the last paragraph of a long page of documentation ... these id's should be of the form <hx id="foo" name="foo"> [2], with foo probably being the node-id of the single book page. adding id's to headings would also be a good idea for headings / sections in single book nodes. for example, it would be nice if i could link to list point 10 in the drupal installation instructions [3] via something like url#10. these id's had to be added manually. the question is: how can these id's be named to be unique within a /printed/ book? probably <node_id>_<section_id> ... [1] [ [drupal-devel] Drupal handbook formatting and heading tags | http://lists.drupal.org/pipermail/drupal-devel/2003-June/025860.html ] [2] [ XHTML Spec, HTML Compatibility Guidelines, Fragment Identifiers | http://www.w3.org/TR/xhtml1/#C_8 ] [3] http://drupal.org/node/view/260 ------------------------------------------------------------------------ June 15, 2003 - 07:23 : al Fixed in latest CVS. Uses all of Ax's suggestions: - <h1 class="depthX"> - Fixed to use CSS rather than tables. - Changes link at top to standard "breadcrumb" style, rather than hierarchy in tree view. - Removes odd and superfluous <ul> tags and the like. - Makes printed docs have proper <html> and <body> wrapping, etc. ------------------------------------------------------------------------ May 5, 2005 - 00:20 : puregin I'm re-opening this, because the problem persists in CVS. 1) The current CVS version (1.288.2.4) of book module still wraps $node->body in an <ul> element in book_print_recurse(). This produces non-conformant HTML. I'm not sure why this was done in the first place - I'm guessing someone wanted to insulate from having bad markup in a book page throw off the enclosing page. Getting rid of the offending UL tags is a one-line solution, but what is the right thing to do here? I'd like to see something very structural - wrapping the /entire/ output (including the generated sectional header) of the book_print_recurse() wrapped in a <div> element. This will properly nest true structural sections, making the structure accessible to applications via DOM. It will also make it easier to use XSLT, for example, to transform into various XML based export formats, for example. Any reason not to do so? Also... when we get the 'printer-friendly' version of a page, the subtree rooted at that page is returned, and the root page is marked up with a class="book-h1". Is there any reason why we don't compute the depth of the root page, and use that to generate the "in-place" header? In other words, if a section is "book-h3" relative to the top-level book, shouldn't it be "book-h3" whenever it is viewed? In this case, we could apply the same principal to generating the <div> tags around each section. ------------------------------------------------------------------------ May 5, 2005 - 15:58 : puregin Attachment: http://drupal.org/files/issues/book_printer_friendly_div.patch (1.57 KB) I am attaching a patch which fixes improper printer-friendly output, as discussed. The patch fixes book_print and book_print_recurse() to insert a <div> start tag before emitting the header, and to close this after subsections have been recursively generated. The <div> tag has an id attribute of the form id="sect-123", where the node's nid is 123, and a class attribute of form class="sect/n/" where the depth of the section is /n/. I'm marking this issue as critical, since the present HTML output plays havoc with any attempts at sane PDF generation. I will address the issue of where printer-friendly subsections should be 'rooted' in a follow-up. Djun ------------------------------------------------------------------------ May 6, 2005 - 01:53 : Dries Let's not abbreviate 'section' to 'sect'. Also, we use '-' to separate words in CSS names. - If the section ID uses the node ID, maybe we should use 'node-' instead of 'sect-'? - If the class uses depth, maybe we should use 'depth-' instead of 'sect'? The same is true for your other patch where you suggest to use 'n'. Better to make that 'node-'. (Feel free to merge both patches.) Looks like the book.module generates a _lot_ of CSS IDs/classes. If the introduction of these new IDs/classes allows us to remove some of the existing IDs/classes, please do. ------------------------------------------------------------------------ May 6, 2005 - 21:26 : Steven Here's my idea.. why not use regular h tags for the book structure, and renumber the header tags in the content, starting one level after the current book depth? Here's a function which does this: <?php function fix_up_headers($text, $level = 1) { // Find all header tags that are used (if any) if (preg_match_all('/<h[0-9]+/i', $text, $tags) == 0) { return $text; } // Discard duplicates and sort them by number $tags = array_unique(array_map('strtolower', $tags[0])); natsort($tags); // Renumber them and replace them $i = $level; foreach ($tags as $tag) { $from[] = '@'. preg_quote($tag) .'(?![0-9])@i'; $from[] = '@'. preg_quote(str_replace('<', '</', $tag)) .'(?![0-9])@i'; $to[] = '<h'. $i; $to[] = '</h'. $i; $i++; } $text = preg_replace($from, $to, $text); // Change level 7 headers and higher to divs. return preg_replace(array('!<h([7-9]|[1-9][0-9]+)!i', '!</h([7-9]|[1-9][0-9]+)!i'), array('<div class="header-\1"', '</div'), $text); } ?> That way you can use header tags safely in the content (which is useful in the context of a single page) without messing up the book output. ------------------------------------------------------------------------ May 7, 2005 - 00:50 : puregin Attachment: http://drupal.org/files/issues/02_book_printer_friendly_div.patch (1.78 KB) The attached patch changes the div id to the form 'node-123'. I was concerned that a document fragment might be created with both a book section and a non-book section from the same node, leading to non-unique id's, but at this point it seems like an unlikely scenario. I've changed 'class=sect1' to 'class=section-1'. I was trying to be lazy and use the DocBook section name convention to make XSLT conversion a little easier, but it's really not saving any effort, in the end, and this is clearer. I think 'section' rather than 'depth' is appropriate - it describes what the role of the div element is. I've closed the other issue regarding id's in header elements, because I've replaced the whole 'class="book-hx"' business in headers with 'class="book-header"'. Since the level can now be selected by context with respect to the enclosing div, separate classes for headers are not required. That is, we can style with CSS like the following: .section-1 h1.book-header { font-weight: bold; font-size: 2.2em; } .section-2 h1.book-header { font-size: 2em; } .section-1 p { text-align: justified; } .section-3 p { margin-left: 3em; margin-right: 3em; } Note that the section-2 h1 will be bold, since the section-2 div is nested within the section-1 div. Also, all paragraphs regardless of section will be justified; and section-3 paragraphs will be indented left and right. Because we can style /everything/ by section, not just headers, and because the styling specifications respect the sectional hierarchy, it's possible to create very elegant and sophisticated book pages with very little CSS. Even I can do it! :) I'm definitely *not* a designer, but I've played around a bit with this - have a look at http://www.puregin.org/book/print/129 for an example. I will attach patches for misc/print.css and misc/drupal.css separately. I don't understand why, but the present version (cvs) of these files has all of the h1.book-hx selectors in drupal.css - the printer-friendly page actually looks at print.css. Am I missing something? For the record, this approach seems to have been suggested by clairem in http://drupal.org/node/8049 where I have a little rant about headings in book pages. Twice :) Djun ------------------------------------------------------------------------ May 7, 2005 - 01:10 : puregin Steven - that's a neat idea, and very clever, clean code! But it still doesn't catch problems such as, for example, an h5 inside of an h1 section with no intervening h2..h4 headers, or? I'm inclined to be a bit suspicious of the approach, though - I rather think it's 'doing magic behind the user's back'. If people are going to insist on putting headings inside of pages, it should be possible. And it would be odd for users to discover that what they typed isn't what they get. ------------------------------------------------------------------------ May 7, 2005 - 01:32 : puregin Attachment: http://drupal.org/files/issues/print_css_div.patch (1.36 KB) Here's the patch for print.css. There's a couple of commented-out lines - these show how to make section headings 'run in' as part of the following paragraph (or other inline element). I am assuming that print.css is supposed to be something of an example that people would use to experiment with? Please feel free to improve this in any way - as I said, I'm far from being a graphic designer, and I'm also not really sure what the intention of this file is. ------------------------------------------------------------------------ May 7, 2005 - 11:40 : Steven Puregin: nope, the code correctly handles missing h numbers. If the book page only uses h2, then that will be the first new level. If it uses h2 and h4, and it is inside a 3rd level book page for example (whose title would be h3), they they get renumbered to respectively h4 and h5 (no missing number). >From a semantic point of view it is crazy not to use h# for book titles. ------------------------------------------------------------------------ May 7, 2005 - 11:44 : Steven A bit more verbose: I think the overwhelming majority of users browse a book on the site itself. Within a single page, there is a h1 tag for the page title. So it makes sense for a writer to use h2, h3, etc inside a single page. This is semantically valid HTML. On the other hand, the print output places the content in a whole new context. I would expect the book module to take whatever steps necessary 'behind the user's back' to ensure it displays correctly. ------------------------------------------------------------------------ May 7, 2005 - 13:43 : puregin Sorry, I replied to the last post on the list - I'll include the post here for the record, and expand a little. Steven, I think that the issue you're addressing is actually a different one than I am trying to fix in my patch, which addresses how sections are wrapped. My patch would still allow users to write headers in their book pages - it would then be up to post-processing - or something like your approach - to deal with how embedded headers are dealt with. Perhaps we should take up your suggestion in another of the book.module issues dealing with the sequences of headers? How about http://drupal.org/node/8049, or http://drupal.org/node/4118? With respect to your suggestion - I understand now that you're proposing to apply the re-writing only to the printer-friendly output. I'm conflicted about this idea - on the one hand, the output which it produces is clearly an improvement over the present output. On the other hand, it changes what the user types to make it 'right' - I guess this is in the long tradition of HTML being interpreted very forgivingly by web-browsers and other applications. On the balance, I support your idea - I guess it's in the spirit of 'make things easier for the user'. With respect to the actual code - I don't think it will handle the case where h2, h3, and h4 are all used in the text, but the sequence h2, h4 occurs. To put it another way, the problem of checking whether headers are in a legal sequence is context sensitive, while a regular-expression search and replace (even if the replacement sequence remaps the targets) is context-free. When I say legal sequence, I mean legal in terms of book hierarchy, not in terms of the XHTML definition, which doesn't address hierarchy, because headers are titles, not sectional elements. SGML/XML publishing DTDs define such things via inclusion/exclusion on true sectional elements) The case I described is, admittedly, a relatively infrequent occurrence, and the general solution would be way to difficult to deal with - it would involve constructing and manipulating a DOM, or something along those lines. Ugh! Djun ------------------------------------------------------------------------ May 18, 2005 - 10:24 : cel4145 The Drupal doc team is very interested in being able to import/export book content. I'm sure that others would be interested in it, too. Djun has indicated that this patch is a good step toward having that functionality So +1 if the developers can help him to get this patch so that it can be implemented. ------------------------------------------------------------------------ June 1, 2005 - 03:37 : puregin This patch is subsumed in my patch for issue http://drupal.org/project/comments/add/1482, which provides for XML export of books. Note that the patch for print.css in this issue still applies with the new patch. ------------------------------------------------------------------------ June 6, 2005 - 02:30 : puregin This issue has been fixed with the application of patch from http://drupal.org/node/1482. However, I'm not sure if the print.css file patch has been commited. Dries? Also, as I mentioned, someone with real page/graphic designer's expertise should probably look over this and tweak/replace with better layout. ------------------------------------------------------------------------ June 6, 2005 - 13:30 : Dries Attachment: http://drupal.org/files/issues/opml.png (39.94 KB) I think this version generates invalid OPML and invalid Docbook code. The Docbook does not seems to validate as XML because sometimes a book-tag is closed with a section-tag. In OPML's case, the section-classes look weird (see screenshot). I'm guessing something is wrong with the computation of the depth. Are the class-sections part of the OPML standard? I'm having a hard time guessing what they are about or why one wants to use these. A couple of code comments: 1. Don't use tabs. Even in absence of tabs, the spacing isn't always 100% consistent -- sometimes two spaces are used. 2. The use of " (double quotes) and ' (single quotes) is sometimes confusing. Only use single quotes when it shortens the code. Often your code can be shortened by using double quotes (and escaping double quotes in the embedded string). Example: $output .= '<div id="node-'.$node->nid. '" class="section-'.$depth.'">'."\n"; 3. Write $depth == 2, not 2 == $depth. ------------------------------------------------------------------------ June 6, 2005 - 17:07 : puregin Attachment: http://drupal.org/files/issues/xml-export-04.patch (23.89 KB) I am now supplying the correct number of parameters to the '_post_node' visitor definitions, so the depth is now passed correctly. This fixes the problems with mismatched tags and strange section depths. The DocBook output for top-level nodes validates against Docbook XML V4.1.2 (using onsgmls). Level 2 and deeper nodes generate Chapters and sections as document fragments and so won't validate per-se. I've checked that they are well-formed, and that they validate on a manual embedding. OPML now validates against the 1.0 DTD (though there seem to be some problems with the DTD itself) I now encode the "id" attribute as string in type attribute of outline for OPML, in order to store this metadata in an existing attribute. The use case I have in mind is someone exporting the book structure (hierarchy) via OPML, editing it and re-importing to modify the structure of the book. I have removed the "class" metadata. Checked for tabs/whitespace - I hope this is now OK. Now using ($depth == 2) rather than (2 == $depth). I've replace single quotes with double where code is simplified by this. ------------------------------------------------------------------------ June 6, 2005 - 17:17 : puregin Attachment: http://drupal.org/files/issues/xml-export-05.patch (23.89 KB) Sorry, ignore the last patch - I goofed with escaping a string. The new patchfile is attached -- Djun ------------------------------------------------------------------------ June 7, 2005 - 12:22 : Dries Committed to HEAD. Great job, Djun. If any outstanding bugs need to be updated, please do.

1 0

[drupal-devel] [bug] user uploads photo to profile -> getimagesize(): Read error! in /includes/image.inc on line 88
by Robin Monks 12 Jun '05

12 Jun '05

Issue status update for http://drupal.org/node/24157 Project: Drupal Version: 4.6.0 Component: other Category: bug reports Priority: normal Assigned to: chx Reported by: lennart Updated by: Robin Monks -Status: active +Status: patch Attachment: http://drupal.org/files/issues/image.exist.patch (1.38 KB) Here is the patch per chx's comments, I made it for him as he didn't have time. He can answer any questions about it. Tested to work on HEAD. Robin Robin Monks Previous comments: ------------------------------------------------------------------------ June 2, 2005 - 13:33 : lennart When a user tries to upload a photo to his or her profile the following error occurs in the log getimagesize(): Read error! in /includes/image.inc on line 88 I am using user.module and profile.module, but NOT image.module and upload.module GD2 is working. The problem is on a fresh 4.6.1 install without any contributed modules I did not know exactly what component to post this under, but I think it is important. ------------------------------------------------------------------------ June 2, 2005 - 16:57 : mariagwyn I am having the identical problem, except that I am using image.module. It happens anytime I click onto my images section. I get about 8 error messages, all the same. It this because images are missing? Any other reason? Thanks, Maria ------------------------------------------------------------------------ June 2, 2005 - 21:47 : rolfijn Me to! I've installed the image-module also and get the same error. I've allready given a 777 permision on my drupal directory, as to make sure it's not a permissions error. When i log into drupal and go to administration -> settings i have the following setting: File system path:files (which resolves to /home/httpd/html/drupal/files, right?) and Temporary directory: tmp (which resolves to /home/httpd/html/drupal/files/tmp, right?). Then going to administration -> settings -> image i see my Default image path: images (which resolves to /home/httpd/html/drupal/files/images, right?) I run trustix secure linux 2.2 with the following (relevant) components installed: apache-2.0.54-3tr apache-html-2.0.54-3tr php-cli-5.0.4-3tr php4-4.3.11-2tr php-pgsql-5.0.4-3tr php-gd-5.0.4-3tr php-imap-5.0.4-3tr php-5.0.4-3tr php4-pgsql-4.3.11-2tr php4-gd-4.3.11-2tr php4-imap-4.3.11-2tr postgresql-8.0.3-1tr postgresql-libs-8.0.3-1tr postgresql-server-8.0.3-1tr gd-2.0.33-3tr gd-utils-2.0.33-3tr I have no idea what the problem can be. Somebody? Thanks in advance, Rolf Deenen ------------------------------------------------------------------------ June 5, 2005 - 20:22 : Dublin Drupaller same problem....drupal 4.6.1. import images...giving a warning: getimagesize(): Read error! Anyone found a solution? Dub ------------------------------------------------------------------------ June 5, 2005 - 20:51 : Dublin Drupaller hi guys... I downloaded the latest image.module from the cvs repository...and it works.. http://cvs.drupal.org/viewcvs/drupal/contributions/modules/image_import/ The only down side is that it is in debug mode (which means a load of extra messages appear)...not sure how to switch that off...but, suffice to say that it looks like the owner/maintainer/developer of the the image_import module is on top of it. best to wait until the module developer updates the module. Dub ------------------------------------------------------------------------ June 5, 2005 - 21:07 : celticess I'm getting the same error but I think it only happens to the one user using the grey box theme. Otherwise I can't find any rhyme or reason to it. Everything seems to be displaying correctly. I have the various image modules installed though not the cvs(I am using drupal 4.6.1). And no pictures have been uploaded to the galleries for it even to give the error. Also I'm getting another include error and one module specific which I have also filed bug reports on. The other include error though is common.inc line 1308 and it seems no real reason for it to be pumping out that code either.(the module error occurs only when I tell it to show browsers in the stats filtering) I'm not exactly a php wiz... haven't programmed since the 80's actually so I have no idea why it's burping out these 3 errors I'm getting. My Drupal 4.6.1 is being run off of a debian site with php4 and mysql. I myself use gentoo linux(thinking about going to BSD) however my isp TOS forbids servers or I'd run it off my own box. ------------------------------------------------------------------------ June 7, 2005 - 02:43 : chueewowee I get the same error message with drupal 4.6.0 even with all content unpublished and all images deleted from the content. The error in all pages even when not creating content. I sue a custom theme, but have not altered anything relevant to images. In fact I am having terrible problems publishing images with the image.module and image_upload.module, in that the galleries do not show in the create pages etc.. as a number of people have reported. ------------------------------------------------------------------------ June 8, 2005 - 00:57 : occy Yeah, I'm getting the same thing. :/ One would think with so many people having the same issue someone would have responded with a fix. I guess maybe I'll try the CVS version. ------------------------------------------------------------------------ June 8, 2005 - 01:31 : celticess I put the cvs module in(I'm not running cvs drupal though just 4.6.1) in and so far that error now seems gone. However the other error I'm still getting and I've got that other annoying image related problem where you've told it to default to not promoting new uploaded images to front page but it still does it. Btw I wonder if the guys realize the directions for patch aren't too clear. I've used linux since '96 and am self taught. Oddly enough never had to use patch as things auto patch on gentoo. And I've now asked friends for plain english explaination of how to do that on my debian host. Why am I mentioning patch? Because if you guys are also having an issue with album someone has put out a patch. ------------------------------------------------------------------------ June 8, 2005 - 17:05 : lennart the problem is not specific to image.module ------------------------------------------------------------------------ June 8, 2005 - 20:17 : chx mage.inc , line 83, it's not file_exists but is_file . I suspect we'd better change theme_image also. Someone should roll a patch. Or I'll do when I have time :)

1 0