- Development - lists.drupal.org

[drupal-devel] [bug] Handle full URLs in url() and l()
by robertDouglass 08 Sep '05

08 Sep '05

Issue status update for http://drupal.org/node/10888 Post a follow up: http://drupal.org/project/comments/add/10888 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Bèr Kessels Reported by: jhriggs Updated by: robertDouglass Status: patch (code needs work) Good - that needs to be removed too, but is there anything that doesn't work right if we don't remove them? Lots of the $output = '<a href....' instances in the code use url() for href and these are not broken. They can't be seen as a reason not to commit this particular patch. If anyone point out things that break with the patch I made, I'll fix them. I can't go through and clean up anchor tags on idealistic grounds at the moment (as much as I would otherwise like to). robertDouglass Previous comments: ------------------------------------------------------------------------ Thu, 16 Sep 2004 15:01:24 +0000 : jhriggs Attachment: http://drupal.org/files/issues/full_url.patch (1.97 KB) This little patch allows full URLs to be used for the url() and l() functions, giving modules a single interface for creating links. Currently, modules must use l() for drupal links and raw HTML for external links. This patch changes url() to handle -- basically pass through -- full URLs while still doing the proper handling of internal paths and works with clean URLs enabled or disabled. It also shortens (and hopefully simplifies) the url() code a bit. Some examples of calls that now work: <?php l('a link', 'some/path'); // same as before l('an external link', 'http://drupal.org/'); l('full with query', 'http://google.com/search?q=define:foo'); l('full with separate query', 'http://google.com/search', array(), 'q=define:foo'); // same link as above l('full with fragment', 'http://drupal.org/node/5942#comment-12374'); l('full with separate fragment', 'http://drupal.org/node/5942', array(), NULL, 'comment-12374'); // same link as above ?> ------------------------------------------------------------------------ Thu, 30 Sep 2004 11:09:30 +0000 : moshe weitzman +1. less HTML in code is a good thing for readability, if nothing else. ------------------------------------------------------------------------ Mon, 24 Jan 2005 20:06:26 +0000 : mathias Attachment: http://drupal.org/files/issues/external_urls.patch (838 bytes) Revisiting this issue one year later shows we're almost there. The only thing that's left is the ability to handle external urls when clean_urls is disabled. Once implemented, menu.module can happily accept external urls and not just Drupal paths, making the menu system much more attractive to end users. I'll update the menu.module docs if this patch (or an alternative solution) is accepted. ------------------------------------------------------------------------ Sun, 13 Mar 2005 22:36:34 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/external_urls_0.patch (909 bytes) Updated from Drupal root directory. ------------------------------------------------------------------------ Mon, 14 Mar 2005 00:05:17 +0000 : gordon +1 This is a good thing for people who can't using clean urls. ------------------------------------------------------------------------ Sat, 02 Apr 2005 06:23:10 +0000 : TDobes This patch seems like a good idea, but does not apply anymore. Also, shouldn't the comments for l() and url() be updated to indicate that the functions will work with both internal and external URL's? ------------------------------------------------------------------------ Sat, 02 Apr 2005 07:25:13 +0000 : asimmonds Attachment: http://drupal.org/files/issues/external_urls_1.patch (897 bytes) The &'s had been changed to & amp; , here's a valid patch ------------------------------------------------------------------------ Thu, 12 May 2005 14:57:07 +0000 : adrian This is a new patch against cvs that allows url() to create external links, with the side-effect that menu can now be used to make external links. Also closes http://drupal.org/node/10177 This will be even more important with the primary links menu integration. (This patch already cleans up a special case in phptemplate for that). Killes benchmarked this, and it turns out the difference is negligible. Requests per second: 1.33 [#/sec] (mean) = with patch Requests per second: 1.31 [#/sec] (mean) = without patch. ------------------------------------------------------------------------ Thu, 12 May 2005 14:59:57 +0000 : killes(a)www.drop.org I had benchmarked /node as anon user without cache. ------------------------------------------------------------------------ Wed, 25 May 2005 04:50:08 +0000 : Steven +1 on the idea of this patch. It allows people to use l() for external links. However, I don't like the implementation. Running that complicated regexp for every url() call is simply wasteful. 95% or more of url() calls have a fixed string as parameter and point to Drupal's own paths. I suggest we create an url_external() wrapper around url() which allows external URLs. Any path which we want to allow external URLs in can use url_external() instead. The places where we use it will not be many anyway. ------------------------------------------------------------------------ Wed, 25 May 2005 05:05:55 +0000 : gordon splitting into url() url_external is not a good idea. The main reason for this patch that I see is for putting external urls into the navigation menu using the menu.module, and if all goes to plan, the primary and secondary menus. ------------------------------------------------------------------------ Wed, 25 May 2005 06:00:01 +0000 : Steven gordon: I still think this is a bit excessive. Perhaps instead of valid_url() we could use a simpler regexp !^[a-z]://!i or even strpos('://'). ------------------------------------------------------------------------ Wed, 25 May 2005 06:15:36 +0000 : gordon I am not says that it is or is not excessive. I was just saying that this is where it is going to be used, so creating url() and url_external() is not really an option ------------------------------------------------------------------------ Wed, 25 May 2005 06:57:53 +0000 : Bèr Kessels we /do/ need an external_url() thing. The links bundle (handling weblinks and the likes) willprovide this /and/ need this. People will get all sorts of tools to do /stuff/ with external links, like add permissions, track clicks etc. depending on the modules they have and the settings. All outgoing weblinks, should somehow respect those. Patches for core are coming, though. On top of that, a external_l() finction is simple. And if people want them in primary links they can do the regexping /before/ the function, not /in/ the function. The calling code can decide whether to call l() or external_l(), instead of putting logic in l() that is unneded 95% of the times. But i hereby promise to introduce such a function tbefore the end of this week :) ------------------------------------------------------------------------ Wed, 25 May 2005 07:17:47 +0000 : gordon The only thing I do not see in this plan is how with this work with the menu module and the navigation menu. ------------------------------------------------------------------------ Wed, 25 May 2005 10:04:12 +0000 : adrian Attachment: http://drupal.org/files/issues/url_external_links.patch (1.75 KB) What happened to the patch i uploaded ? It doesn't use a complex regexp, but uses strpos($url, '://') < 5. ------------------------------------------------------------------------ Thu, 26 May 2005 06:19:32 +0000 : Bèr Kessels Allthough this works, is clean AND does what it should do, I still am in favour of a better nitegrated solution. So please do not yet commit this, untill I upload my patch. We can then discuss what solution to take. It will not differ a big lot though. ------------------------------------------------------------------------ Thu, 26 May 2005 08:39:32 +0000 : adrian is this the patch where you want to introduce an el() to allow for counting of external links ? That's a different patch altogether. This is primarily about stopping url() from not handling external links when there is no good reason for it to do so. Your el() has a very specific use in the weblinks api, and should probably be included with it. ------------------------------------------------------------------------ Thu, 26 May 2005 16:18:31 +0000 : Bèr Kessels Yes, that el() But let me explain why i am anxious for my solution: el(), by default will not do much, in fact it will just be an URL()-clone but one that allows outgoing links. But it has a hook-caller. Again: by default it does nothing much, for there will not be any hooks around in core. It will allow, for example, weblinks, to track clicks, or to look up a permission and allow or disallow users to follow a link. Now, the whole issue, IMO is, that we should *not* provide two ways for handling external URLS, but rather a single one, that is weblink-safe. If we would introduce URL() with outgoing links-capabilities, we can no longer track any outgoing URLs trough them. We will then have two ways for handling outgoing URLs, resulting in inconsistencies, and confused users. (hey, my anonymous users can follow link foo, while i have "follow outgig links" permission disabled for them). (BTW weblink will soon be renamed into links, just for reference) ------------------------------------------------------------------------ Thu, 30 Jun 2005 11:06:11 +0000 : adrian Attachment: http://drupal.org/files/issues/url_external_links_0.patch (824 bytes) I've updated the patch. ------------------------------------------------------------------------ Mon, 18 Jul 2005 14:12:36 +0000 : adrian spoke to dries about this patch, and apart from the incomplete patch I uploaded, there are about a dozen instances where in-line html is used were l() would suffice. I will be cleaning them up and submitting a new patch soon. ------------------------------------------------------------------------ Fri, 05 Aug 2005 18:44:03 +0000 : moshe weitzman hopefully adrian or someone else will chase down the remaining urls which should use this function. ------------------------------------------------------------------------ Fri, 05 Aug 2005 19:25:08 +0000 : jsloan I've come to this party late but would like to see a patch soon... I've used the patch from Adrian (#15) but this did not work correctly in my setup (IIS 5.0 / no clean urls) I applied it against 4.6.2 and cvs. Where is the latest patch that I can test ... I'll throw some time at this because I need it for a module used within our Intranet. ------------------------------------------------------------------------ Mon, 08 Aug 2005 20:06:03 +0000 : jsloan Attachment: http://drupal.org/files/issues/external_urls_2.patch (3.67 KB) I'm not sure what the final plans are for this in the CVS so I have developed a patch for use on 4.6.2 I decided to add a parameter named $external that is passed to url() function. The default is FALSE so this should not break any existing calls to either l() or url(). I have also refactored url() so that it is much easier to understand and extend --- I'm thinking of Bèr Kessels intention to "allow, for example, weblinks, to track clicks, or to look up a permission and allow or disallow users to follow a link" " Since the parameter is passed to activate the external url there is no need for regex or strpos, this should be handled by the calling function and then set the $external parameter to TRUE For example I have patched the theme_menu_item_link() function in menu.inc to accept external urls: <?php function theme_menu_item_link($item, $link_item) { // this next line was added so that a menu item could point to the default home $link_item['path'] = preg_replace('/^\/$/','',$link_item['path']); // this next line was added so that a menu item could point to an external url $external = (preg_match('<^[a-z]+://>', $link_item['path'])) ? TRUE : FALSE ; return l($item['title'], $link_item['path'], array_key_exists('description', $item) ? array('title' => $items['description']) : array(),NULL,NULL,FALSE,FALSE,$external); } ?> I hope that this patch will spur some solutions - I'm using this now in production but have not tested extensively across every scenario. thanks jim sloan ------------------------------------------------------------------------ Fri, 19 Aug 2005 12:10:50 +0000 : jsloan ... are there any thoughts on this? What is the status of the final patch? ------------------------------------------------------------------------ Sun, 28 Aug 2005 15:48:08 +0000 : adrian The problem with this patch is that we still need to fix all the inline occurrences of the <a href tag. I don't think we should confuse the issue and implement a hook at this point, as url() breaking with external links is a bug, pure and simple. ------------------------------------------------------------------------ Thu, 08 Sep 2005 15:24:16 +0000 : robertDouglass Attachment: http://drupal.org/files/issues/theme.inc.patch.txt (1.16 KB) Adrian, your second patch for theme.inc looks wrong to me. Don't you still need the line $settings[$type .'_links'][] = l($text, $link, $attributes); ? I also got rid of unset($attributes); Where was that supposed to come from that it needed unsetting? ------------------------------------------------------------------------ Thu, 08 Sep 2005 15:48:02 +0000 : robertDouglass Attachment: http://drupal.org/files/issues/theme.common.inc.patch.txt (1.83 KB) Here is a unified patch that I updated to fix a bug in Adrian's common.inc code (everything was being handled as absolute), plus the update to theme.inc that I questioned in the previous post. In my initial testing, this works perfectly. I can put absolute URLs in for the paths in menu items and it handles them fine, and changes no other behavior. I searched core for all uses of url() and http:// and found no more violating code such as that in theme.inc. Net change in core with this patch: 0 lines of code. Please review quickly, I'm writing in the upcoming book about this functionality as if it is in. =) ------------------------------------------------------------------------ Thu, 08 Sep 2005 15:49:43 +0000 : robertDouglass Correction: net change to core -1 line of code. ------------------------------------------------------------------------ Thu, 08 Sep 2005 18:58:01 +0000 : adrian What needs to be removed from core are not uses of url() and l(), but all occurences of the anchor tag. The anchor tag was mostly used because l() didn't do external links. Also, in the case of phptemplate , that is the line that creates the array that is passed to the template.

1 0

[drupal-devel] [task] Improvments to Filter UI
by m3avrck 08 Sep '05

08 Sep '05

Issue status update for http://drupal.org/node/27364 Post a follow up: http://drupal.org/project/comments/add/27364 Project: Drupal Version: cvs Component: filter.module Category: tasks Priority: normal Assigned to: Bèr Kessels Reported by: Bèr Kessels Updated by: m3avrck Status: patch (code needs work) Found a few more issues: If I go into 'Configure' for 'PHP Code' and click the 'Configure' tab at the top and then click 'list filters' link in the text, I get this warning with PHP5 warning: Missing argument 1 for filter_admin_format() in \drupal\modules\filter.module on line 378. Also, it is sort of confusing as to what the 'List' tab implies. For example, if I click on 'input formats' in the admin menu, I get a list of the current filters. When I click 'configure' for one of those, I see the options for that filter. However, it says 'list' in one of the tab, which doesn't really mean much. I thought that was the list of filters, not the options for that filter. That should be cleaned up. Also, there is no way to get back to the full list of filters unless you click on 'input formats' in the admin menu. A setting/link to fix this would be great. Also, where is the link to add filters??? I think the options/layout/tabs should mimic of that of admin > users. So on admin > input formats, it has the tabs 'list', 'add format' ... very clear and consistent. On a configure page for a filter, it should have 'list', 'view', 'configure', 'rearrange' ... this would make it easier to navigate and get back to the original list of filters. Make sense? I think this and Dries' comments put this patch over the top :) m3avrck Previous comments: ------------------------------------------------------------------------ Sun, 24 Jul 2005 09:30:54 +0000 : Bèr Kessels This is a mockup for the Filter UI. Currently it is soo complex, that I dare to call it broken. Please comment on this, for I will not ake any patches, when the general idea is disliked. The idea is t split filters and input formats better. Filters are filters, defined in modules. Input formats are bundles of filters. This is how we have it ATM, but the interface fails to communicate that. I tried to develop a consistent (with the rest of Drupal) interfce that makes it clearer what is what. The menu is as follows: * administer ** .... ** settings *** input formats *** filters ** ... ------------------------------------------------------------------------ Sun, 24 Jul 2005 09:37:10 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/input-formats_list1.png (16.34 KB) * administer ** .... ** settings *** input formats >> see attachement 1 *** filters ** ------------------------------------------------------------------------ Sun, 24 Jul 2005 09:39:02 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/input-formats_add.png (23.42 KB) * administer ** .... ** settings *** input formats >> see attachement 2, tab 2 *** filters ** Note: The default checkbox lmay seem a bit odd. But checking it will remove the "default status" from the current "default" one and make this one "default". The help text, and a drupal_set_message should explain this. ------------------------------------------------------------------------ Sun, 24 Jul 2005 09:40:08 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/input-formats_edit_format.png (22.17 KB) * administer ** .... ** settings *** input formats >> clicking a 'configure' link in the table of attachement 1 *** filters ** ------------------------------------------------------------------------ Sun, 24 Jul 2005 09:40:55 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/input-filters_list.png (36.24 KB) * administer ** .... ** settings *** input formats *** filters >> see attachement 4 ** ------------------------------------------------------------------------ Sun, 24 Jul 2005 09:41:40 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/input-formats_edit.png (22.63 KB) * administer ** .... ** settings *** input formats *** filters >> clicking a 'configure' link in the table of attachement 4 ** ------------------------------------------------------------------------ Mon, 25 Jul 2005 14:46:27 +0000 : Bèr Kessels IMO this makes the interface easier. But other disagree. And because it also removes one feature: (being able to use E.g. HTML in different input formats, with different settings) I'll simply wont fix this :( ------------------------------------------------------------------------ Thu, 28 Jul 2005 13:36:12 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/filter_module_UI_consistancy.patch (10.28 KB) I decided to go for it anyway. Be it in a slightly different way then my initial mockups. The difference is that I left the filters where they are, hidden beneath the formats. So, here is the patch that makes the filter UI more consistent with screens like blocs, menus, vocabularies et al also nice to note is that: 93 lines are added, 104 are removed. So netto we have less code :) (cvs diff filter.module | grep ^+ | wc -l) ------------------------------------------------------------------------ Thu, 28 Jul 2005 13:37:02 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/input-formats_list_html.png (44.64 KB) Here is how the listing now looks. ------------------------------------------------------------------------ Thu, 28 Jul 2005 13:37:57 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/input-formats_edit_format_html.png (113.71 KB) And the "configure" screen. The 'add' screen looks exactly the same, only with the default values pre-filled. ------------------------------------------------------------------------ Thu, 28 Jul 2005 13:39:19 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/input-formats_edit_format_too_long.png (121.82 KB) Oh, And here is a really nice example of why the current screen is no good. And here I "only" have four roles. I run a site with 12 roles, imagine this screen then! ------------------------------------------------------------------------ Thu, 28 Jul 2005 13:48:35 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/filter_module_UI_consistancy_HEAD.patch (10.24 KB) And here is a pathc for HEAD (I forgot that I developed this on a 4.6.2 codebase. Sorry) ------------------------------------------------------------------------ Thu, 28 Jul 2005 13:55:25 +0000 : stefan nagtegaal I like this very, very much! This is - indeed - much better than the current UI. This always fits, and makes the life of people who use a fixed width theme much nicer.. Ber, FYI (you probably know this too), 'admin/access' does also break fixed width themes due to the fact that it expands the width of the table when more roles are being added.. Good catch! I like it much more this way.. +1 from me... ------------------------------------------------------------------------ Fri, 29 Jul 2005 07:27:34 +0000 : Dries The code style needs work (spaces, tabs, 'as' -> 'AS'), the code comments need work, and debug statements should be removed. Screenshots look good to me. db_query('UPDATE {filter_formats} SET cache = %d, name = \'%s\', roles = \'%s\' WHERE format = %d', (int)$cache, $name, $roles, $format); can be written better/shorter quote-wise. ------------------------------------------------------------------------ Fri, 29 Jul 2005 09:04:16 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/filter_module_UI_consistancy_HEAD_0.patch (10.37 KB) Dries, thanks a lot for the review. OI fixed your comments. Or so I hope. code-style.pl was of no use (can that *please* be removed from core?) because it chokes on the help texts. It also pointed out a lot of old formatting isues, whic, when fixed wuold flood this patch with unrelated fixes. I tried to narow my style-searching to the areas I touched. And I made the comments more verbous. The query is fixed. it saves no characters, but looks better now. Thanks for the tip. The print_r..... blush. thats what I get for not running my default grep -r print_r on my code after finishing up... I could not find a lot of spaces and tabs. So it lmight be that i missed one. I triple checked it though. ------------------------------------------------------------------------ Fri, 29 Jul 2005 09:32:21 +0000 : Steven Moving the role settings inward does fix the wide-table problem, but it makes the filter configuration much more opaque. We could perhaps fix this by showing the roles as a comma-separate list in the input formats table. Such a list can wrap and will not stretch the table. Other than that: * Having the instructions for the Roles and Filters form_group() at the bottom is a bit weird. The distance is too big. * The table that was used for toggling filters on/off was much more compact before, and IMO a lot clearer. Why change to a flat list with the description staggered below each item? ------------------------------------------------------------------------ Fri, 29 Jul 2005 10:31:30 +0000 : Bèr Kessels consistancy, consistancy and more consistancy. There really is nothing worse that having a different 'concept' of UI for every thing in Drupal. We are going in the right direction, with the blocks being editable objects, menus acting like that, taxonomy, users, etc.. I wanted to make this act similar. I am not at all happy with the fact tah the list is still a form. But we need this, for setting the default. A big -1 for a comma-separate list. That is extremely hackish, even worse useabilitywise and just silly * we should avoid typing when not needed. * we should avoid errors on input, when they can be avoided (a typo is made very easy, I know all about it) Form set errors won't fix that usability -wise. I agree that you sort-of loose the overview. But IMO that is a minor loss comared to what we gain, usability-wise. The wtoo-wide was onlywhat triggered me. But do not think that I am noly making this patch to fix that issue. This patch is there to improve usability, through consistancy. o make drupal behave the same in most places in admin. If you know one screen, you know most of them. We really should move away from custom-hacks for every page, because that is useability rule #1: consistancy. Stadardisation, even if sometimes a custom hack might make things easier in that single case, it will reduce your overall usability so much that nearly ever it is worth that hack. That select-boxes instead of the table issue: consistancy. As well that it completely broke fluid CSS layouts, my solution is more consistant, and uses forms the way they are meant. If we cannot use $descritption in checkboxes they should be removed. but IMO it feels much more consistant. Tables are for tabular data, and this is simply a list of items, with additional data. And i do not really get what you mean with "too far down". Do you mean that you would like to see the order of the groups different? ------------------------------------------------------------------------ Fri, 29 Jul 2005 13:31:24 +0000 : Dries I haven't tested Ber's patch yet but looking at the screenshots, it like what I see. I remember being confused by the current filter UI, and often, I still am. ------------------------------------------------------------------------ Fri, 29 Jul 2005 19:53:26 +0000 : moshe weitzman I dislike the recent movement away from overview pages. I think we can still provide and overview while allowing editing to happen in a dedicated form. An example in the wrong direction is 'content types' admin. It is impossible to get an overview, and the listing page is worthless. I agree with Steven on this one. It is true that consistency is desirable. But consistent crap is not. ------------------------------------------------------------------------ Fri, 29 Jul 2005 20:02:39 +0000 : killes(a)www.drop.org I agree with Moshe. Getting a quick overview about what is set how is essential for an admin. ------------------------------------------------------------------------ Sat, 30 Jul 2005 00:27:15 +0000 : Robrecht Jacques Overall I like the screenshots. Did not test the patch. Bèr, what I think Steven (#15) meant by "showing the roles as a comma-separate list in the input formats table" is that as an overview page it would be better if you added a list (not an input box!) to screenshot "input-formats_list1.png" (comment #1). Add a column that reads "Roles" (or "Enabled for") and then for each row a comma seperated list of the roles that can use this input format (or maybe even better a HTML list). You would still need to click "configure" to _change_ the roles. The "Default" text (not a radio button) you already show in the "Options" column is something similar: it immediately shows that this input format is the default without you having to go to the "configure" page. Why not do the same for "roles"? If this is what Steven meant, then I tend to agree with him. And to help moshe and killes: maybe you could even add a column that _lists_ the used input filters. Again, only a list, not a way to change it. That's what the "operations" column is for. A table like that will "fluid" better than what we have now, still it is a _complete_ overview of the settings. I could have misunderstand someone... I have the tendency to do that... ------------------------------------------------------------------------ Mon, 22 Aug 2005 08:21:32 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/filter_interface_improvements.patch (14.52 KB) by popular demand: a comma separated list. ------------------------------------------------------------------------ Mon, 22 Aug 2005 08:24:31 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/format_now_contains_roles.png (22.09 KB) and a screenshot ------------------------------------------------------------------------ Mon, 22 Aug 2005 18:39:44 +0000 : Dries Please check your use of print theme('page', $output). Normally, you just return $output. ------------------------------------------------------------------------ Mon, 29 Aug 2005 18:54:28 +0000 : Bèr Kessels All instances of theme('page',...) in filter.module are now changed to return ... ; furthermore, the patch is the same. ------------------------------------------------------------------------ Mon, 29 Aug 2005 20:04:02 +0000 : Uwe Hermann Bèr, I think you forgot to attach your updated patch. ------------------------------------------------------------------------ Mon, 29 Aug 2005 20:27:39 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/filter_interface_improvements_2.patch (13.52 KB) my email client has this nice warning system for attachements. Maybe drupal should search for the word attachement too, and when no att. found give an error ;). Or maybe i should just learn to pay attention. Guess that is easier. Anyway, here it is. ------------------------------------------------------------------------ Thu, 08 Sep 2005 20:22:54 +0000 : Dries I wanted to apply this patch but: 1. I can't change the default input format. 'Save'-button doesn't work. 2. I got confused by the fact I can't change the roles of the default filter. I think the form group description should explain this. 3. form_group(t(filter)) should be form_group(t('filter'), ...).

1 0

[drupal-devel] [feature] Replace core archive.module w/ codemonkeyx archive.module
by m3avrck 08 Sep '05

08 Sep '05

Issue status update for http://drupal.org/node/29676 Post a follow up: http://drupal.org/project/comments/add/29676 Project: Drupal Version: cvs Component: archive.module Category: feature requests Priority: normal Assigned to: Morbus Iff Reported by: Morbus Iff Updated by: m3avrck Status: patch (ready to be committed) Just retested with the latest patch I made, applies cleanly against HEAD and *everything* works great and looks great too. No errors of any kind, even works on PHP 5.0.5 with no call by reference errors, :D Let's get this baby in! m3avrck Previous comments: ------------------------------------------------------------------------ Thu, 25 Aug 2005 21:08:49 +0000 : Morbus Iff Over at http://drupal.org/node/8287, Berkes mentions that the core archive.module was considered being removed, per a discussion at the Drupal Sprint. Kjartan also mentions he would "love to have the archive module improved in general." In chatting with chx about this, he mentioned codemonkeyx's rewrite sitting in contrib/modules/archive/. I'll be doing some work with the archive.module over the next few days, and will be basing my changes around codemonkeyx's version, and making it compatible with HEAD. This general Issue is to move codemonkeyx's version into HEAD as a replacement to the existing archive.module. An unknown version of his replacement can be seen at http://www.codemonkeyx.net/archive. I'll be running a live HEAD version soon as well. These patches were made during the customization of Drupal by http://www.NHPR.org. In loving support of open source software, http://www.NHPR.org will continue to contribute patches they feel the community will benefit from. Questions about this patch should be directed to morbus(a)disobey.com. ------------------------------------------------------------------------ Fri, 26 Aug 2005 19:45:59 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues_2 (9.56 KB) As an example of a very early revision, see the attached, with the following changes from the current contrib CVS: * removed the year offset from theme_archive_navigation_years, which controlled how many year links to show at once in the top nav. For those with sites with more than five years, they'll WANT people to notice that they have five years, not to have to click on the earliest date and then have their expectations changed. * made the "created > $date" in archive_buildQuery "created >= $date" instead, to allow posts that were created at exactly midnight that day (like me, by design). * since there's no block, I made the menu item visible upon first load. this menu item is given "access content" permissions. More to come, including doxygen and gmt considerations. ------------------------------------------------------------------------ Fri, 26 Aug 2005 19:47:41 +0000 : Morbus Iff Might as well start getting a review of it so I can fix 'em as they come in. ------------------------------------------------------------------------ Fri, 26 Aug 2005 19:56:49 +0000 : Tobias Maier cant you provide a patch file? thanks for your work ------------------------------------------------------------------------ Fri, 26 Aug 2005 20:04:01 +0000 : Morbus Iff The codemonkeyx version is a complete rewrite of the core archive.module. If I were to create a patch file against core, every line would be deleted, and every line would be new. Once I finish my revisions to codemonkey's version, I'll post the final version here, as well as a patch against his current CVS. ------------------------------------------------------------------------ Fri, 26 Aug 2005 20:09:13 +0000 : Tobias Maier ok, thanks again :D ------------------------------------------------------------------------ Mon, 29 Aug 2005 13:41:43 +0000 : Junyor +1 for this change. The archive.module in core is dead. ------------------------------------------------------------------------ Mon, 29 Aug 2005 16:14:30 +0000 : adrian What is the progress on this morbus ? ------------------------------------------------------------------------ Mon, 29 Aug 2005 16:29:13 +0000 : Morbus Iff Adrian - I'll be attaching a new version either later today or tomorrow, with a CHANGELOG. I'll also be running a live version of it over on NHPR.org for people to play with. The three major things I'm worried about right now are a) doxygen, b) variable/function naming, c) GMT considerations. After those, I'll be exploring a patch for my own needs: the ability to get archives for particular term. ------------------------------------------------------------------------ Mon, 29 Aug 2005 18:53:34 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues/archive.module (9.93 KB) Here's the latest, with the following changelog: * reordered some routines to be a little more workflowish. * renamed archive_buildQuery to archive_build_query. * general whitespace and formatting cleanup. * HEADish update: returning $output, not page templating it. * removed the reference of &$ad in archive_build_query. * test for the existence of arg(#)'s before validating them. * archive_validSomething changed to archive_valid_something. * removed unused vars: cur_date, cur_date_end. * renamed archive_buildURL to archive_build_url. * removed the HTML whitespace from the theming. * twiddled a lot of quotes and apostraphes. * removed 'future' CSS class. ill-defined. * reordered/renamed the CSS classes. ------------------------------------------------------------------------ Mon, 29 Aug 2005 18:54:08 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues/_p_29676_archive_css.patch (1.56 KB) And the drupal.css patch. ------------------------------------------------------------------------ Mon, 29 Aug 2005 18:56:29 +0000 : Morbus Iff This version of the module is currently running live at http://www.nhpr.org/archive/. ------------------------------------------------------------------------ Mon, 29 Aug 2005 19:59:57 +0000 : Tobias Maier if i click for example on 2003 it would be good if this would go to january or december and marks them that this one will be shown now as you can see it if you click on january 2003. it has to select * on the first: the first month of writing * on the last: the last month of writing * on every else: january I hope you can understand what I mean... greets tobias ------------------------------------------------------------------------ Mon, 29 Aug 2005 20:17:40 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues/archive_0.module (11.28 KB) Alright. I've attached another new version that adds a new feature that wasn't part of the original codemonkeyx CVS, but was chatted about on the devel list back in April. If this particular feature has bad code or needs heavy refactoring, certainly consider ONLY the version in comment #9 (and the matching drupal.css patch in #10). This new version supports dated archives based on taxonomy tids. It was a quick addition which NHPR.org needed (for the date nav; the normal tid archive pager wasn't strong enough for our needs). Since it was a quick addition, it supports only ONE tid at a time - the 'and/or' syntax for the taxonomy.module was not brought over. If that syntax was desired, it'd make more sense to create some sort of API for archive.module so that other nodes can take advantage of the dated nav in their normal pages (like node types, users, forums, etc.) The added code supports term matches at any granularity: # all node types that match tid 15000 ('The Front Porch') http://www.nhpr.org/archive/term/15000 # all 2005 node types that match tid 20 ('Health') http://www.nhpr.org/archive/2005/term/20 # all March, 2003 node types that match tid 9 ('Education') http://www.nhpr.org/archive/2003/3/term/9 # all July 11, 2003 node types that match tid 49 ('Economy') http://morbus.totalnetnh.net/nhpr/archive/2002/7/11/term/49 ------------------------------------------------------------------------ Mon, 29 Aug 2005 20:27:26 +0000 : Tobias Maier what does this mean? "Story Archives of 'archives' " on http://www.nhpr.org/archive/term/15000 should this maybe named? "Archive of 'The Front Porch'" if I go to http://www.nhpr.org/archive/term/20 I can only read "archives"... why is there a difference? - I never tested this module on my test site, because I'm not at home - ------------------------------------------------------------------------ Mon, 29 Aug 2005 20:29:34 +0000 : Morbus Iff Tobias: that wouldn't be possible, at least not accurately. The new archive.module supports browsing by year, month, and day, as you know. archive/2005 loads up all the data from a particular year and starts creating a pager out of it. Consider if you have 3 posts in December, and 15 posts in November. It wouldn't be "right" to highlight December because the pager display for the entire year would also include some of November's entries (since 3 is less than the pager increment). Likewise, if we ONLY showed the items from December, then we wouldn't have a "pager by year" feature, only a "pager by month (defaulting to December when none is selected)" feature. ------------------------------------------------------------------------ Mon, 29 Aug 2005 20:30:41 +0000 : Morbus Iff Tobias: regarding #14, that's an artifact of the templates that I'm using for NHPR, and has nothing to do with archive.module itself (in fact, once the anonymous cache expires, you'll see that little oopsie go away). ------------------------------------------------------------------------ Mon, 29 Aug 2005 20:31:47 +0000 : Tobias Maier I can see your right :) I hope it comes in HEAD before tomorrow :D ------------------------------------------------------------------------ Tue, 30 Aug 2005 00:20:12 +0000 : dtan I apologize if this is already a known issue. http://www.nhpr.org/archive/2005/9 does not create a link for september 1st, even though there are 2 nodes listed (http://www.nhpr.org/archive/2005/9/1) ------------------------------------------------------------------------ Tue, 30 Aug 2005 17:11:21 +0000 : Morbus Iff dtan: I'm pretty sure I know what this is - I'll address it either later today or tomorrow. ------------------------------------------------------------------------ Tue, 30 Aug 2005 20:49:58 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues/archive_1.module (11.53 KB) Alright, I've attached a new archive.module - this is the version WITH term filtering enabled. I can make one without terms if necessary - otherwise, I'll just work from this one for now. This version fixes the bug that dtan saw, as a well as a bunch of other off-by-one errors. Of primary importance, however, is that all mktime's that mattered have been switched to gmmktime, which was one of the oft-reported Issues with the old archive.module. I want to eyeball them all again and make sure they're right though. The URLs from #13 are still operational and the CSS from #10 is still required. ------------------------------------------------------------------------ Wed, 31 Aug 2005 00:02:06 +0000 : Morbus Iff In testing with a few people in #drupal, we've discovered a much bigger problem, which affects this rewritten module as well as the current core archive.module. In a nutshell, the node.created time is stored with time(). PHP's time() bases itself on the server time, NOT on GMT. Thus, for archive.module to work correctly, it must ALWAYS use mktime (relative to server time) and never consider the $user->timezone (relative to GMT). For archive.module, this would cause dates to always be considered via server time, which isn't good, but is better than the craziness going on now. Alternatively, we could try to convert server time to GMT first, and then work with that. The proper solution is to fix node.module to use gmmktime without any arguments for node.created, then have an update path that modifies all node .created and .modified values to GMT, not server time. ------------------------------------------------------------------------ Wed, 31 Aug 2005 05:53:51 +0000 : Junyor Since that is also a problem with the old archive.module, I don't see why it should stop this from getting into core. ------------------------------------------------------------------------ Wed, 31 Aug 2005 06:37:28 +0000 : stefan nagtegaal "Since that is also a problem with the old archive.module, I don't see why it should stop this from getting into core. " Well, I think it's better to only accept the best code rather than accepting bugs getting in core. ------------------------------------------------------------------------ Wed, 31 Aug 2005 07:05:51 +0000 : Kobus I am with Junyor on this. If this can be fixed, great, but if not, it's not a train smash, as the old one exhibits the same problem. I say add the new archive module, if there are no other ciritical bugs with it. It is much more robust and usable than the old one. We desperately need a new archive module. I couldn't find any other bugs while testing with the links Morbus posted. I don't have a HEAD installation anywhere, so can't test it locally at this moment. Regards, Kobus ------------------------------------------------------------------------ Wed, 31 Aug 2005 07:07:49 +0000 : Kobus BTW, code freeze means no new code added, right? Can't this module put in Core as is for the code freeze and the bug sorted out before the official release? Or is that just mean of me to suggest that? Kobus ------------------------------------------------------------------------ Wed, 31 Aug 2005 07:13:09 +0000 : Junyor Stefan: The bug is already in core, since node.module is in core. Archive.module (old and new!) just shows that bug. ------------------------------------------------------------------------ Wed, 31 Aug 2005 07:20:35 +0000 : Tobias Maier I want to have it in core for 4.7, too!!! ------------------------------------------------------------------------ Wed, 31 Aug 2005 07:25:24 +0000 : stefan nagtegaal Junyor: I know that.. Though I think it's not good to accept code which we are aware of that it has bugs in it.. Offcourse this is very double, because drupal contains (probably) a lot of bugs, only they weren't spotted yet.. But, accepting code which has bugs in unacceptable imo.. For example, the node revisions patch had almost 40 reviews/rewritings from Gerhard and several others before it was accepted to be in core.. If we do not allow bugs to go into core, we don't have to bughunt and fix later which is a good thing.. So, imo we should first sort out the problem, then discuss what the best way is to fix the problem, and after that Squeeze that moron! ;-) ------------------------------------------------------------------------ Wed, 31 Aug 2005 09:11:06 +0000 : adrian I vote we include this module, and open up a new issue for the bug. It's not archive's fault that this occurs, it is just showing the data it has access to. The bug already exists in core too. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:50:59 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues/archive_2.module (10.63 KB) Attached is a new, and most likely final, version of the archive.module. I've removed all user->timezone references - all date determinations are based on server time, which is also the time used in the "created" column of the node table. This is "more accurate" than what the core archive.module currently does (which'll always be wrong because it's treating server time as GMT, which isn't always the case). When node.module does start saving times as GMT properly, archive.module will have be to be tweaked with timezones and blah blah blah. I did fiddle around with determining the server offset in an attempt to get to a GMT base, but I didn't have much luck with that. The NHPR.org URLs above are still valid for testing. I need testers and reviewers. ------------------------------------------------------------------------ Wed, 31 Aug 2005 14:55:16 +0000 : m3avrck I get these two PHP errors when I have *no* content in my Drupal install (just did a clean install to test it): warning: mktime() [<a href='function.mktime'>function.mktime</a>]: Windows does not support negative values for this function in websites\drupal_cvs\drupal\modules\archive.module on line 274. warning: date() [<a href='function.date'>function.date</a>]: Windows does not support dates prior to midnight (00:00:00), January 1, 1970 in websites\drupal_cvs\drupal\modules\archive.module on line 274. Once I add content, these go away. Need some better checks to make sure if there is *no* content you don't get weird errors and what not :) ------------------------------------------------------------------------ Wed, 31 Aug 2005 14:55:45 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues/_p_29676_archive_css_0.patch (1.56 KB) Updated drupal.css patch for HEAD. ------------------------------------------------------------------------ Wed, 31 Aug 2005 15:26:01 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues/archive_3.module (10.75 KB) Attached is the complete module, fixed for m3avrck's #31. ------------------------------------------------------------------------ Wed, 31 Aug 2005 17:30:36 +0000 : m3avrck Reviewed patch and further test, running great over here! Definetly +1 for this one! ------------------------------------------------------------------------ Wed, 31 Aug 2005 20:03:36 +0000 : Morbus Iff Note: The NHPR folks preferred their archives to be sorted from earliest to latest (SQL ASC vs. DESC) for month/day listings, so no longer use the NHPR.org URLs above as representative of the module itself. ------------------------------------------------------------------------ Wed, 31 Aug 2005 21:26:37 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues/drupal.css (10.35 KB) CVS updated for HEAD again. ------------------------------------------------------------------------ Wed, 31 Aug 2005 21:27:00 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues/_p_29676_archive_css_1.patch (1.56 KB) GRRR. ------------------------------------------------------------------------ Thu, 08 Sep 2005 20:18:48 +0000 : m3avrck Attachment: http://drupal.org/files/issues/archive_0.patch (20.72 KB) Rerolled patch against head. Also created one patch that fixes both archive.module and drupal.css so it's a clean and simple process now :)

1 0

[drupal-devel] [task] Improvments to Filter UI
by Dries 08 Sep '05

08 Sep '05

Issue status update for http://drupal.org/node/27364 Post a follow up: http://drupal.org/project/comments/add/27364 Project: Drupal Version: cvs Component: filter.module Category: tasks Priority: normal Assigned to: Bèr Kessels Reported by: Bèr Kessels Updated by: Dries -Status: patch (code needs review) +Status: patch (code needs work) I wanted to apply this patch but: 1. I can't change the default input format. 'Save'-button doesn't work. 2. I got confused by the fact I can't change the roles of the default filter. I think the form group description should explain this. 3. form_group(t(filter)) should be form_group(t('filter'), ...). Dries Previous comments: ------------------------------------------------------------------------ Sun, 24 Jul 2005 09:30:54 +0000 : Bèr Kessels This is a mockup for the Filter UI. Currently it is soo complex, that I dare to call it broken. Please comment on this, for I will not ake any patches, when the general idea is disliked. The idea is t split filters and input formats better. Filters are filters, defined in modules. Input formats are bundles of filters. This is how we have it ATM, but the interface fails to communicate that. I tried to develop a consistent (with the rest of Drupal) interfce that makes it clearer what is what. The menu is as follows: * administer ** .... ** settings *** input formats *** filters ** ... ------------------------------------------------------------------------ Sun, 24 Jul 2005 09:37:10 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/input-formats_list1.png (16.34 KB) * administer ** .... ** settings *** input formats >> see attachement 1 *** filters ** ------------------------------------------------------------------------ Sun, 24 Jul 2005 09:39:02 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/input-formats_add.png (23.42 KB) * administer ** .... ** settings *** input formats >> see attachement 2, tab 2 *** filters ** Note: The default checkbox lmay seem a bit odd. But checking it will remove the "default status" from the current "default" one and make this one "default". The help text, and a drupal_set_message should explain this. ------------------------------------------------------------------------ Sun, 24 Jul 2005 09:40:08 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/input-formats_edit_format.png (22.17 KB) * administer ** .... ** settings *** input formats >> clicking a 'configure' link in the table of attachement 1 *** filters ** ------------------------------------------------------------------------ Sun, 24 Jul 2005 09:40:55 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/input-filters_list.png (36.24 KB) * administer ** .... ** settings *** input formats *** filters >> see attachement 4 ** ------------------------------------------------------------------------ Sun, 24 Jul 2005 09:41:40 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/input-formats_edit.png (22.63 KB) * administer ** .... ** settings *** input formats *** filters >> clicking a 'configure' link in the table of attachement 4 ** ------------------------------------------------------------------------ Mon, 25 Jul 2005 14:46:27 +0000 : Bèr Kessels IMO this makes the interface easier. But other disagree. And because it also removes one feature: (being able to use E.g. HTML in different input formats, with different settings) I'll simply wont fix this :( ------------------------------------------------------------------------ Thu, 28 Jul 2005 13:36:12 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/filter_module_UI_consistancy.patch (10.28 KB) I decided to go for it anyway. Be it in a slightly different way then my initial mockups. The difference is that I left the filters where they are, hidden beneath the formats. So, here is the patch that makes the filter UI more consistent with screens like blocs, menus, vocabularies et al also nice to note is that: 93 lines are added, 104 are removed. So netto we have less code :) (cvs diff filter.module | grep ^+ | wc -l) ------------------------------------------------------------------------ Thu, 28 Jul 2005 13:37:02 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/input-formats_list_html.png (44.64 KB) Here is how the listing now looks. ------------------------------------------------------------------------ Thu, 28 Jul 2005 13:37:57 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/input-formats_edit_format_html.png (113.71 KB) And the "configure" screen. The 'add' screen looks exactly the same, only with the default values pre-filled. ------------------------------------------------------------------------ Thu, 28 Jul 2005 13:39:19 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/input-formats_edit_format_too_long.png (121.82 KB) Oh, And here is a really nice example of why the current screen is no good. And here I "only" have four roles. I run a site with 12 roles, imagine this screen then! ------------------------------------------------------------------------ Thu, 28 Jul 2005 13:48:35 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/filter_module_UI_consistancy_HEAD.patch (10.24 KB) And here is a pathc for HEAD (I forgot that I developed this on a 4.6.2 codebase. Sorry) ------------------------------------------------------------------------ Thu, 28 Jul 2005 13:55:25 +0000 : stefan nagtegaal I like this very, very much! This is - indeed - much better than the current UI. This always fits, and makes the life of people who use a fixed width theme much nicer.. Ber, FYI (you probably know this too), 'admin/access' does also break fixed width themes due to the fact that it expands the width of the table when more roles are being added.. Good catch! I like it much more this way.. +1 from me... ------------------------------------------------------------------------ Fri, 29 Jul 2005 07:27:34 +0000 : Dries The code style needs work (spaces, tabs, 'as' -> 'AS'), the code comments need work, and debug statements should be removed. Screenshots look good to me. db_query('UPDATE {filter_formats} SET cache = %d, name = \'%s\', roles = \'%s\' WHERE format = %d', (int)$cache, $name, $roles, $format); can be written better/shorter quote-wise. ------------------------------------------------------------------------ Fri, 29 Jul 2005 09:04:16 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/filter_module_UI_consistancy_HEAD_0.patch (10.37 KB) Dries, thanks a lot for the review. OI fixed your comments. Or so I hope. code-style.pl was of no use (can that *please* be removed from core?) because it chokes on the help texts. It also pointed out a lot of old formatting isues, whic, when fixed wuold flood this patch with unrelated fixes. I tried to narow my style-searching to the areas I touched. And I made the comments more verbous. The query is fixed. it saves no characters, but looks better now. Thanks for the tip. The print_r..... blush. thats what I get for not running my default grep -r print_r on my code after finishing up... I could not find a lot of spaces and tabs. So it lmight be that i missed one. I triple checked it though. ------------------------------------------------------------------------ Fri, 29 Jul 2005 09:32:21 +0000 : Steven Moving the role settings inward does fix the wide-table problem, but it makes the filter configuration much more opaque. We could perhaps fix this by showing the roles as a comma-separate list in the input formats table. Such a list can wrap and will not stretch the table. Other than that: * Having the instructions for the Roles and Filters form_group() at the bottom is a bit weird. The distance is too big. * The table that was used for toggling filters on/off was much more compact before, and IMO a lot clearer. Why change to a flat list with the description staggered below each item? ------------------------------------------------------------------------ Fri, 29 Jul 2005 10:31:30 +0000 : Bèr Kessels consistancy, consistancy and more consistancy. There really is nothing worse that having a different 'concept' of UI for every thing in Drupal. We are going in the right direction, with the blocks being editable objects, menus acting like that, taxonomy, users, etc.. I wanted to make this act similar. I am not at all happy with the fact tah the list is still a form. But we need this, for setting the default. A big -1 for a comma-separate list. That is extremely hackish, even worse useabilitywise and just silly * we should avoid typing when not needed. * we should avoid errors on input, when they can be avoided (a typo is made very easy, I know all about it) Form set errors won't fix that usability -wise. I agree that you sort-of loose the overview. But IMO that is a minor loss comared to what we gain, usability-wise. The wtoo-wide was onlywhat triggered me. But do not think that I am noly making this patch to fix that issue. This patch is there to improve usability, through consistancy. o make drupal behave the same in most places in admin. If you know one screen, you know most of them. We really should move away from custom-hacks for every page, because that is useability rule #1: consistancy. Stadardisation, even if sometimes a custom hack might make things easier in that single case, it will reduce your overall usability so much that nearly ever it is worth that hack. That select-boxes instead of the table issue: consistancy. As well that it completely broke fluid CSS layouts, my solution is more consistant, and uses forms the way they are meant. If we cannot use $descritption in checkboxes they should be removed. but IMO it feels much more consistant. Tables are for tabular data, and this is simply a list of items, with additional data. And i do not really get what you mean with "too far down". Do you mean that you would like to see the order of the groups different? ------------------------------------------------------------------------ Fri, 29 Jul 2005 13:31:24 +0000 : Dries I haven't tested Ber's patch yet but looking at the screenshots, it like what I see. I remember being confused by the current filter UI, and often, I still am. ------------------------------------------------------------------------ Fri, 29 Jul 2005 19:53:26 +0000 : moshe weitzman I dislike the recent movement away from overview pages. I think we can still provide and overview while allowing editing to happen in a dedicated form. An example in the wrong direction is 'content types' admin. It is impossible to get an overview, and the listing page is worthless. I agree with Steven on this one. It is true that consistency is desirable. But consistent crap is not. ------------------------------------------------------------------------ Fri, 29 Jul 2005 20:02:39 +0000 : killes(a)www.drop.org I agree with Moshe. Getting a quick overview about what is set how is essential for an admin. ------------------------------------------------------------------------ Sat, 30 Jul 2005 00:27:15 +0000 : Robrecht Jacques Overall I like the screenshots. Did not test the patch. Bèr, what I think Steven (#15) meant by "showing the roles as a comma-separate list in the input formats table" is that as an overview page it would be better if you added a list (not an input box!) to screenshot "input-formats_list1.png" (comment #1). Add a column that reads "Roles" (or "Enabled for") and then for each row a comma seperated list of the roles that can use this input format (or maybe even better a HTML list). You would still need to click "configure" to _change_ the roles. The "Default" text (not a radio button) you already show in the "Options" column is something similar: it immediately shows that this input format is the default without you having to go to the "configure" page. Why not do the same for "roles"? If this is what Steven meant, then I tend to agree with him. And to help moshe and killes: maybe you could even add a column that _lists_ the used input filters. Again, only a list, not a way to change it. That's what the "operations" column is for. A table like that will "fluid" better than what we have now, still it is a _complete_ overview of the settings. I could have misunderstand someone... I have the tendency to do that... ------------------------------------------------------------------------ Mon, 22 Aug 2005 08:21:32 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/filter_interface_improvements.patch (14.52 KB) by popular demand: a comma separated list. ------------------------------------------------------------------------ Mon, 22 Aug 2005 08:24:31 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/format_now_contains_roles.png (22.09 KB) and a screenshot ------------------------------------------------------------------------ Mon, 22 Aug 2005 18:39:44 +0000 : Dries Please check your use of print theme('page', $output). Normally, you just return $output. ------------------------------------------------------------------------ Mon, 29 Aug 2005 18:54:28 +0000 : Bèr Kessels All instances of theme('page',...) in filter.module are now changed to return ... ; furthermore, the patch is the same. ------------------------------------------------------------------------ Mon, 29 Aug 2005 20:04:02 +0000 : Uwe Hermann Bèr, I think you forgot to attach your updated patch. ------------------------------------------------------------------------ Mon, 29 Aug 2005 20:27:39 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/filter_interface_improvements_2.patch (13.52 KB) my email client has this nice warning system for attachements. Maybe drupal should search for the word attachement too, and when no att. found give an error ;). Or maybe i should just learn to pay attention. Guess that is easier. Anyway, here it is.

1 0

[drupal-devel] [feature] Replace core archive.module w/ codemonkeyx archive.module
by m3avrck 08 Sep '05

08 Sep '05

Issue status update for http://drupal.org/node/29676 Post a follow up: http://drupal.org/project/comments/add/29676 Project: Drupal Version: cvs Component: archive.module Category: feature requests Priority: normal Assigned to: Morbus Iff Reported by: Morbus Iff Updated by: m3avrck Status: patch (ready to be committed) Attachment: http://drupal.org/files/issues/archive_0.patch (20.72 KB) Rerolled patch against head. Also created one patch that fixes both archive.module and drupal.css so it's a clean and simple process now :) m3avrck Previous comments: ------------------------------------------------------------------------ Thu, 25 Aug 2005 21:08:49 +0000 : Morbus Iff Over at http://drupal.org/node/8287, Berkes mentions that the core archive.module was considered being removed, per a discussion at the Drupal Sprint. Kjartan also mentions he would "love to have the archive module improved in general." In chatting with chx about this, he mentioned codemonkeyx's rewrite sitting in contrib/modules/archive/. I'll be doing some work with the archive.module over the next few days, and will be basing my changes around codemonkeyx's version, and making it compatible with HEAD. This general Issue is to move codemonkeyx's version into HEAD as a replacement to the existing archive.module. An unknown version of his replacement can be seen at http://www.codemonkeyx.net/archive. I'll be running a live HEAD version soon as well. These patches were made during the customization of Drupal by http://www.NHPR.org. In loving support of open source software, http://www.NHPR.org will continue to contribute patches they feel the community will benefit from. Questions about this patch should be directed to morbus(a)disobey.com. ------------------------------------------------------------------------ Fri, 26 Aug 2005 19:45:59 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues_2 (9.56 KB) As an example of a very early revision, see the attached, with the following changes from the current contrib CVS: * removed the year offset from theme_archive_navigation_years, which controlled how many year links to show at once in the top nav. For those with sites with more than five years, they'll WANT people to notice that they have five years, not to have to click on the earliest date and then have their expectations changed. * made the "created > $date" in archive_buildQuery "created >= $date" instead, to allow posts that were created at exactly midnight that day (like me, by design). * since there's no block, I made the menu item visible upon first load. this menu item is given "access content" permissions. More to come, including doxygen and gmt considerations. ------------------------------------------------------------------------ Fri, 26 Aug 2005 19:47:41 +0000 : Morbus Iff Might as well start getting a review of it so I can fix 'em as they come in. ------------------------------------------------------------------------ Fri, 26 Aug 2005 19:56:49 +0000 : Tobias Maier cant you provide a patch file? thanks for your work ------------------------------------------------------------------------ Fri, 26 Aug 2005 20:04:01 +0000 : Morbus Iff The codemonkeyx version is a complete rewrite of the core archive.module. If I were to create a patch file against core, every line would be deleted, and every line would be new. Once I finish my revisions to codemonkey's version, I'll post the final version here, as well as a patch against his current CVS. ------------------------------------------------------------------------ Fri, 26 Aug 2005 20:09:13 +0000 : Tobias Maier ok, thanks again :D ------------------------------------------------------------------------ Mon, 29 Aug 2005 13:41:43 +0000 : Junyor +1 for this change. The archive.module in core is dead. ------------------------------------------------------------------------ Mon, 29 Aug 2005 16:14:30 +0000 : adrian What is the progress on this morbus ? ------------------------------------------------------------------------ Mon, 29 Aug 2005 16:29:13 +0000 : Morbus Iff Adrian - I'll be attaching a new version either later today or tomorrow, with a CHANGELOG. I'll also be running a live version of it over on NHPR.org for people to play with. The three major things I'm worried about right now are a) doxygen, b) variable/function naming, c) GMT considerations. After those, I'll be exploring a patch for my own needs: the ability to get archives for particular term. ------------------------------------------------------------------------ Mon, 29 Aug 2005 18:53:34 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues/archive.module (9.93 KB) Here's the latest, with the following changelog: * reordered some routines to be a little more workflowish. * renamed archive_buildQuery to archive_build_query. * general whitespace and formatting cleanup. * HEADish update: returning $output, not page templating it. * removed the reference of &$ad in archive_build_query. * test for the existence of arg(#)'s before validating them. * archive_validSomething changed to archive_valid_something. * removed unused vars: cur_date, cur_date_end. * renamed archive_buildURL to archive_build_url. * removed the HTML whitespace from the theming. * twiddled a lot of quotes and apostraphes. * removed 'future' CSS class. ill-defined. * reordered/renamed the CSS classes. ------------------------------------------------------------------------ Mon, 29 Aug 2005 18:54:08 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues/_p_29676_archive_css.patch (1.56 KB) And the drupal.css patch. ------------------------------------------------------------------------ Mon, 29 Aug 2005 18:56:29 +0000 : Morbus Iff This version of the module is currently running live at http://www.nhpr.org/archive/. ------------------------------------------------------------------------ Mon, 29 Aug 2005 19:59:57 +0000 : Tobias Maier if i click for example on 2003 it would be good if this would go to january or december and marks them that this one will be shown now as you can see it if you click on january 2003. it has to select * on the first: the first month of writing * on the last: the last month of writing * on every else: january I hope you can understand what I mean... greets tobias ------------------------------------------------------------------------ Mon, 29 Aug 2005 20:17:40 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues/archive_0.module (11.28 KB) Alright. I've attached another new version that adds a new feature that wasn't part of the original codemonkeyx CVS, but was chatted about on the devel list back in April. If this particular feature has bad code or needs heavy refactoring, certainly consider ONLY the version in comment #9 (and the matching drupal.css patch in #10). This new version supports dated archives based on taxonomy tids. It was a quick addition which NHPR.org needed (for the date nav; the normal tid archive pager wasn't strong enough for our needs). Since it was a quick addition, it supports only ONE tid at a time - the 'and/or' syntax for the taxonomy.module was not brought over. If that syntax was desired, it'd make more sense to create some sort of API for archive.module so that other nodes can take advantage of the dated nav in their normal pages (like node types, users, forums, etc.) The added code supports term matches at any granularity: # all node types that match tid 15000 ('The Front Porch') http://www.nhpr.org/archive/term/15000 # all 2005 node types that match tid 20 ('Health') http://www.nhpr.org/archive/2005/term/20 # all March, 2003 node types that match tid 9 ('Education') http://www.nhpr.org/archive/2003/3/term/9 # all July 11, 2003 node types that match tid 49 ('Economy') http://morbus.totalnetnh.net/nhpr/archive/2002/7/11/term/49 ------------------------------------------------------------------------ Mon, 29 Aug 2005 20:27:26 +0000 : Tobias Maier what does this mean? "Story Archives of 'archives' " on http://www.nhpr.org/archive/term/15000 should this maybe named? "Archive of 'The Front Porch'" if I go to http://www.nhpr.org/archive/term/20 I can only read "archives"... why is there a difference? - I never tested this module on my test site, because I'm not at home - ------------------------------------------------------------------------ Mon, 29 Aug 2005 20:29:34 +0000 : Morbus Iff Tobias: that wouldn't be possible, at least not accurately. The new archive.module supports browsing by year, month, and day, as you know. archive/2005 loads up all the data from a particular year and starts creating a pager out of it. Consider if you have 3 posts in December, and 15 posts in November. It wouldn't be "right" to highlight December because the pager display for the entire year would also include some of November's entries (since 3 is less than the pager increment). Likewise, if we ONLY showed the items from December, then we wouldn't have a "pager by year" feature, only a "pager by month (defaulting to December when none is selected)" feature. ------------------------------------------------------------------------ Mon, 29 Aug 2005 20:30:41 +0000 : Morbus Iff Tobias: regarding #14, that's an artifact of the templates that I'm using for NHPR, and has nothing to do with archive.module itself (in fact, once the anonymous cache expires, you'll see that little oopsie go away). ------------------------------------------------------------------------ Mon, 29 Aug 2005 20:31:47 +0000 : Tobias Maier I can see your right :) I hope it comes in HEAD before tomorrow :D ------------------------------------------------------------------------ Tue, 30 Aug 2005 00:20:12 +0000 : dtan I apologize if this is already a known issue. http://www.nhpr.org/archive/2005/9 does not create a link for september 1st, even though there are 2 nodes listed (http://www.nhpr.org/archive/2005/9/1) ------------------------------------------------------------------------ Tue, 30 Aug 2005 17:11:21 +0000 : Morbus Iff dtan: I'm pretty sure I know what this is - I'll address it either later today or tomorrow. ------------------------------------------------------------------------ Tue, 30 Aug 2005 20:49:58 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues/archive_1.module (11.53 KB) Alright, I've attached a new archive.module - this is the version WITH term filtering enabled. I can make one without terms if necessary - otherwise, I'll just work from this one for now. This version fixes the bug that dtan saw, as a well as a bunch of other off-by-one errors. Of primary importance, however, is that all mktime's that mattered have been switched to gmmktime, which was one of the oft-reported Issues with the old archive.module. I want to eyeball them all again and make sure they're right though. The URLs from #13 are still operational and the CSS from #10 is still required. ------------------------------------------------------------------------ Wed, 31 Aug 2005 00:02:06 +0000 : Morbus Iff In testing with a few people in #drupal, we've discovered a much bigger problem, which affects this rewritten module as well as the current core archive.module. In a nutshell, the node.created time is stored with time(). PHP's time() bases itself on the server time, NOT on GMT. Thus, for archive.module to work correctly, it must ALWAYS use mktime (relative to server time) and never consider the $user->timezone (relative to GMT). For archive.module, this would cause dates to always be considered via server time, which isn't good, but is better than the craziness going on now. Alternatively, we could try to convert server time to GMT first, and then work with that. The proper solution is to fix node.module to use gmmktime without any arguments for node.created, then have an update path that modifies all node .created and .modified values to GMT, not server time. ------------------------------------------------------------------------ Wed, 31 Aug 2005 05:53:51 +0000 : Junyor Since that is also a problem with the old archive.module, I don't see why it should stop this from getting into core. ------------------------------------------------------------------------ Wed, 31 Aug 2005 06:37:28 +0000 : stefan nagtegaal "Since that is also a problem with the old archive.module, I don't see why it should stop this from getting into core. " Well, I think it's better to only accept the best code rather than accepting bugs getting in core. ------------------------------------------------------------------------ Wed, 31 Aug 2005 07:05:51 +0000 : Kobus I am with Junyor on this. If this can be fixed, great, but if not, it's not a train smash, as the old one exhibits the same problem. I say add the new archive module, if there are no other ciritical bugs with it. It is much more robust and usable than the old one. We desperately need a new archive module. I couldn't find any other bugs while testing with the links Morbus posted. I don't have a HEAD installation anywhere, so can't test it locally at this moment. Regards, Kobus ------------------------------------------------------------------------ Wed, 31 Aug 2005 07:07:49 +0000 : Kobus BTW, code freeze means no new code added, right? Can't this module put in Core as is for the code freeze and the bug sorted out before the official release? Or is that just mean of me to suggest that? Kobus ------------------------------------------------------------------------ Wed, 31 Aug 2005 07:13:09 +0000 : Junyor Stefan: The bug is already in core, since node.module is in core. Archive.module (old and new!) just shows that bug. ------------------------------------------------------------------------ Wed, 31 Aug 2005 07:20:35 +0000 : Tobias Maier I want to have it in core for 4.7, too!!! ------------------------------------------------------------------------ Wed, 31 Aug 2005 07:25:24 +0000 : stefan nagtegaal Junyor: I know that.. Though I think it's not good to accept code which we are aware of that it has bugs in it.. Offcourse this is very double, because drupal contains (probably) a lot of bugs, only they weren't spotted yet.. But, accepting code which has bugs in unacceptable imo.. For example, the node revisions patch had almost 40 reviews/rewritings from Gerhard and several others before it was accepted to be in core.. If we do not allow bugs to go into core, we don't have to bughunt and fix later which is a good thing.. So, imo we should first sort out the problem, then discuss what the best way is to fix the problem, and after that Squeeze that moron! ;-) ------------------------------------------------------------------------ Wed, 31 Aug 2005 09:11:06 +0000 : adrian I vote we include this module, and open up a new issue for the bug. It's not archive's fault that this occurs, it is just showing the data it has access to. The bug already exists in core too. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:50:59 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues/archive_2.module (10.63 KB) Attached is a new, and most likely final, version of the archive.module. I've removed all user->timezone references - all date determinations are based on server time, which is also the time used in the "created" column of the node table. This is "more accurate" than what the core archive.module currently does (which'll always be wrong because it's treating server time as GMT, which isn't always the case). When node.module does start saving times as GMT properly, archive.module will have be to be tweaked with timezones and blah blah blah. I did fiddle around with determining the server offset in an attempt to get to a GMT base, but I didn't have much luck with that. The NHPR.org URLs above are still valid for testing. I need testers and reviewers. ------------------------------------------------------------------------ Wed, 31 Aug 2005 14:55:16 +0000 : m3avrck I get these two PHP errors when I have *no* content in my Drupal install (just did a clean install to test it): warning: mktime() [<a href='function.mktime'>function.mktime</a>]: Windows does not support negative values for this function in websites\drupal_cvs\drupal\modules\archive.module on line 274. warning: date() [<a href='function.date'>function.date</a>]: Windows does not support dates prior to midnight (00:00:00), January 1, 1970 in websites\drupal_cvs\drupal\modules\archive.module on line 274. Once I add content, these go away. Need some better checks to make sure if there is *no* content you don't get weird errors and what not :) ------------------------------------------------------------------------ Wed, 31 Aug 2005 14:55:45 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues/_p_29676_archive_css_0.patch (1.56 KB) Updated drupal.css patch for HEAD. ------------------------------------------------------------------------ Wed, 31 Aug 2005 15:26:01 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues/archive_3.module (10.75 KB) Attached is the complete module, fixed for m3avrck's #31. ------------------------------------------------------------------------ Wed, 31 Aug 2005 17:30:36 +0000 : m3avrck Reviewed patch and further test, running great over here! Definetly +1 for this one! ------------------------------------------------------------------------ Wed, 31 Aug 2005 20:03:36 +0000 : Morbus Iff Note: The NHPR folks preferred their archives to be sorted from earliest to latest (SQL ASC vs. DESC) for month/day listings, so no longer use the NHPR.org URLs above as representative of the module itself. ------------------------------------------------------------------------ Wed, 31 Aug 2005 21:26:37 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues/drupal.css (10.35 KB) CVS updated for HEAD again. ------------------------------------------------------------------------ Wed, 31 Aug 2005 21:27:00 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues/_p_29676_archive_css_1.patch (1.56 KB) GRRR.

1 0

[drupal-devel] [bug] Multiple node types are broken
by chx 08 Sep '05

08 Sep '05

Issue status update for http://drupal.org/node/29785 Post a follow up: http://drupal.org/project/comments/add/29785 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: chx Status: patch (code needs review) Attachment: http://drupal.org/files/issues/node_basename.patch (3.92 KB) People come and tell me that I have broken node_get_module_name. I tell them I fixed it. This leads nowhere. Yesterday Kjartan, jakeg and me had finally a conversation during which we have arrived to the conclusion that there is no need for two hooks and hook_node_name could return either a string or an array in the form of: <?php array('type-a' => array('node type a', 'base')), 'type-b' => 'node type b', 'node type c'); ?> Which would define a 'type-a' , with a human readable name 'node type a' and base_load would be called for hook_load and so forth. for 'type-b' , the difference is that modulename_load would be called. For 'node type c', the node type would be modulename. chx

5 18

[drupal-devel] [bug] Can't delete subjects with '&' in title
by Souvent22 08 Sep '05

08 Sep '05

Issue status update for http://drupal.org/node/27140 Post a follow up: http://drupal.org/project/comments/add/27140 Project: Drupal Version: cvs Component: contact.module Category: bug reports Priority: normal -Assigned to: Anonymous +Assigned to: Souvent22 Reported by: m3avrck Updated by: Souvent22 Status: patch (code needs review) Thanks. yes, you're right. didn't know if that was universal across the board though since that'st he default, but someone could give their primary key constraint a diff. name. But, with your comment, I'll wait and let you take care of fixing my pgsql portion of my patch. Thanks for your comments though, really helpful...and not confusing. :). Souvent22 Previous comments: ------------------------------------------------------------------------ Wed, 20 Jul 2005 14:57:03 +0000 : m3avrck Using the site-wide contact forms, any subjects with an '&' cannot be deleted. Clicking delete for these subjects brings up the expected action flow, however, confirming the delete does nothing. Subject still exists. ------------------------------------------------------------------------ Mon, 25 Jul 2005 13:45:38 +0000 : m3avrck It appears this issue might be linked with this one: http://drupal.org/node/23685 A problem with mod_rewrite and not the actual contact module. ------------------------------------------------------------------------ Wed, 31 Aug 2005 16:51:56 +0000 : Souvent22 Attachment: http://drupal.org/files/issues/contact.module_6.patch (4.7 KB) Redid how contacts work. It now references contacts by ID instead of their category as the unique identifier. It also allows for contacts to have weights, so that one may control where the contact will appear in the listing. ------------------------------------------------------------------------ Wed, 31 Aug 2005 16:53:01 +0000 : Souvent22 Attachment: http://drupal.org/files/issues/updates.inc (27.23 KB) This is the update.inc for the database cahnges that goes along with this patch. ------------------------------------------------------------------------ Wed, 31 Aug 2005 18:31:37 +0000 : Souvent22 Attachment: http://drupal.org/files/issues/contact.module_7.patch (4.5 KB) Updated patch. ------------------------------------------------------------------------ Wed, 31 Aug 2005 18:31:55 +0000 : Souvent22 Attachment: http://drupal.org/files/issues/updates.inc_4.patch (1.29 KB) Updated database patch. ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:17:21 +0000 : Cvbge Hi, 0. You should keep all changes in one file, i.e. pake one patch, not patch for every file 1. You should include changes for database/database.{mysql,pgsql} 2. IIRC auto_increment was depreciated, you should use db_next_id() (but I see auto_increment in cvs...) 3. You use "WHERE cid = '%s'", but cid is integer so this should probably be %d (although this probably would not break anything..) ------------------------------------------------------------------------ Wed, 07 Sep 2005 17:31:21 +0000 : Souvent22 Attachment: http://drupal.org/files/issues/contact_full.patch (6.38 KB) Ok, I've taken your suggestions in, and i think I have it this time. Let me know if this is a correct format for a patch. This only fixes the bug of not being able to delete special characters. ------------------------------------------------------------------------ Wed, 07 Sep 2005 17:37:30 +0000 : m3avrck Check your database updates, the UNIQUE fields don't look right syntactically. ------------------------------------------------------------------------ Wed, 07 Sep 2005 18:23:07 +0000 : Souvent22 Attachment: http://drupal.org/files/issues/contact_full_0.patch (6.25 KB) Ok, i apologize for the last upload. Pleasle ignore. I send the wrong file. :-[. Anyway, here's a tested, tried, and correct patch. ------------------------------------------------------------------------ Wed, 07 Sep 2005 18:26:33 +0000 : m3avrck +1 patch works great! Latest HEAD version applied with update.php works great, modifies database correctly for MySQL (and PostgreSQL looks correct as well) and contacts can now be deleted if they have '&' in the subject. Also, dumped entire database and created from modified database.mysql and everything works great. I'd say this patch is ready to be committed! ------------------------------------------------------------------------ Wed, 07 Sep 2005 21:44:47 +0000 : Cvbge Hi, patch does not applies to currect cvs anymore. For the database.pgsql you need to remove (11) from int(11) and also first "category" from UNIQUE line, so it looks like this: CREATE TABLE contact ( cid int NOT NULL, category varchar(255) NOT NULL default '', recipients text NOT NULL default '', reply text NOT NULL default '', UNIQUE (category), PRIMARY KEY (cid) ); The updates are also not compatibile with postgres, but postgres upgrade path is completly broken in cvs and I'll fix it later, so do not pay attention to it. ------------------------------------------------------------------------ Thu, 08 Sep 2005 13:05:54 +0000 : Souvent22 Hello. Thanks for the feedback. I'm in the middle of fixing it, but i have a question. You'll have to excuse my ignorance with PostGres, I'm not as familiar with it as I am MySQL. With that said, I have a question..... I am trying to make my update where one does not lose data. However, in PostGres, I have to make a seperate "temp" table before hand and then copy the date over the the new table (cause i don't know the exact name of the constraint to drop e.g. the primary key). Basically I'm asking if there is a better suggestion in which I doint have to use a temp table, and i could just manipulate the original table? Thanks. ------------------------------------------------------------------------ Thu, 08 Sep 2005 13:12:35 +0000 : Souvent22 Also, I created a temp name for my temp table. However, for prefix purposes, should I enclose it? Example: $temp_name = rand(); $res = db_query("SELECT * FROM $temp_name"); or $temp_name = rand(); $res = db_query("SELECT * FROM {$temp_name}"); Thanks. ------------------------------------------------------------------------ Thu, 08 Sep 2005 13:55:40 +0000 : Souvent22 Clarification: $temp_name = rand(); db_query("CREATE TABLE $temp_name AS SELECT * FROM {contact}"); $res = db_query("SELECT * FROM $temp_name"); or $temp_name = rand(); db_query("CREATE TABLE {$temp_name} AS SELECT * FROM {contact}"); $res = db_query("SELECT * FROM {$temp_name}"); ------------------------------------------------------------------------ Thu, 08 Sep 2005 17:52:06 +0000 : Cvbge "cause i don't know the exact name of the constraint to drop e.g. the primary key " Yes, you know it, it's tablename_pkey. In this case it's contact_pkey. You can drop it by dropping index with it's name (you could drop constraint with it's name but it was not available in 7.2) Adding a column and adding unique attribute is also doable without temp table. You can look at (some) previous updates. But some of them are incorrect ;) By saying that I'll fix it later I meant that I want to introduce function to modify tables and don't want to make temporary fixes now.

1 0

[drupal-devel] [bug] On admin/themes/settings, the "Display post information on" section does not appear on HEAD.
by amanuel 08 Sep '05

08 Sep '05

Issue status update for http://drupal.org/node/30716 Post a follow up: http://drupal.org/project/comments/add/30716 Project: Drupal Version: cvs Component: system.module Category: bug reports Priority: critical Assigned to: Anonymous Reported by: amanuel Updated by: amanuel Status: patch (ready to be committed) On admin/themes/settings, the "Display post information on" section does not appear on HEAD. this is due to a simple typo: $node_type shoulde be $node_types patch: --- system.module 2005-08-28 14:17:47.000000000 -0400 +++ system.module.fixed 2005-09-08 15:24:31.000000000 -0400 @@ -794,7 +794,7 @@ } // Toggle node display. - $node_type = module_invoke('node', 'get_types'); + $node_types = module_invoke('node', 'get_types'); if ($node_types) { $group = ''; foreach ($node_types as $type => $name) { amanuel

1 0

[drupal-devel] [bug] fix empty form elements
by Dries 08 Sep '05

08 Sep '05

Issue status update for http://drupal.org/node/18663 Post a follow up: http://drupal.org/project/comments/add/18663 Project: Drupal Version: 4.6.3 Component: base system Category: bug reports Priority: critical Assigned to: chx Reported by: killes(a)www.drop.org Updated by: Dries -Status: patch (ready to be committed) +Status: patch (code needs review) Can someone test this first? Dries Previous comments: ------------------------------------------------------------------------ Thu, 10 Mar 2005 01:51:56 +0000 : 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. ------------------------------------------------------------------------ Sat, 12 Mar 2005 18:47:42 +0000 : 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. ------------------------------------------------------------------------ Mon, 28 Mar 2005 16:27:03 +0000 : 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. ------------------------------------------------------------------------ Mon, 28 Mar 2005 16:32:16 +0000 : 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.... ------------------------------------------------------------------------ Mon, 28 Mar 2005 17:36:01 +0000 : 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... ------------------------------------------------------------------------ Fri, 08 Apr 2005 07:19:28 +0000 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_2.patch (1.9 KB) killes did not like the previous version. ------------------------------------------------------------------------ Mon, 11 Apr 2005 23:15:05 +0000 : chx Dries? ------------------------------------------------------------------------ Tue, 12 Apr 2005 19:15:52 +0000 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_3.patch (1.64 KB) ------------------------------------------------------------------------ Tue, 12 Apr 2005 19:39:00 +0000 : jhriggs I have to agree with Steven that it is up to the calling code...and I hardly think this is critical. Code that is expecting an array from form_checkboxes() should ensure that it is dealing with an array (i.e. is_array()). As for dealing with variables set through things like system_settings_save(), the same thing applies. The code that is retrieving the variable should ensure that it is working with an array. With regards to your patch, what's the difference if the calling code has to remember to check via is_array() or remember to call fix_checkboxes()? This method is no less error prone as it's just as easy to forget this call as it is to check for an array. That is unless you are suggesting that fix_checkboxes() gets called for every page load which seems rather crufty. I wrote the original patch to add form_checkboxes() -- which didn't exist -- and if I remember, this was the desired behavior if nothing was selected. Isn't this the same behavior one would see using a multiple form_select() if the user chooses nothing? They should behave the same. A single select widget is the same as a radio group, and a multiple select widget is the same as a group of checkboxes. They should behave the same; the select types just take up less realestate. ------------------------------------------------------------------------ Tue, 12 Apr 2005 21:23:06 +0000 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_4.patch (2.06 KB) Previous patch had index.php missing. jhriggs, the current problem is that a form_checkboxes expected to be an array. Currently, empty form_checkboxes is a string. That's bad. ------------------------------------------------------------------------ Tue, 12 Apr 2005 22:51:25 +0000 : jhriggs -1 on this patch. I still don't understand the problem. What do you mean by "form_checkboxes expected to be an array"? Is there code in core that assumes it is an array but it is not checked with is_array()? If so, let's add the missing checks. The same goes for contrib. This seems like swatting a fly with a sledge hammer to me. If we do go ahead and make this change, however, then we will need to change form_select() also (when multiple = TRUE). Radios and single select should be consistent, as should checkboxes and multiple select. ------------------------------------------------------------------------ Sat, 23 Apr 2005 11:35:30 +0000 : chx Attachment: http://drupal.org/files/issues/fix_form.patch (4.71 KB) See Gordon's complaint http://lists.drupal.org/archives/drupal-devel/2005-04/msg00934.html ------------------------------------------------------------------------ Sat, 23 Apr 2005 11:38:07 +0000 : chx Forgot to change the title. ------------------------------------------------------------------------ Sat, 23 Apr 2005 11:46:46 +0000 : chx It seems that it's not obvious why this is necessary. The calling code can not always handle itself the situation if the save is done by system_save_settings. The edge case is: I set up something, so variable_get no longer falls to its default value, later I decide to empty it. If we do not do something like this patch, system_save_settings can not know there is something to be set to 0/array() because it is just not in $_POST. ------------------------------------------------------------------------ Sat, 23 Apr 2005 13:23:37 +0000 : chx Attachment: http://drupal.org/files/issues/fix_form_0.patch (4.72 KB) Small bugfix. ------------------------------------------------------------------------ Sat, 23 Apr 2005 14:53:05 +0000 : chx doh. still has serious problems when the name is an array. stay tuned. ------------------------------------------------------------------------ Sun, 24 Apr 2005 09:13:00 +0000 : chx Attachment: http://drupal.org/files/issues/fix_form_1.patch (4.97 KB) Recursive version. ------------------------------------------------------------------------ Sun, 24 Apr 2005 12:51:18 +0000 : Dries Patch looks good but I would add some phpdoc to document what and why we are doing this. ------------------------------------------------------------------------ Sat, 21 May 2005 16:43:47 +0000 : chx Attachment: http://drupal.org/files/issues/fix_form_2.patch (3.23 KB) People told me that form_radios does not need this because we can reasonably expect that a radio is checked, that's different between a bunch of checkboxes and a bunch of radios. form_select submits an empty array (as also told by some). So we are back to checkbox and checkboxes which need the fix. ------------------------------------------------------------------------ Sat, 21 May 2005 16:55:24 +0000 : chx Attachment: http://drupal.org/files/issues/fix_form_3.patch (3.24 KB) ------------------------------------------------------------------------ Sat, 21 May 2005 18:34:20 +0000 : Dries Committed to HEAD. Thanks. ------------------------------------------------------------------------ Mon, 23 May 2005 00:09:30 +0000 : TDobes Why do we now have both a _fix_checkboxes() and fix_checkboxes() function? Shouldn't we name these something a little more intuitive? Without poking through the code, I have no idea what _fix_checkboxes is for and why it's different than fix_checkboxes. It really deserves some phpdoc too. Any sort of more-pleasant variable naming in _fix_checkboxes() would be much appreciated as well... It's easy to get lost in that array1/array2/single letter variable business pretty fast. Sorry for nit-picking, but most of the Drupal core code is far more readable than this. ------------------------------------------------------------------------ Tue, 06 Sep 2005 03:00:14 +0000 : mr700 It's strange that this problem still appears in drupal 4.6.3 (August 15, 2005); 4.6 CVS, and CVS Head and has been marked fixed on May 21, 2005. The last patch (fix_form_3.patch from comment 20 [1]) still applies with little fuzziness and fixes the problem. [1] http://drupal.org/node/18663#comment-30411 ------------------------------------------------------------------------ Wed, 07 Sep 2005 22:30:42 +0000 : tostinni Attachment: http://drupal.org/files/issues/fix_form-4.6.3.patch (3.54 KB) "It's strange that this problem still appears in drupal 4.6.3 (August 15, 2005); 4.6 CVS, and CVS Head and has been marked fixed on May 21, 2005. The last patch (fix_form_3.patch from comment 20) still applies with little fuzziness and fixes the problem. " The problem has been fixed in HEAD as Dries said in #21 HEAD refer to the future 4.7 version. Here is the updated patch that apply to 4.6.3

1 0

[drupal-devel] Release critical bugs for September 08, 2005
by drupal-devel＠drupal.org 08 Sep '05

08 Sep '05

/ bug reports email spam filtering does not "see" email addresses when they are NOT preceded by a word age: 33 weeks 5 days url: http://drupal.org/node/15654 / bug reports Submit button needs double-press when menu options dropped-down age: 1 week 1 day url: http://drupal.org/node/30097 update.php is broken when drupal is installed in subdirectory age: 1 week 1 day url: http://drupal.org/node/30075 Drupal "forgets" some blocks... age: 1 week 6 days url: http://drupal.org/node/29733 system_settings_save() bypasses access_control age: 1 week 6 days url: http://drupal.org/node/29680 files table needs a "module" column age: 2 weeks 4 days url: http://drupal.org/node/29297 Dont't work rewrite for apache 2 assigned: Алексей age: 2 weeks 5 days url: http://drupal.org/node/29238 logo uploads still broken...? can't replace default logo with new drupal 4.6.3 tar ball? age: 2 weeks 5 days url: http://drupal.org/node/29221 Error when anonymous user creates content age: 3 weeks 1 day url: http://drupal.org/node/29032 Reset user mail variables. age: 3 weeks 3 days url: http://drupal.org/node/28868 Free Tagging broken on forums age: 4 weeks 1 day url: http://drupal.org/node/28607 js addLoadEvent not working. age: 5 weeks 4 days url: http://drupal.org/node/27884 Forum bug with drupal 462 age: 6 weeks 10 hours url: http://drupal.org/node/27663 enable MySQL client side for UTF8 age: 7 weeks 2 days url: http://drupal.org/node/26990 Drupal 4.6.2 Doesn't allow login (IIS 5.1, PHP 5.0.4) assigned: bigeye age: 7 weeks 6 days url: http://drupal.org/node/26780 postgresql autocomplete code age: 10 weeks 20 hours url: http://drupal.org/node/26027 Accidentally Deleting Forums Vocabulary in Categories Breaks Forums? age: 13 weeks 5 days url: http://drupal.org/node/24274 Taxonomy Multiple Hierarchy doesn't display children for all its parent age: 14 weeks 1 day url: http://drupal.org/node/24023 Getting term node counts fails without "Administer Nodes" permission age: 14 weeks 2 days url: http://drupal.org/node/24015 Mysql Password with special symbols assigned: zignit age: 18 weeks 4 days url: http://drupal.org/node/21719 4.6 aggregator showing/not interpreting HTML tags age: 22 weeks 5 days url: http://drupal.org/node/19874 User deleted - nodes still "crippled", NOT deleted! age: 24 weeks 6 days url: http://drupal.org/node/19098 New submit error, non-admin user, postgresql database assigned: adrian age: 26 weeks 2 days url: http://drupal.org/node/18552 user redirected to homepage when logging in age: 26 weeks 6 days url: http://drupal.org/node/18381 Change to sesssion.inc violates UID database constraint age: 33 weeks 4 days url: http://drupal.org/node/15666 forum.module problem with postgresql v7.4.1 age: 34 weeks 3 days url: http://drupal.org/node/15411 PHP 5 gives error when session table has no session assigned: adschar age: 34 weeks 3 days url: http://drupal.org/node/15399 Argument NOT must be type boolean age: 42 weeks 3 days url: http://drupal.org/node/12950 Comments in approval queue still show up in tracker age: 46 weeks 5 days url: http://drupal.org/node/11647 Twin problems with comments assigned: Junyor age: 48 weeks 5 hours url: http://drupal.org/node/11366 user error: Duplicate entry assigned: Kjartan age: 1 year 39 weeks url: http://drupal.org/node/4428 / feature requests Site Slogan Margin age: 4 weeks 4 days url: http://drupal.org/node/28382 Filter the title to allow HTML comments age: 7 weeks 4 hours url: http://drupal.org/node/27205 moving pages en masse age: 23 weeks 5 hours url: http://drupal.org/node/19754 Taxonomy module should use nodeapi 'load' age: 26 weeks 1 day url: http://drupal.org/node/18631 Auto PHP memory maximisation age: 28 weeks 3 days url: http://drupal.org/node/17663 Allow named anchors to work without specifying full path to node age: 1 year 41 weeks url: http://drupal.org/node/4213 / support requests blocks age: 11 weeks 3 days url: http://drupal.org/node/25378 Problem with Forums age: 34 weeks 5 days url: http://drupal.org/node/15299 / bug reports Users with non-english locale cannot add events. age: 2 weeks 2 days url: http://drupal.org/node/29452 search over fields.inc fields conceptual error age: 44 weeks 6 days url: http://drupal.org/node/12259 fields.inc screws up with multible multiselect fields age: 44 weeks 6 days url: http://drupal.org/node/12257 / feature requests event.module support of repeating/recurring events? assigned: thehunmonkgroup age: 1 year 42 weeks url: http://drupal.org/node/4122 / bug reports Menu item not showing age: 48 weeks 6 days url: http://drupal.org/node/11210 / bug reports segmentation fault related to filestore age: 1 year 50 weeks url: http://drupal.org/node/3001 / bug reports file size bug age: 12 weeks 6 days url: http://drupal.org/node/24728 / bug reports gallery2 embeded bug with RC1 age: 3 weeks 20 hours url: http://drupal.org/node/29090 gallery bug - not integrated in drupal assigned: vidy age: 4 weeks 1 day url: http://drupal.org/node/28598 gallery.module doesn't synch passwords when they're changed age: 6 weeks 2 hours url: http://drupal.org/node/27702 Full screen slideshow in Gallery module is not working (erroring out on the wrong URL) age: 9 weeks 20 hours url: http://drupal.org/node/26479 Drupal session being logged out each time gallery module is called age: 11 weeks 4 hours url: http://drupal.org/node/25614 Gallery error age: 19 weeks 3 days url: http://drupal.org/node/21290 Fatal Error: variable_get() age: 24 weeks 4 days url: http://drupal.org/node/19189 No Gallery Image Block age: 25 weeks 9 hours url: http://drupal.org/node/19045 Mini applet can't find gallery at url / age: 25 weeks 5 days url: http://drupal.org/node/18785 Fatal error: Undefined class name 'galleryembed' age: 28 weeks 2 days url: http://drupal.org/node/17746 / support requests Can't Embed Gallery into Drupal age: 27 weeks 4 days url: http://drupal.org/node/18029 / bug reports Drupal-4.4-rc breaks registration setups and groups.module age: 1 year 25 weeks url: http://drupal.org/node/6392 / bug reports Path for image folder not inserted correctly when nodetype = image age: 1 week 2 days url: http://drupal.org/node/30031 image module patch & image_import module age: 2 weeks 3 days url: http://drupal.org/node/29377 image module fails on dual site configuration age: 8 weeks 17 hours url: http://drupal.org/node/26657 Editing a previously uploaded and thumnailed image loses the thumbnail. age: 11 weeks 2 days url: http://drupal.org/node/25465 Cannot upload .jpgs age: 27 weeks 3 days url: http://drupal.org/node/18076 Image.Module does not know the correct filename age: 42 weeks 2 days url: http://drupal.org/node/13032 / feature requests Image deletions? age: 1 day 16 hours url: http://drupal.org/node/30557 / support requests Images are there but not showing assigned: Stepfordtwit age: 6 weeks 1 day url: http://drupal.org/node/27615 Image Module Permissions age: 9 weeks 4 days url: http://drupal.org/node/26280 path problems on upgrading... age: 18 weeks 4 days url: http://drupal.org/node/21725 / bug reports RSS "no element found at line 1" age: 9 weeks 6 days url: http://drupal.org/node/26185 Outdated help texts age: 1 year 5 weeks url: http://drupal.org/node/9692 Import doesn't like XML/RSS feeds with no Title element age: 1 year 14 weeks url: http://drupal.org/node/8128 / tasks move XML-parser into an inclde file. age: 11 weeks 2 days url: http://drupal.org/node/25439 / feature requests Notify.module ignores the true submission queue age: 1 year 45 weeks url: http://drupal.org/node/3765 / feature requests Internationalization support for PDF assigned: TheLibrarian age: 1 year 23 weeks url: http://drupal.org/node/6795 / bug reports Reminder email links don't work age: 10 weeks 6 days url: http://drupal.org/node/25677 / bug reports access control problems age: 6 weeks 5 days url: http://drupal.org/node/27273 Download link is wrong age: 11 weeks 3 days url: http://drupal.org/node/25363 Projects not listed and page not complete age: 14 weeks 2 days url: http://drupal.org/node/23956 Date for latest is incorrect age: 21 weeks 12 hours url: http://drupal.org/node/20485 Invalid link to release files age: 28 weeks 4 days url: http://drupal.org/node/17625 Node body not complete w/o releases age: 30 weeks 6 days url: http://drupal.org/node/16733 blank page after install age: 31 weeks 1 day url: http://drupal.org/node/16575 / support requests Help with everything - nothing seems to work in the latest build age: 41 weeks 3 days url: http://drupal.org/node/13310 / bug reports Error with URL to recipe age: 1 week 3 days url: http://drupal.org/node/29885 / bug reports Can't get the inline tag to show series age: 17 weeks 2 days url: http://drupal.org/node/22520 / feature requests Request to generate email for all content changes/additions, including comments age: 7 weeks 5 days url: http://drupal.org/node/26824 / bug reports javascrip downloads square.gif for every category age: 29 weeks 2 days url: http://drupal.org/node/17366 / bug reports Sends trackbacks for unpublished nodes assigned: ankur age: 32 weeks 1 day url: http://drupal.org/node/16239

1 0