- Development - lists.drupal.org

[drupal-devel] [bug] Menu, is not linked
by Uwe Hermann 22 Aug '05

22 Aug '05

Issue status update for http://drupal.org/node/17079 Post a follow up: http://drupal.org/project/comments/add/17079 Project: Drupal Version: cvs Component: menu system Category: bug reports Priority: minor Assigned to: Anonymous Reported by: pz Updated by: Uwe Hermann -Status: active +Status: patch (code needs review) Attachment: http://drupal.org/files/issues/menu_20.patch (747 bytes) Still valid in HEAD. Here's a patch. Uwe Hermann Previous comments: ------------------------------------------------------------------------ Wed, 09 Feb 2005 16:07:41 +0000 : pz When creating a new menu the text "The name to display for this link." appears as description for the title, but the menu name is never linked. So either the menu-name should have a link or the text should be changed. ------------------------------------------------------------------------ Sun, 13 Mar 2005 23:37:43 +0000 : killes(a)www.drop.org I may be a bit sense, but I really don't get it. Can you please elaborate? ------------------------------------------------------------------------ Mon, 14 Mar 2005 10:59:46 +0000 : pz What I mean is that when creating a new menu (not a menuitem) the text "The name to display for this link." appears as description for the input field (same text as when creating a menutime). Problem is it is not a link we are creating it is only the name of a menu, this name is never linked to anything. So either the description of the input field should change depending on if we are creating a menitem or a new menu, or it should be possible to add a link to the menuname.

1 0

[drupal-devel] no ?> needed at the end of modules
by Karoly Negyesi 22 Aug '05

22 Aug '05

Hi! Can anyone (Goba?) confirm or not confirm that ?> is unnecessary at the end of included PHP files (like Drupal modules)? If it turns out that it is not needed, I think we need to adapt this practice because this avoids that some whitespace at the end of cause havoc. I tested and this works with PHP4 but I'd like to see something more "official" on this. Regards NK

11 26

[drupal-devel] [bug] Teaser always truncated at 600 characters
by Goba 22 Aug '05

22 Aug '05

Issue status update for http://drupal.org/node/28211 Post a follow up: http://drupal.org/project/comments/add/28211 Project: Drupal Version: cvs Component: node.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: neale Updated by: Goba -Status: patch (code needs review) +Status: patch (code needs work) Neale, your code does not reflect the intentions in Drupal. The problem with this patch is that it forces a truncate call on the body, regardless of content, and then reduces the teaser size even more, to the first paragraph, first sentence, etc. Problems with your approach are: * If the first paragraph is too short, it will be the teaser, regardless of the set size. * If there are no paragraph or break tags in the truncated teaser part, the first sentence will be used (regardless of how short is it). * On the other hand, if the first paragraph is too long, and the closing tag is out of the initial truncated part, you will end up with broken HTML. Now all these are not the intention of the teaser generation code. Having the exact set length or shorter is sacrificed for having valid HTML, and complete content. While the current Drupal code might produce too long teasers, your code can easily end up producing too short ones. A teaser for this text with your patch would be: /Neale, your code does not reflect the intentions in Drupal./ Goba Previous comments: ------------------------------------------------------------------------ Thu, 04 Aug 2005 19:18:23 +0000 : neale Attachment: http://drupal.org/files/issues/diff_27 (1.16 KB) The truncation code in node_teaser doesn't make any sense to me: <?php foreach ($breakpoints as $point => $charnum) { if ($length = strpos($body, $point, $size)) { return substr($body, 0, $length + $charnum); } } ?> I don't understand: why is the offset given to strpos the maximum teaser size? This would cause teasers to only be nicely split if they are enough longer than the max. teaser size to have one of the special characters. But this guarantees the teaser will either be abruptly truncated, or over the max. teaser size! In fact, this is what I'm seeing on my installation. Attached is a patch that makes it work the way I think it was intended. At least, it makes more sense to me. ------------------------------------------------------------------------ Sun, 21 Aug 2005 19:49:15 +0000 : killes(a)www.drop.org changing version and status.

1 0

[drupal-devel] [bug] Fix 404 handling without clean URLs
by Steven 22 Aug '05

22 Aug '05

Issue status update for http://drupal.org/node/13594 Post a follow up: http://drupal.org/project/comments/add/13594 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Steven Reported by: Steven Updated by: Steven Status: patch (code needs work) As far as I can tell, the issue is still there. I can't make an updated patch atm. Either we need to check for a non-clean 404, or stop intercepting non-clean 404s. It affects setups where htaccess is being read, but mod_rewrite is not installed or not working. Steven Previous comments: ------------------------------------------------------------------------ Sun, 28 Nov 2004 15:08:31 +0000 : Steven Attachment: http://drupal.org/files/issues/404.patch (1.07 KB) When clean urls are enabled, non-existant pages are rewritten to www.site.com/?q=non/existant. The menu handler will throw a 404 on the non-existant Drupal path. However, if clean urls are not active, then the 404 page (index.php) is called as is. $GET['q'] will be empty, and the front page will be shown without any indication of the 404 or without a log entry in the watchdog. Note that this only happens if .htaccess is being read, but mod_rewrite is not present or enabled. Not too common, but still a bug. The attached patch fixes this problem by checking for a 404 in index.php and calling drupal_not_found() if needed. It also sets $_GET['q'] to request_uri(), so the 404'd URL shows up correctly in the watchdog (same as the rewrite rule). This should go in 4.5 as well as HEAD. Patch is for HEAD, but it should apply to 4.5. ------------------------------------------------------------------------ Sun, 28 Nov 2004 15:17:22 +0000 : Steven An alternative solution is to drop the ErrorDocument 404 from .htaccess, then the web server will handle them if there are no clean URLs. However I think the intended behaviour is for Drupal to log all 404s. ------------------------------------------------------------------------ Sun, 13 Mar 2005 20:44:58 +0000 : killes(a)www.drop.org Doesn't apply anymore. ------------------------------------------------------------------------ Fri, 19 Aug 2005 13:53:59 +0000 : killes(a)www.drop.org Steven what's up with this?

1 0

[drupal-devel] [bug] wrong search indexing in some cases
by Steven 22 Aug '05

22 Aug '05

Issue status update for http://drupal.org/node/25923 Post a follow up: http://drupal.org/project/comments/add/25923 Project: Drupal Version: 4.5.5 Component: search.module Category: bug reports Priority: normal Assigned to: robertgarrigos Reported by: robertgarrigos Updated by: Steven Status: patch (ready to be committed) This patch fixes the described issue, but do we want to be bothered maintaining 4.5 search? It's pretty darn crappy. Steven Previous comments: ------------------------------------------------------------------------ Tue, 28 Jun 2005 15:16:10 +0000 : robertgarrigos In some cases, search module doesn't index some words, for instance, when there are only tags between words. In that case they are indexed all together: This is part of a real node text in one of my web pages (in catalan): 1732/1735<br\><b>Instrumentació:</b> this got indexed like this in the search_index table: 17321735instrumentació 169 1 which means I couldn't get a search result over 'instrumentació' I fixed that by adding a white space into the code of search.moulde file: original file (lines 253-254): // Strip heaps of stuff out of it. $wordlist = preg_replace("'<[\/\!]*?[^<>]*?>'si", '', $wordlist); fixed file (lines 253-254): // Strip heaps of stuff out of it. $wordlist = preg_replace("'<[\/\!]*?[^<>]*?>'si", ' ', $wordlist); ------------------------------------------------------------------------ Wed, 29 Jun 2005 21:12:59 +0000 : benshell Have you tried this on 4.6.x? I read this issue because I'm also having search indexing problems, but this particular problem looks like it has been fixed on 4.6.1. On line 344 on the search.module, I'm reading this: // Strip off all ignored tags to speed up processing, but insert space before/after // them to keep word boundaries. $text = str_replace(array('<', '>'), array(' <', '> '), $text); $text = strip_tags($text, '<'. implode('><', array_keys($tags)) .'>'); ------------------------------------------------------------------------ Thu, 30 Jun 2005 06:02:29 +0000 : robertgarrigos No, I haven't. The web page I was having this problem is on a shared server running php 4, thus no way to get drupal 4.6 on it. ------------------------------------------------------------------------ Mon, 22 Aug 2005 00:08:03 +0000 : robertgarrigos This is not yet fixed with 4.5.5. Apparently there is no problem with 4.6.x versions. ------------------------------------------------------------------------ Mon, 22 Aug 2005 01:29:35 +0000 : robertgarrigos Attachment: http://drupal.org/files/issues/search.module_0.patch (607 bytes) I enclose a patch for this. Please, forgive me if this is not the right way of doing. It's the first time I'm using cvs with my macosx. Also the first time I'm using diff to get a patch file, so take it as a simple "hello world" patch file, which should work and fix the problem anyway. ------------------------------------------------------------------------ Mon, 22 Aug 2005 08:02:01 +0000 : Dries Robert: your patch looks OK, but I'll let Steven (UnConeD) review it.

1 0

[drupal-devel] [bug] user/register returns user deny when a user is already logged in
by gufo1 22 Aug '05

22 Aug '05

Issue status update for http://drupal.org/node/17664 Post a follow up: http://drupal.org/project/comments/add/17664 Project: Drupal Version: cvs Component: user system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: gufo1 Updated by: gufo1 Status: patch (code needs work) Anybody else, feeling this way? You may have a point. But, if the user is already registered and clicks this link, he can still have the message and the user page below. This way you let him know that he is in a 404 and save him a click of having to go back to the user pages. gufo1 Previous comments: ------------------------------------------------------------------------ Mon, 21 Feb 2005 15:46:56 +0000 : gufo1 Any user, even the administrator gets this user access error when he is already logged in. This causes confusion for users who are already logged in, instead it should redirect the user to the user page. ------------------------------------------------------------------------ Fri, 01 Jul 2005 18:55:33 +0000 : alexis Agree, I have the same problem with 4.6.2. ------------------------------------------------------------------------ Tue, 16 Aug 2005 11:28:48 +0000 : Tobias Maier Attachment: http://drupal.org/files/issues/user_register_return_to_userspage_user.modul… (1023 bytes) here comes a patch for this i needed relatively long to find out why this happens. the behavour was theoretically implemented but hook_menu() denied it... ------------------------------------------------------------------------ Tue, 16 Aug 2005 14:05:06 +0000 : breyten I'd rather opt for Drupal to display a message along the lines of 'You can't register an account because you're already logged in.', instead of an equally confusing redirect. Secondly, I'd like to see consistency: If you're logged in now, and try to request a new password, Drupal will give you a 403 as well.

1 0

[drupal-devel] [task] Improvments to Filter UI
by Bèr Kessels 22 Aug '05

22 Aug '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: Bèr Kessels Status: patch (code needs review) Attachment: http://drupal.org/files/issues/format_now_contains_roles.png (22.09 KB) and a screenshot Bèr Kessels 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.

1 0

[drupal-devel] [task] Improvments to Filter UI
by Bèr Kessels 22 Aug '05

22 Aug '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: Anonymous +Assigned to: Bèr Kessels Reported by: Bèr Kessels Updated by: Bèr Kessels -Status: patch (code needs work) +Status: patch (code needs review) Attachment: http://drupal.org/files/issues/filter_interface_improvements.patch (14.52 KB) by popular demand: a comma separated list. Bèr Kessels 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...

1 0

[drupal-devel] [bug] wrong search indexing in some cases
by Dries 22 Aug '05

22 Aug '05

Issue status update for http://drupal.org/node/25923 Post a follow up: http://drupal.org/project/comments/add/25923 Project: Drupal Version: 4.5.5 Component: search.module Category: bug reports Priority: normal Assigned to: robertgarrigos Reported by: robertgarrigos Updated by: Dries Status: patch (ready to be committed) Robert: your patch looks OK, but I'll let Steven (UnConeD) review it. Dries Previous comments: ------------------------------------------------------------------------ Tue, 28 Jun 2005 15:16:10 +0000 : robertgarrigos In some cases, search module doesn't index some words, for instance, when there are only tags between words. In that case they are indexed all together: This is part of a real node text in one of my web pages (in catalan): 1732/1735<br\><b>Instrumentació:</b> this got indexed like this in the search_index table: 17321735instrumentació 169 1 which means I couldn't get a search result over 'instrumentació' I fixed that by adding a white space into the code of search.moulde file: original file (lines 253-254): // Strip heaps of stuff out of it. $wordlist = preg_replace("'<[\/\!]*?[^<>]*?>'si", '', $wordlist); fixed file (lines 253-254): // Strip heaps of stuff out of it. $wordlist = preg_replace("'<[\/\!]*?[^<>]*?>'si", ' ', $wordlist); ------------------------------------------------------------------------ Wed, 29 Jun 2005 21:12:59 +0000 : benshell Have you tried this on 4.6.x? I read this issue because I'm also having search indexing problems, but this particular problem looks like it has been fixed on 4.6.1. On line 344 on the search.module, I'm reading this: // Strip off all ignored tags to speed up processing, but insert space before/after // them to keep word boundaries. $text = str_replace(array('<', '>'), array(' <', '> '), $text); $text = strip_tags($text, '<'. implode('><', array_keys($tags)) .'>'); ------------------------------------------------------------------------ Thu, 30 Jun 2005 06:02:29 +0000 : robertgarrigos No, I haven't. The web page I was having this problem is on a shared server running php 4, thus no way to get drupal 4.6 on it. ------------------------------------------------------------------------ Mon, 22 Aug 2005 00:08:03 +0000 : robertgarrigos This is not yet fixed with 4.5.5. Apparently there is no problem with 4.6.x versions. ------------------------------------------------------------------------ Mon, 22 Aug 2005 01:29:35 +0000 : robertgarrigos Attachment: http://drupal.org/files/issues/search.module_0.patch (607 bytes) I enclose a patch for this. Please, forgive me if this is not the right way of doing. It's the first time I'm using cvs with my macosx. Also the first time I'm using diff to get a patch file, so take it as a simple "hello world" patch file, which should work and fix the problem anyway.

1 0

[drupal-devel] [feature] move user-selected themes out of core
by TDobes 22 Aug '05

22 Aug '05

Issue status update for http://drupal.org/node/26302 Post a follow up: http://drupal.org/project/comments/add/26302 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: TDobes Reported by: Torenware Updated by: TDobes Status: patch (code needs review) Attachment: http://drupal.org/files/issues/usertheme.module.txt (3.06 KB) Here's a quick copy-and-paste module containing the code removed from core... it allows users (whom are granted the appropriate permission) to switch the theme with which they view the site. I made it as a proof-of-concept that this same functionality can easily be recreated by a standalone module. TDobes Previous comments: ------------------------------------------------------------------------ Mon, 04 Jul 2005 01:50:45 +0000 : Torenware Attachment: http://drupal.org/files/issues/add-user-themable-option.diff (1.86 KB) >From the forum thread How To Prevent Users From Changing The Theme. I asked: " I'm working on a module that uses organic groups. I want to allow different groups to choose different themes, but I don't want individual users to have that capability. I'm assuming that this is easy to do, but I can't find a setting. Can anybody point me in the right direction? Thanks, Rob " At sepeck's suggestion, I'm submitting a patch to do this. It's against 4.6.0, and modifies theme.inc and system.module. It creates a variable 'configurable_user_themes' which defaults to "enabled" (4.6.0's current behavior), and if activated, makes system.module remove the UI for selected a theme, and also causes theme.inc to ignore any setting for $user->theme if it exists. ------------------------------------------------------------------------ Mon, 04 Jul 2005 01:54:40 +0000 : Torenware Attachment: http://drupal.org/files/issues/add-user-themable-option_0.diff (1.86 KB) Didn't set this to "patch" initially, so I'm including the patch again. Not sure what the procedure is just yet. ------------------------------------------------------------------------ Mon, 04 Jul 2005 05:07:44 +0000 : TDobes -1 -- sorry, but I feel that we should use a different method for this sort of thing. Any theme that is checked in the admin-> themes screen should be selectable by users. Modules should get their list of available themes from list_themes(), which provides a list of all available themes, not just those that are selected for use by users. ------------------------------------------------------------------------ Mon, 04 Jul 2005 09:21:57 +0000 : Bèr Kessels I think we should simply rename the "enabled" into "selecatble". Or any better term. For the only thing tha checkbox does is enable it for users to select. It does not enable or disable the theme at all. ------------------------------------------------------------------------ Mon, 04 Jul 2005 18:56:53 +0000 : Torenware A couple of comments: To TDobbs: for most applications, you're right. But for some applications where it's important to the site that user experience is well tested, letting individual users set the theme is something of a nightmare; a problem with the theme is perceived as a problem with the site. So for /those/ sites (such as the one I'm currently working on), letting the user set the theme was APITA. Still, for a community site, I agree that the current behavior is fine. So I made the current behavior the default. To Bèr Kessels: Let me see if I understand what you mean. Currently, I've coded the UI to say in its English default: User theme settings Configurable Themes: * Disabled * Enabled Enable or disable user-configurable themes. When enabled, users can set their own theme to override the site's default theme. Would you prefer: User theme settings Selectable Themes: * Disabled * Enabled Enable or disable user-selectable themes. When enabled, users can select their own theme to override the site's default theme. I don't really want to lose the "Disable"/"Enable" part, because if you look at the current UI in 4.6, all similar settings are done that way. So using a checkbox would be inconsistent with the rest of the page. How's that? Rob ------------------------------------------------------------------------ Mon, 04 Jul 2005 21:31:14 +0000 : Bèr Kessels Attachment: http://drupal.org/files/issues/theme.png (17.19 KB) you indeed misunderstood me. We have a "enable theme" checkbox in the theme overview. That checkbos *does not enable anything* it only gives users the opportunity to select that theme. Please se the screeny (sorry, Dutch locale enabled) for what I mean. Th e second comumn. That chackbox. Just investigte its use. You will its complete and utter cruft, that checkbox. Ber ------------------------------------------------------------------------ Mon, 04 Jul 2005 23:20:55 +0000 : Torenware Ber, I have a site that may well use themes set at the group level, so unactivating the theme via admin/theme is not a great solution for me. The problem with your solution is that if a theme is deactivated, it is not available for anyone. I need multiple themes available, but need to make the rather confusing UI that appears in user/n/edit to go away. Your options will not do that. ------------------------------------------------------------------------ Mon, 04 Jul 2005 23:28:27 +0000 : Torenware Ber, /Please se the screeny (sorry, Dutch locale enabled) for what I mean. Th e second comumn. That chackbox. Just investigte its use. You will its complete and utter cruft, that checkbox./ I'm not following you. Could you please explain a bit more? Thanks, Rob ------------------------------------------------------------------------ Mon, 04 Jul 2005 23:37:13 +0000 : TDobes /I have a site that may well use themes set at the group level, so unactivating the theme via admin/theme is not a great solution for me./ Sure it is! Deactivating themes in admin->theme removes them *ONLY* from the list of themes available for users to select. It should not remove them from the list of available themes for modules to use (via. the $custom_theme variable). If themes are disappearing from the modules' lists, these are bugs in the modules and should be reported elsewhere. I was under the impression that you were writing your own module, which is why I suggested you use the list_themes function. This function returns all available themes, not just those that are selected in admin->themes. Ber has mentioned that the term "enabled" is poorly chosen and should be replaced with something else, such as "user-selectable". I agree with him. /The problem with your solution is that if a theme is deactivated, it is not available for anyone./ This is completely false. It is not available to be chosen in the "my account" screen, but it is still very readily available for use by modules through the $custom_theme variable. /I need multiple themes available, but need to make the rather confusing UI that appears in user/n/edit to go away. Your options will not do that./ Yes they do. How are you determining the list of available themes in your module? If the themes are disappearing when you deselect them from admin->themes, then your module is working improperly. ------------------------------------------------------------------------ Mon, 04 Jul 2005 23:59:25 +0000 : Torenware OK, if I'm understanding you, then indeed the UI in admin/themes is bad done enough to fool a native English speaker. What y'all from Europe make of some of the terms, I can't even guess :-) I'll see if the UI disappears if all of the items are "deactivated" makes the UI go away (and doesn't break Organic Groups...). But it sounds like the real "bug" is in the documentation. This string: / Select which themes are available to your users and specify the default theme. To configure site-wide display settings, click the "configure" task above. Alternately, to override these settings in a specific theme, click the "configure" link for the corresponding theme. / needs changing. Perhaps something like "Choose a default theme for your site, and select which themes your users can choose to override the default." For the record: I do not have a clue what "Alternately, to override these settings in a specific theme, click the "configure" link for the corresponding theme.". If someone will tell me what that means, maybe some of us can translate it into English :-) Then someone can translate it into whatever else we need to support. ------------------------------------------------------------------------ Sat, 20 Aug 2005 05:22:53 +0000 : TDobes Please see the patch I've attached to this issue [1], which renames "enabled" to "user-selectable"... this should hopefully clear up a lot of confusion. [1] http://drupal.org/node/29002 ------------------------------------------------------------------------ Sat, 20 Aug 2005 07:52:28 +0000 : robertDouglass I'd like to propose a different solution; create a "can select own theme" permission for users. I agree that there are cases where some people should be able to choose their theme and others shouldn't. Personally, I don't make many sites where you get to "skin" the site just for fun. More likely, I use theme selection for development purposes (I develop the next version of a site's theme and can switch back and forth, and so can the developer, but nobody else). If we had a permission that could be given to roles, then og group administrators could be given this permission, if I'm not mistaken. Would this fit the bill, Torenware? ------------------------------------------------------------------------ Sat, 20 Aug 2005 17:30:29 +0000 : Bèr Kessels personally I am all for removing that option alltogether. "for development reasons" is simply not a reason to leave this in. What about getting rid of this "let users select their alternative theme". And focus more on sections/themeswitchers ? ------------------------------------------------------------------------ Sat, 20 Aug 2005 18:52:03 +0000 : robertDouglass I'm not for removing it. There are a lot of efforts going forward to do things like theme an individual's blog differently (blog_theme module, I think), and this is very interesting, especially if combined with some form of theme editor. The best interface we have for choosing themes is part of the user edit page. Let's not get rid of it. What's wrong with an permission to allow users to choose their own theme? ------------------------------------------------------------------------ Sun, 21 Aug 2005 08:56:32 +0000 : Bèr Kessels There is nothnig wrong witha permission. But with the options in your profile. * Does anyone actually allow his users to choose his or her theme in the profile? I have never come across a drupal site that does this. Never! * As you state, there are loads of much better uses of personal themes, like letting a user choose a theme for his blog. Or a print theme, or a theme for better visibility. Or even a theme special for bots. sections, admin themes, etc. None of these cases benefit from the in-profile theme choice. I think this is some odd *nuke ripoff feature, that is only there, because no-one took the time to remove it. Really, have you ever seen this being used on a site? So to conclude: IMO the in-profile theme choice should go. We should focus on writing or improving modules that do advanced themeswitching. ------------------------------------------------------------------------ Sun, 21 Aug 2005 09:07:02 +0000 : robertDouglass Ok, I agree that the in-profile theme changing stuff is cruft. Better would be a module that lets users do this. It could even show up in the same place, but at least it wouldn't be in core. ------------------------------------------------------------------------ Sun, 21 Aug 2005 09:13:59 +0000 : TDobes In response to Bèr: "* Does anyone actually allow his users to choose his or her theme in the profile? I have never come across a drupal site that does this. Never! " Well, I use the feature. In fact, it was the reason I got interested in the Drupal theming system to begin with. I was extremely frustrated by users' inability to switch between chameleon styles and xtemplate templates in Drupal prior the the last major theming patch. However, I tend to agree with you that this feature could be removed from core. This sort of functionality could be added to the user profile page by a contrib module and implemented by using the $custom_theme variable. ------------------------------------------------------------------------ Sun, 21 Aug 2005 21:37:56 +0000 : Dries Agreed. Let's move this to a contributed module and let's tidy up the theme configuration page (unless I'm mistaken, the enabled-checkboxes could probably be removed?). ------------------------------------------------------------------------ Sun, 21 Aug 2005 21:43:23 +0000 : killes(a)www.drop.org Moving to cvs. I am delighted about this change. This "feature" annoyed me right from when I started to use Drupal... ------------------------------------------------------------------------ Mon, 22 Aug 2005 04:14:48 +0000 : TDobes Attachment: http://drupal.org/files/issues/no_more_user_selectable_themes.patch (5.39 KB) I've attached a patch which removes the option for selecting themes from the _user hook and the special handling of $user->theme from init_theme. While I did consider removing the "enabled" field, I decided against it. When all themes are always enabled, the number of sub-tabs of "configure" can become unmanagable. drumm's recent usability improvement [2] to the theme admin involved hiding tabs for those themes which are not enabled. Additionally, contrib modules which manipulate $custom_theme should be obtaining their list of selections by using list_themes(). Therefore, they too would benefit by displaying only those themes which have been enabled by the site admin. For example, a site admin may wish to make only certain themes available to other site developers who have the ability to switch themes in certain sections of the site. [2] http://drupal.org/node/29002

1 0