April 2005 - Development - lists.drupal.org

[drupal-devel] [task] Remove hard coded HTML for comment moderation
by drumm 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/20103 Project: Drupal Version: cvs Component: comment.module Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: drumm Status: patch Attachment: http://drupal.org/files/issues/comment.module_8.diff (5.34 KB) Removing another place where we have hard coded HTML instead of the form functions. The unpublish selection actually didn't seem to work, so I removed it since we have delete. It should be implemented as a link anyway. drumm

4 5

[drupal-devel] [bug] Make block configuration understandable
by Dries 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/11875 Project: Drupal Version: cvs Component: block.module Category: bug reports Priority: normal Assigned to: killes(a)www.drop.org Reported by: drumm Updated by: Dries Status: patch I'll add this to the release announcement together with the other upgrade issues. Better to centralize everything imho. Dries Previous comments: ------------------------------------------------------------------------ October 21, 2004 - 21:42 : drumm Attachment: http://drupal.org/files/issues/tmp (42.68 KB) The primary goal of this patch is to take the 'custom' and 'path' columns of the block overview page and make them into something understandable. As of Drupal 4.5 'custom' lacked an explanation which wasn't buried in help text and path required dealing with regular expressions. Every block now has a configuration page to control these options. This gives more space to make form controls which do not require a lengthy explanation. This page also gives modules a chance to put their block configuration options in a place that makes sense using new operations in the block hook. The only required changes to modules implementing hook_block() is to be careful about what is returned. Do not return anything if $op is not 'list' or 'view'. Once this change is made, modules will still be compatible with Drupal 4.5. Required changes to core modules are included in this path. An additional optional change to modules is to implement the additional $op options added. 'configure' should return a string containing the configuration form for the block with the appropriate $delta. 'configure save' will come with an additional $edit argument, which will contain the submitted form data for saving. These changes to core modules are also included in this patch. I have posted screenshots at http://foo.delocalizedham.com/tmp/blocks/. Additional work, such as further rearrangement of the block overview page, is as always a task for another patch. ------------------------------------------------------------------------ October 21, 2004 - 23:56 : drumm Attachment: http://drupal.org/files/issues/block_0.diff (42.79 KB) Now with a few more comments. ------------------------------------------------------------------------ October 22, 2004 - 00:14 : drumm Attachment: http://drupal.org/files/issues/blocks.sql (211 bytes) And the DB changes. ------------------------------------------------------------------------ October 23, 2004 - 19:06 : Dries This patch marks a significant usability improvement. Excellent job. I had a quick look at the patch: 1. Taken from the patch: <?php + else if ($op == 'configure' && $delta = 3) { ?> There are a number of '$delta = 3's that probably should be '$delta == 3's. 2. I would use 'save' rather than 'configure save'. 3. block_box_add() is a little weird: 3.a. Is the $bid parameter being used? 3.b. <?php + drupal_set_message(block_box_save($edit)); + drupal_goto('admin/block'); ?> I would move the drupal_set_message() and the drupal_goto() inside block_box_save(), if possible without having to add extra code. 3.c. <?php + $form = block_box_form(); + $form .= form_submit(t('Save block')); + $output .= form($form); ?> Any reason we can't move all form stuff into block_box_form()? 3.d. It would be good if we could get rid of the block vs box thing. 4. We need proper database updates. 5. [?+ php block['pages_show'] = $old_blocks[$module][$delta]->pages_show; + $block['pages'] = $old_blocks[$module][$delta]->pages; ?] What is the difference between 'pages' and 'pages_show'? It is not clear from the names so maybe we should try to come up with more explanatory names. 6. The changes will need to be documented in a new 'Converting modules from 4.5.x to CVS' page. 7. Has the block API Doxygen comments? ------------------------------------------------------------------------ October 23, 2004 - 22:40 : moshe weitzman this is an improvement. one nit: don't use a form_group() around all the form fields on a page. There is no need to group if everything is of the same type. If you need to name that single group of forms, use the page title ------------------------------------------------------------------------ October 23, 2004 - 23:13 : Steven I agree, this is a very nice patch. 1) One thing I do miss is that there is no (longer?) a clear overview of blocks (which blocks are always enabled, which are contextual, ...). However, this is really the fault of the giant configuration table. Reworking that is a different patch. 2) I have a usability issue with block.module which could go in this patch: the description field is really only useful for module-defined blocks where the title is not descriptive. Custom blocks have static titles, so the description is really redundant. I know I never fill it in on my own site, only to curse myself when I see the administration screen and have a bunch of unknown blocks sitting there. So either: - Get rid of the description field for custom blocks and use the title instead. - Keep the description field, but use the block title if the description field was left empty. I prefer the first. 3) Here's a reworked contextual help text for the block admin. The giant paragraph that we have now is not going to be read by anyone: "Blocks are the boxes in the left and right side bars of the web site. They are made available by modules or created manually. Only enabled blocks are shown. You can position the blocks by deciding which side of the page they will show up on (region) and in which order they appear (weight). If you want certain blocks to disable themselves temporarily during high server loads, check the 'Throttle' box. You will need to enable the auto-throttle on the [[throttle configuration page]] after having enabled the throttle module. " 4) The general module help needs work: "If the block\'s throttle checkbox is checked, the throttle level must me low." (me -> be) The paragraph about "A block is visible when:" is also structured oddly. The part about the throttle is confusing and the last option does not match the preceding fragment at all (it starts a new sentence). Maybe it's better to say: " A block's visibility depends on: - Its enabled checkbox. Disabled blocks are never shown. - Its throttle checkbox. Throttled blocks are hidden during high server loads. - Its path options. Blocks can be configured to only show/hide on certain pageS. - User settings. You can choose to let your users decide whether to show/hide certain blocks. - Its function. Dynamic blocks (such as those defined by modules) may be empty on certain pages and will not be shown. " This is a bit vaguer, but shorter and easier to understand. Where the exact options are located is not so important, because they are in a very logical place, and people are bound to run into them when making their own blocks. ------------------------------------------------------------------------ October 24, 2004 - 02:05 : Dries I'm OK with removing the description. ------------------------------------------------------------------------ October 29, 2004 - 02:42 : drumm Attachment: http://drupal.org/files/issues/block_1.diff (44.79 KB) #3: 1. Changed '=' to '=='. 2. Changed 'configure save' to 'save'. 3. Cleaned up block_box_add(), specific code is there because block_box_form() and block_box_save() are reused for editing. The distinction between block and box is useful, boxes are user created blocks, however box might not be the best name. 4. Added database updates for mysql. 5. Pages stores the list of pages for the textarea, pages_show stores whether those are pages to show, or not show, on for the radio buttons. 6. Yes it will. Text can probably be copied from #1. 7. Finishing those up now. I can commit them to contrib cvs myself. #4: It works well on blocks with specific configurations. I expect a limited number of additional options such as block name will be added before 4.6. That will create more groupings. #5: 1. Yes, lets save that for another patch. 2. I have a good idea for the description's needed removal which I plan to work into a patch which allows every block to be renamed. 3. Replaced with the suggested help text. 4. Changed 'me' to 'be'. #6: See #5.2. ------------------------------------------------------------------------ October 29, 2004 - 09:39 : stefan nagtegaal I think - to be more consistent with the rest of drupal - that if the throttle.module is not enabled, you should _not_ display the throttle fields at all. We do this all the time inside drupal, right? I suggest you do something like: <?php - $header = array(t('Block'), t('Enabled'), t('Throttle'), t('Weight'), t('Region'), array('data' => t('Operations'), 'colspan' => 2)); + $header = array(t('Block'), t('Enabled'), (module_exist('throttle') ? t('Throttle')), t('Weight'), t('Region'), array('data' => t('Operations'), 'colspan' => 2)); ?> and: <?php - $rows[] = array($block['info'], array('data' => form_checkbox(NULL, $block['module'] .']['. $block['delta'] .'][status', 1, $block['status']), 'align' => 'center'), array('data' => form_checkbox(NULL, $block['module'] .']['. $block['delta'] .'][throttle', 1, $block['throttle'], NULL, module_exist('throttle') ? NULL : array('disabled' => 'disabled')), 'align' => 'center'), form_weight(NULL, $block['module'] .']['. $block['delta'] .'][weight', $block['weight']), form_radios(NULL, $block['module'] .']['. $block['delta'] .'][region', $block['region'], array(t('left'), t('right'))), l(t('configure'), 'admin/block/configure/'. $block['module'] .'/'. $block['delta']), $operation); + $rows[] = array($block['info'], array('data' => form_checkbox(NULL, $block['module'] .']['. $block['delta'] .'][status', 1, $block['status']), 'align' => 'center'), (module_exist('throttle') ? array('data' => form_checkbox(NULL, $block['module'] .']['. $block['delta'] .'][throttle', 1, $block['throttle'], NULL)), 'align' => 'center'), form_weight(NULL, $block['module'] .']['. $block['delta'] .'][weight', $block['weight']), form_radios(NULL, $block['module'] .']['. $block['delta'] .'][region', $block['region'], array(t('left'), t('right'))), l(t('configure'), 'admin/block/configure/'. $block['module'] .'/'. $block['delta']), $operation); ?> The above patch is untested, and i'm not fully trusted with the fact that it works, but your should get an idea of what I mean.. ------------------------------------------------------------------------ October 29, 2004 - 22:41 : drumm Actually this patch leaves the overview page as is, excpet for removing the columns that are expanded into the configuration page. I believe the standard is to show the throttle configuration, but disabled if not applicable. See the modules administration page. ------------------------------------------------------------------------ October 29, 2004 - 23:15 : Dries I tried the patch: running the update.php script disabled all blocks. Can't look into it right now (lunch) but it is probably an easy fix. user error: Unknown column 'pages_show' in 'field list' query: INSERT INTO blocks (module, delta, status, weight, region, pages_show, pages, custom, throttle) VALUES ('user', '3', 0, 5, 1, 0, '', 0, 1) in /mnt/redhat/foo/cvs/web/drupal.org/cvs/includes/database.mysql.inc on line 125. ------------------------------------------------------------------------ October 30, 2004 - 08:20 : stefan nagtegaal Drumm, I know how the current overview looks and that's why I am asking you to make this fix.. The throttle-checkbox should _not_ be displayed instead of being disabled when the module is not enabled.. If a module is disabled, we don't display their help text and configuration options at all, right? So why are we doing that for the throttle module? Stefan. ------------------------------------------------------------------------ October 31, 2004 - 09:38 : Dries Committed. The UI could do with a tad more polish though, but that can be part of a new patch. Please make sure to update the documentation. ------------------------------------------------------------------------ April 2, 2005 - 20:52 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/update_2.patch (823 bytes) The upgrade simply dropped all the stored regexes. I understand that an upgrade patch was difficult to provide, but we should at least document this. Attached patch does this.

1 0

[drupal-devel] [bug] Cannot delete users
by Dries 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/19750 Project: Drupal Version: cvs Component: user.module Category: bug reports Priority: critical Assigned to: Anonymous Reported by: AtomicMaestro Updated by: Dries Status: patch Could this be fixed properly by making a stand-alone callback for the delete view? Dries Previous comments: ------------------------------------------------------------------------ March 31, 2005 - 11:33 : AtomicMaestro I can't seem to delete users. To reproduce: 1. Create a user. 2. View the user created in 1. 3. Click Delete I would expect to be told I have to block the user first, but instead, I'm sent back to the user report. There's no message, nothing in the log. ------------------------------------------------------------------------ April 1, 2005 - 09:40 : asimmonds Attachment: http://drupal.org/files/issues/fix_delete_user.patch (581 bytes) Patch attached to fix this, the delete code in node.module works the same way ------------------------------------------------------------------------ April 3, 2005 - 16:15 : moshe weitzman when one deletes a user from list of users in admin, he expects to return back to that list after successfully deleting. thats the intent of the $_REQUEST['destination']. I think this patch will defeat that behavior. There certainly is a bug here, but I don't think we have found the right fix yet. I suggest testing a delete from both admin and user/edit pages. make sure you arrive at the expected page after a successful delete. ------------------------------------------------------------------------ April 4, 2005 - 09:19 : asimmonds There are two methods to enter the user edit page as a admin: 1/ View a user, then select the edit tab. 2/ Click 'edit' for the user in the admin/user list. The 2nd method sets destination=admin/user in the URL, when the user edit page is shown. If the Delete button is clicked, it calls drupal_goto("user/$account->uid/delete") (the code's comment mentions this is done to make the tabs disappear). As destination is already set, drupal_goto() redirects to that ie. "admin/user" instead of "user/$account->uid/delete" If after successfully deleting a user, the code calls drupal_goto("admin/user"). I looked at how node.module does the node delete/confirm sequence and it has this unset($_REQUEST['destination']) in there as well. Maybe this delete/confirm sequence pattern needs a re-think for 4.7 ------------------------------------------------------------------------ April 6, 2005 - 06:01 : moshe weitzman Attachment: http://drupal.org/files/issues/user_15.patch (1020 bytes) here is an alternative. this one properly maintains the destination parameter which is useful especially when you are not on the 1st page of your user listing. the redirect insures that you return to the page you were on. another advantage is that i was able to delete a few lines of code which were performing a drupal_goto(). the downside is that the pupose of the drupal_goto() was to get rid of the tabs. I don't mind the tabs on the delete confirm page, so I think my patch is better, and removes a couple lines of code. either patch presented here would work ok. ------------------------------------------------------------------------ April 6, 2005 - 07:13 : moshe weitzman Attachment: http://drupal.org/files/issues/menu_set_location.patch (2.23 KB) here is a different patch suggested by UnConed. Uses menu_set_location() to remove the tabs on the delete confirmation page for both nodes and users. ------------------------------------------------------------------------ April 6, 2005 - 21:28 : Dries Mmm, Steven's patch fixes the tabs but still is a bit of a hack. (Also, parts of the strings can't be translated, the use of arg(1) vs $edit['nid'] looks a bit weird.) Can't we fix this properly?

1 0

[drupal-devel] [bug] have db_prefix_tables() handle 'default' key not being set
by moshe weitzman 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/19609 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: danielc Updated by: moshe weitzman Status: patch docs in the administrators guide - http://drupal.org/node/2622 moshe weitzman Previous comments: ------------------------------------------------------------------------ March 28, 2005 - 16:24 : danielc Attachment: http://drupal.org/files/issues/database.diff (720 bytes) The db_prefix_tables() function in includes/database.inc generates notices if $db_prefix['default'] is not set. This patch fixes that problem. This has a side benefit of making $db_prefix easier to use. ------------------------------------------------------------------------ March 28, 2005 - 17:06 : moshe weitzman nice ... this has been a bit a pimple for a while. ------------------------------------------------------------------------ March 29, 2005 - 10:40 : danielc Attachment: http://drupal.org/files/issues/database2.diff (1.17 KB) I've come up with a new version of the patch that optimizes the speed of the whole function. ------------------------------------------------------------------------ March 29, 2005 - 10:56 : danielc Change Version from "4.5.2" to "cvs". ------------------------------------------------------------------------ March 29, 2005 - 11:01 : danielc Pardon me, I'm testing if this goes the drupal-dev list now that the version is "cvs." ------------------------------------------------------------------------ April 3, 2005 - 07:45 : Dries Any idea of how much faster the second patch is? I don't think it makes a real difference in which case I'd favor the first patch. ------------------------------------------------------------------------ April 3, 2005 - 13:40 : danielc The speed increase is 5%. ------------------------------------------------------------------------ April 3, 2005 - 13:52 : Dries 5% compared to what? What did you measure? ------------------------------------------------------------------------ April 3, 2005 - 23:56 : danielc I tested the db_prefix_tables() function in two configurations. With my first patch applied and with my second patch applied. The second patch is 5% faster than the first. This improvement is the same for both the string and array formats of $db_url. ------------------------------------------------------------------------ April 7, 2005 - 19:43 : danielc Can this please be applied to 4-6 and HEAD? ------------------------------------------------------------------------ April 8, 2005 - 06:34 : Dries I was checking settings.php to read up on how to use $db_prefix but noticed that only the 'simple' case was documented. The fact that $db_prefix can be an array isn't explained anywhere (not in the phpdoc of db_prefix_tables() and not in settings.php). Where did that go? I wanted to check what the documentation had to say on the availability of a 'default' entry ...

1 0

[drupal-devel] [bug] have db_prefix_tables() handle 'default' key not being set
by Dries 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/19609 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: danielc Updated by: Dries Status: patch I was checking settings.php to read up on how to use $db_prefix but noticed that only the 'simple' case was documented. The fact that $db_prefix can be an array isn't explained anywhere (not in the phpdoc of db_prefix_tables() and not in settings.php). Where did that go? I wanted to check what the documentation had to say on the availability of a 'default' entry ... Dries Previous comments: ------------------------------------------------------------------------ March 28, 2005 - 23:24 : danielc Attachment: http://drupal.org/files/issues/database.diff (720 bytes) The db_prefix_tables() function in includes/database.inc generates notices if $db_prefix['default'] is not set. This patch fixes that problem. This has a side benefit of making $db_prefix easier to use. ------------------------------------------------------------------------ March 29, 2005 - 00:06 : moshe weitzman nice ... this has been a bit a pimple for a while. ------------------------------------------------------------------------ March 29, 2005 - 17:40 : danielc Attachment: http://drupal.org/files/issues/database2.diff (1.17 KB) I've come up with a new version of the patch that optimizes the speed of the whole function. ------------------------------------------------------------------------ March 29, 2005 - 17:56 : danielc Change Version from "4.5.2" to "cvs". ------------------------------------------------------------------------ March 29, 2005 - 18:01 : danielc Pardon me, I'm testing if this goes the drupal-dev list now that the version is "cvs." ------------------------------------------------------------------------ April 3, 2005 - 14:45 : Dries Any idea of how much faster the second patch is? I don't think it makes a real difference in which case I'd favor the first patch. ------------------------------------------------------------------------ April 3, 2005 - 20:40 : danielc The speed increase is 5%. ------------------------------------------------------------------------ April 3, 2005 - 20:52 : Dries 5% compared to what? What did you measure? ------------------------------------------------------------------------ April 4, 2005 - 06:56 : danielc I tested the db_prefix_tables() function in two configurations. With my first patch applied and with my second patch applied. The second patch is 5% faster than the first. This improvement is the same for both the string and array formats of $db_url. ------------------------------------------------------------------------ April 8, 2005 - 02:43 : danielc Can this please be applied to 4-6 and HEAD?

1 0

[drupal-devel] [bug] conf_init should strip out port numbers in bootstrap.inc
by Dries 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/19934 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: chx Reported by: bwooster47 Updated by: Dries Status: patch Won't commit. Removing the port seems like a bad idea (I could have two drupal sites at two different ports but otherwise identical URLs) and adding regex' all over the map isn't advised. If ":" is an invalid character, then there might be more such characters. I suggest we replace them with '.' and that we document this behavior much like we do with $db_url. Dries Previous comments: ------------------------------------------------------------------------ April 4, 2005 - 01:07 : bwooster47 The latest Drupal from CVS (4.6.0) offers multisite configuration files, but currently the conf_init function in includes/bootstrap.inc will also include the :portnum URL specification in the directory name, which does not seem right (and the colon character may cause problems on Mac/Windows). Snippet from bootstrap.inc: * Example for a fictitious site installed at * http://www.drupal.org/mysite/test/ the 'settings.php' is * searched in the following directories: ..... If the site is: http://www.drupal.org:8080/mysite/test/ the same list of directories should be searched as listed in the example in the file, and the :8080 should not be added to the directory name (I think...) I don't know enough PHP to submit a patch, but this is probably easy for those who know this stuff :-) ------------------------------------------------------------------------ April 8, 2005 - 09:08 : chx Attachment: http://drupal.org/files/issues/conf_init_ports.patch (768 bytes) Here you are.

1 0

[drupal-devel] [bug] Private downloads aren't working
by stefan nagtegaal 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/12267 Project: Drupal Version: cvs Component: file system Category: bug reports Priority: critical Assigned to: Anonymous Reported by: stefan nagtegaal Updated by: stefan nagtegaal -Status: active +Status: patch This is still an issue in 4.6 RC and can still be reproduced by follwing exactly what I propose above.. While this is a longstanding bug, and with drupal 4.6 I'll changed status to patch.. Maybe some other people ould have a look at this? stefan nagtegaal Previous comments: ------------------------------------------------------------------------ October 29, 2004 - 11:37 : stefan nagtegaal The private downloads are not working properly.. How to reproduce: Goto 'http://www.your-site.com/admin/settings' and set the 'Download method' to 'Private - files are transferred by Drupal'. Now, you make a new item (blog/story/page/poll, whatever) and upload an image. You should use this image inside your post, by doing an '' inside your post, where $path is the url which is given in the 'Attachments' form-group on the submission page. (Something like 'system/files?file=$yourfile.ext') Then click 'Preview' and do not submit the node! Now, have a look what we see.. We see that the image inside the post isn't displayed properly. The image is broken. If we ask the properties of the image it points to the right location ('system/files?file=$yourfile.ext'), but isn't displayed. If we take a look at the properties of the table under the node (attached files table with 'Attachement', and 'List') we see that the attachment is pointing to something like 'http://www.frontaal-online.com/$file-system-path/$yourfile.ext' instead of the private download url.. I'm not sure how to fix this, but I think it's a bug inside function file_create_url() which could be found inside file.inc. ------------------------------------------------------------------------ October 29, 2004 - 11:51 : stefan nagtegaal Attachment: http://drupal.org/files/issues/upload-and-body.png (19.87 KB) Screenshot after upload en filling in the body.form ------------------------------------------------------------------------ October 29, 2004 - 11:52 : stefan nagtegaal Attachment: http://drupal.org/files/issues/node-preview-and-image-properties.png (78.18 KB) Now we are previewing the node with a broken image in it.. See the properties of the image, which points to another location as the 'metallic.png' hyperlink inside the 'Attachment'-table under the node preview. ------------------------------------------------------------------------ October 29, 2004 - 11:54 : stefan nagtegaal Attachment: http://drupal.org/files/issues/properties-of-metallic-link-in-attachments-t… (53.52 KB) The properties of the 'metallic.png'-link inside the table under the node preview differs from where the source points us to.. The url inside the table works, (which is the same as in public_dowloads), but the url inside the body isn't... ------------------------------------------------------------------------ October 29, 2004 - 12:47 : stefan nagtegaal This bug is also appearing inside drupal 4.5... ------------------------------------------------------------------------ January 28, 2005 - 18:04 : walkah Attachment: http://drupal.org/files/issues/upload-private-files.patch (1.88 KB) here's a small patch to fix. basically the custom url rewriting (for when files haven't been yet saved to their final location) wasn't taking into account FILE_DOWNLOADS_PRIVATE. this fixes that (oh, and it also fixes an sql error in upload_download as well). ------------------------------------------------------------------------ January 28, 2005 - 18:07 : walkah Attachment: http://drupal.org/files/issues/upload-4-5.patch (1.36 KB) here's a version for 4.5 as well ... (where the SQL wasn't broken). ------------------------------------------------------------------------ February 5, 2005 - 09:34 : Steven Isn't that SQL 'error' there to make query rewriting work? ------------------------------------------------------------------------ February 5, 2005 - 09:36 : Steven By the way, I still think we should alter the autop filter so it uses & rather than making this one use a numbered entity. 'amp' is the most common notation out there, and it's shorter too. ------------------------------------------------------------------------ March 13, 2005 - 20:14 : killes(a)www.drop.org Doesn't apply anymore. Is this still an issue? ------------------------------------------------------------------------ April 7, 2005 - 03:56 : moshe weitzman this one is marked critical. Walkah, Stefan - please comment on whether this is still a bug. ------------------------------------------------------------------------ April 8, 2005 - 02:13 : Shane Birley This might point some people in the correct direction. It isn't completely the same issue, but related, I would think and might need to be fixed prior. http://drupal.org/node/17364

1 0

[drupal-devel] [bug] update.inc is a bit broken
by chx 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/20172 Project: Drupal Version: cvs Component: database system Category: bug reports Priority: normal Assigned to: chx Reported by: chx Updated by: chx Status: patch Attachment: http://drupal.org/files/issues/update_3.patch (1.13 KB) I do not know why but update_sql() is not the last function anymore. Let's move it... chx

2 1

[drupal-devel] [bug] Container lost when editing a forum
by chx 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/19615 Project: Drupal Version: cvs Component: forum.module Category: bug reports -Priority: normal +Priority: critical -Assigned to: Anonymous +Assigned to: chx Reported by: Anonymous Updated by: chx -Status: active +Status: patch Attachment: http://drupal.org/files/issues/forum_edit.patch (1.04 KB) It seems that we are handing an array of taxonomy objects as parent instead of an integer as we should. This happens on all editing not just right after creation. chx Previous comments: ------------------------------------------------------------------------ March 28, 2005 - 23:20 : Anonymous To reproduce: 1. Create a forum container 2. Create a forum as a child of that container 3. Edit the forum created in 2. You will see that the Parent dropdown has been set back to .

1 0

[drupal-devel] [bug] form_checkboxes saves "no selection" as string
by chx 08 Apr '05

08 Apr '05

Issue status update for http://drupal.org/node/18663 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: chx Reported by: killes(a)www.drop.org Updated by: chx Status: patch Attachment: http://drupal.org/files/issues/form_checkboxes_2.patch (1.9 KB) killes did not like the previous version. chx Previous comments: ------------------------------------------------------------------------ March 10, 2005 - 02:51 : killes(a)www.drop.org If you select nothing from a set of checkboxes an empty string is save to the variables table. This is a problem if the calling code assumes, it would get an array. We should make it save an empty array. The problem is the form_hidden field that we add. even if we replace form_hidden($name, 0) by form_hidden($name, array()) it isn't going to work, because the empty array is eaten by htmlspecialchars. redLED found this bug and he does not like it: 02:53 < redLED> i just like things saying 'CRITICAL' in red 02:53 < redLED> it gives this warm impending doom feeling 02:53 < redLED> and is generally associated with nuclear reactors in people's subconsciousness Suggestions welcome. ------------------------------------------------------------------------ March 12, 2005 - 19:47 : Steven There is no way to fix it in form_checkboxes() itself afaik, because even without htmlspecialchars, it would still come out as "Array". You can only create arrays indirectly through form submissions, by naming your values "foo[]". But you can not create empty arrays this way. So it is really up to the module which uses form_checkboxes() to handle this. A simple is_array() should do the trick. ------------------------------------------------------------------------ March 28, 2005 - 17:27 : chx Attachment: http://drupal.org/files/issues/checkboxes.patch (1.71 KB) system_settings_save can not really cope with this situation. This makes a problem with empty node options. And the fact that we have a check everywhere we use form_checkboxes is error prone, too. Here is a patch. If this accepted I'll comb the core for dead code. For eg. the first three lines of taxonomy_save_vocabulary and so. ------------------------------------------------------------------------ March 28, 2005 - 17:32 : chx Attachment: http://drupal.org/files/issues/checkboxes_0.patch (1.71 KB) Oh NO! One strayed space! I fixed it before steven beheads me.... ------------------------------------------------------------------------ March 28, 2005 - 18:36 : chx Attachment: http://drupal.org/files/issues/checkboxes_1.patch (1.64 KB) Among other changes, I have moved the hidden field form_checkboxes[] out of $edit. I have included HTML code, yes, but this is a very special thing, so it does not merit changing form_input just 'cos of this. And you won't need to theme a hidden field...

1 0