September 2005 - Development - lists.drupal.org

[drupal-devel] [feature] rename 'path alias' to 'custom url'
by moshe weitzman 09 Sep '05

09 Sep '05

Issue status update for http://drupal.org/node/30509 Post a follow up: http://drupal.org/project/comments/add/30509 Project: Drupal Version: cvs Component: path.module Category: feature requests Priority: normal Assigned to: moshe weitzman Reported by: moshe weitzman Updated by: moshe weitzman Status: patch (code needs review) Attachment: http://drupal.org/files/issues/custom_url.patch (1.71 KB) i have had several people frown and ask what the heck we mean in the posting UI by 'path alias'. Once I explain, they adore the feature and suggest the alternative wording 'custom url'. So here is a patch which changes the user facing name of this feature. Admin area is unchanged and probably shouldn't be since the concept of an alias is more important there. moshe weitzman

15 20

[drupal-devel] [task] Fix and enhance postgres and mysql support in updates.inc
by Cvbge 09 Sep '05

09 Sep '05

Issue status update for http://drupal.org/node/29082 Post a follow up: http://drupal.org/project/comments/add/29082 Project: Drupal Version: cvs Component: database system Category: tasks Priority: critical Assigned to: Cvbge Reported by: Cvbge Updated by: Cvbge Status: patch (code needs review) Attachment: http://drupal.org/files/issues/drupal-head-db_add_column-29082_1.diff (9.16 KB) Updated patch based on http://drupal.org/node/7582 Following functions are defined: db_add_key(&$ret, $table, $column); db_drop_column(&$ret, $table, $column); db_add_primary_key(&$ret, $table, $column); db_drop_primary_key(&$ret, $table); db_add_column(&$ret, $table, $column, $type, $attributes = array()); db_change_column(&$ret, $table, $column, $column_new, $type, $attributes = array()); drupal_pg_version(); db_change_column() still need some work, the rest should work as documented. Cvbge Previous comments: ------------------------------------------------------------------------ Wed, 17 Aug 2005 21:25:43 +0000 : Cvbge Postgres, since 6.4, has version() function so you can check db version from SQL. This can be used in updates.inc for example for conditional dropping of the column (which is available since 7.3, not 7.4 as stated in updates.inc). I need to check how the version string is returned in different versions before making changes. ------------------------------------------------------------------------ Thu, 18 Aug 2005 10:08:25 +0000 : Cvbge Attachment: http://drupal.org/files/issues/drupal_pg_version2.diff (1.01 KB) Here's a function returning postgres db version. I'm not sure if it fits in updates.inc, maybe it should be somewhere else. ------------------------------------------------------------------------ Fri, 19 Aug 2005 14:33:32 +0000 : Cvbge Attachment: http://drupal.org/files/issues/drupal_pg_version_and_other.diff (2.76 KB) Added 2 new functions to add or change column definition. The take care about mysql/pgsql. Code not tested, will test and update old code when I hear that this has a chanse to go in ;) ------------------------------------------------------------------------ Fri, 19 Aug 2005 17:03:39 +0000 : Cvbge Attachment: http://drupal.org/files/issues/updates.inc-pgsql_fix.diff (9.97 KB) Still more work needed. ------------------------------------------------------------------------ Sat, 20 Aug 2005 14:28:13 +0000 : Cvbge Attachment: http://drupal.org/files/issues/screenshot_3.jpg (119.33 KB) With attached patch I could make all updates without errors (my LOCKs patch was needed - http://drupal.org/node/22911) There are still some issues that should be addressed (see the patch). Also mysql update is not tested - please test. There are some problems when updating from 4.6 to cvs. Maybe the update should be made by 4.6 base itself? So I've started with fresh 4.6.3. After changing the base to cvs, patching with the LOCKs patch and simple debug helper patch, on update.php?op=update I get following errors: warning: pg_query(): Query failed: ERROR: column "cache" of relation "sessions" does not exist in /var/www/dt/d/includes/database.pgsql.inc on line 71. Warning: pg_query(): Query failed: ERROR: column "referer" of relation "watchdog" does not exist in /var/www/dt/d/includes/database.pgsql.inc on line 71 Bad query: Array ( [0] => SELECT u.*, s.* FROM users u INNER JOIN sessions s ON u.uid = s.uid WHERE s.sid = 'd5b1aa940eaeab37fe690ce9cc500396' AND u.status < 3 LIMIT 1 OFFSET 0 [1] => SELECT r.rid, r.name FROM role r INNER JOIN users_roles ur ON ur.rid = r.rid WHERE ur.uid = 1 [2] => SELECT * FROM access WHERE status = 1 AND type = 'host' AND LOWER('127.0.0.1') LIKE LOWER(mask) [3] => SELECT * FROM access WHERE status = 0 AND type = 'host' AND LOWER('127.0.0.1') LIKE LOWER(mask) [4] => SELECT data, created, headers, expire FROM cache WHERE cid = 'variables' [5] => SELECT COUNT(pid) FROM url_alias [6] => SELECT name, filename, throttle, bootstrap FROM system WHERE type = 'module' AND status = 1 [7] => UPDATE sessions SET uid = 1, cache = 0, hostname = '127.0.0.1', session = 'messages|a:0:{}', timestamp = 1124545039 WHERE sid = 'd5b1aa940eaeab37fe690ce9cc500396' [8] => INSERT INTO watchdog (uid, type, message, severity, link, location, referer, hostname, timestamp) VALUES (1, 'php', 'pg_query(): Query failed: ERROR: column "cache" of relation "sessions" does not exist in /var/www/dt/d/includes/database.pgsql.inc on line 71.', 2, '', '/dt/d/update.php?op=update', 'http://localhost/dt/d/update.php', '127.0.0.1', 1124545039) ) Fatal error: ERROR: column "referer" of relation "watchdog" does not exist query: INSERT INTO watchdog (uid, type, message, severity, link, location, referer, hostname, timestamp) VALUES (1, 'php', 'pg_query(): Query failed: ERROR: column "cache" of relation "sessions" does not exist in /var/www/dt/d/includes/database.pgsql.inc on line 71.', 2, '', '/dt/d/update.php?op=update', 'http://localhost/dt/d/update.php', '127.0.0.1', 1124545039) in /var/www/dt/d/includes/database.pgsql.inc on line 94 Warning: Unknown(): A session is active. You cannot change the session module's ini settings at this time. in Unknown on line 0 As we can see the problem is that there is no cache column is session table - that's update_130. Also there's no column 'referer' in watchdog table - that's update_142. So I've commented out update_130 and 142, added the columns by hand in the db and started again from the begining. After that I got another error: warning: pg_query(): Query failed: ERROR: column "access" of relation "users" does not exist in /var/www/dt/d/includes/database.pgsql.inc on line 71. Bad query: Array ( [0] => SELECT u.*, s.* FROM users u INNER JOIN sessions s ON u.uid = s.uid WHERE s.sid = 'd5b1aa940eaeab37fe690ce9cc500396' AND u.status < 3 LIMIT 1 OFFSET 0 [1] => SELECT r.rid, r.name FROM role r INNER JOIN users_roles ur ON ur.rid = r.rid WHERE ur.uid = 1 [2] => SELECT * FROM access WHERE status = 1 AND type = 'host' AND LOWER('127.0.0.1') LIKE LOWER(mask) [3] => SELECT * FROM access WHERE status = 0 AND type = 'host' AND LOWER('127.0.0.1') LIKE LOWER(mask) [4] => SELECT data, created, headers, expire FROM cache WHERE cid = 'variables' [5] => SELECT COUNT(pid) FROM url_alias [6] => SELECT name, filename, throttle, bootstrap FROM system WHERE type = 'module' AND status = 1 [7] => UPDATE sessions SET uid = 1, cache = 0, hostname = '127.0.0.1', session = 'messages|a:0:{}', timestamp = 1124547147 WHERE sid = 'd5b1aa940eaeab37fe690ce9cc500396' [8] => UPDATE users SET access = 1124547147 WHERE uid = 1 [9] => INSERT INTO watchdog (uid, type, message, severity, link, location, referer, hostname, timestamp) VALUES (1, 'php', 'pg_query(): Query failed: ERROR: column "access" of relation "users" does not exist in /var/www/dt/d/includes/database.pgsql.inc on line 71.', 2, '', '/dt/d/update.php?op=update', 'http://localhost/dt/d/update.php', '127.0.0.1', 1124547147) ) user error: query: UPDATE users SET access = 1124547147 WHERE uid = 1 in /var/www/dt/d/includes/database.pgsql.inc on line 94. That's update_136. So, disable update_136 and do it by hand and start again. After all that, when accessing update.php?op=update I noticed no errors. After pressing Update all updates were applied without any errors. But when accessed main page I got a screen as on attached screenshot. ------------------------------------------------------------------------ Sat, 20 Aug 2005 14:32:27 +0000 : Cvbge Attachment: http://drupal.org/files/issues/updates.inc-pgsql_fix4.diff (12.86 KB) And the mentioned patch. Critical because otherwise upgrades (at least for postgres) are impossibile. ------------------------------------------------------------------------ Sat, 20 Aug 2005 14:36:40 +0000 : Cvbge I need people to test mysql updates, as I changed mysql part too. ------------------------------------------------------------------------ Fri, 26 Aug 2005 11:40:59 +0000 : Cvbge Attachment: http://drupal.org/files/issues/drupal-head-db_add_column.2.diff (7.44 KB) A diffrent patch. Only adds 3 functions for adding and changing table columns, does not modifies any old updates. There are still some issues to be solved, for example: - when changing a column in mysql the PRIMARY KEY attribute is retained. In Postgres, due to the nature of changing process (i.e. dropping the column) this attribute is dropped. How to workaround it? Need to check other attributes too (e.g. NOT NULL, index etc). - add more attributes? Like primary key, key (mysql: synonim dla unique), unique, index, auto increment (mysql: must be indexed and must have a default) ? - in CHANGE when setting NOT NOLL, and DEFAULT is set, update the table to default WHERE column IS NULL ?

1 0

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

09 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: Bèr Kessels Status: patch (code needs work) a heads up: I ma redoing the patch. But decided no to change the name yet. That is a too big change. IT should be in a separate issue, IMO. 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. ------------------------------------------------------------------------ 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'), ...). ------------------------------------------------------------------------ Thu, 08 Sep 2005 20:51:24 +0000 : m3avrck 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 :) ------------------------------------------------------------------------ Fri, 09 Sep 2005 01:48:55 +0000 : tag In my initial battle to figure out this module's (quite nice) functionality, what tripped me up the most was actually the nomenclature. The way I keep it straight in my head now is to translate "input format" to "filter set". If you view those (the current) screens with this in mind, I think it's a lot easier to understand what's what. It sort of describes this in the help text, but the terms "filter" and "input format" don't really convey the relationship between the two -- there's no way to intuit that an "input format" is a collection of "filters". Not to my mind at least... and well, we know nobody reads instructions... Are there any technicalities that make renaming "input format" to "filter set" not accurate? Or does someone have a better term/terms to better show the relationship of these elements? (I'm aware of filters in servlet architectures but I don't think there's much of a collision with that usage). Anyhow, I guess this doesn't quite address the UI controls and/or layout, but would still help a lot. ------------------------------------------------------------------------ Fri, 09 Sep 2005 06:35:34 +0000 : Bèr Kessels I prefer filter set. In fact: I like it a lot. Anyone else ideas on this? ------------------------------------------------------------------------ Fri, 09 Sep 2005 10:18:18 +0000 : Dries I too had problems with the terminology; it took me months to get used to 'input formats' versus 'filters'. Using 'filter sets' might improve the situation -- especially from an administrator's point of view when you have to wrap your head around the UI/difference. I'm not native English, but 'filter set' sounds more explanatory, yet slightly more geeky so I'm not sure if it flies for users who don't need to understand the underlying concepts (eg. under the node and comment submission forms).

1 0

[drupal-devel] [bug] Node module is not marked "required", but is required by other required modules
by robertDouglass 09 Sep '05

09 Sep '05

Issue status update for http://drupal.org/node/21084 Post a follow up: http://drupal.org/project/comments/add/21084 Project: Drupal -Version: 4.6.0 +Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: Thox Updated by: robertDouglass -Status: closed +Status: patch (code needs review) Attachment: http://drupal.org/files/issues/node.module.required.txt (1.39 KB) I couldn't find the duplicate so I'm reopening this. There are 206 instances of node_list alone in the repository (core+contributions). I cycled through 50 or so of these, and none of them check for the existence of the node module. Our collective usage of the node module indicates that it is, in fact, a required module. robertDouglass Previous comments: ------------------------------------------------------------------------ Fri, 22 Apr 2005 09:17:27 +0000 : Thox The "node" module isn't marked as one of the required modules, but it is used by a handful of other modules which are required. Specifically, the system and block modules both call the function node_list(), without using it through module_invoke. ------------------------------------------------------------------------ Mon, 20 Jun 2005 08:50:01 +0000 : Thox Duplicate

1 0

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

09 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 work) I too had problems with the terminology; it took me months to get used to 'input formats' versus 'filters'. Using 'filter sets' might improve the situation -- especially from an administrator's point of view when you have to wrap your head around the UI/difference. I'm not native English, but 'filter set' sounds more explanatory, yet slightly more geeky so I'm not sure if it flies for users who don't need to understand the underlying concepts (eg. under the node and comment submission forms). 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. ------------------------------------------------------------------------ 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'), ...). ------------------------------------------------------------------------ Thu, 08 Sep 2005 20:51:24 +0000 : m3avrck 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 :) ------------------------------------------------------------------------ Fri, 09 Sep 2005 01:48:55 +0000 : tag In my initial battle to figure out this module's (quite nice) functionality, what tripped me up the most was actually the nomenclature. The way I keep it straight in my head now is to translate "input format" to "filter set". If you view those (the current) screens with this in mind, I think it's a lot easier to understand what's what. It sort of describes this in the help text, but the terms "filter" and "input format" don't really convey the relationship between the two -- there's no way to intuit that an "input format" is a collection of "filters". Not to my mind at least... and well, we know nobody reads instructions... Are there any technicalities that make renaming "input format" to "filter set" not accurate? Or does someone have a better term/terms to better show the relationship of these elements? (I'm aware of filters in servlet architectures but I don't think there's much of a collision with that usage). Anyhow, I guess this doesn't quite address the UI controls and/or layout, but would still help a lot. ------------------------------------------------------------------------ Fri, 09 Sep 2005 06:35:34 +0000 : Bèr Kessels I prefer filter set. In fact: I like it a lot. Anyone else ideas on this?

1 0

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

09 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: Bèr Kessels Status: patch (code needs work) I prefer filter set. In fact: I like it a lot. Anyone else ideas on this? 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. ------------------------------------------------------------------------ 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'), ...). ------------------------------------------------------------------------ Thu, 08 Sep 2005 20:51:24 +0000 : m3avrck 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 :) ------------------------------------------------------------------------ Fri, 09 Sep 2005 01:48:55 +0000 : tag In my initial battle to figure out this module's (quite nice) functionality, what tripped me up the most was actually the nomenclature. The way I keep it straight in my head now is to translate "input format" to "filter set". If you view those (the current) screens with this in mind, I think it's a lot easier to understand what's what. It sort of describes this in the help text, but the terms "filter" and "input format" don't really convey the relationship between the two -- there's no way to intuit that an "input format" is a collection of "filters". Not to my mind at least... and well, we know nobody reads instructions... Are there any technicalities that make renaming "input format" to "filter set" not accurate? Or does someone have a better term/terms to better show the relationship of these elements? (I'm aware of filters in servlet architectures but I don't think there's much of a collision with that usage). Anyhow, I guess this doesn't quite address the UI controls and/or layout, but would still help a lot.

1 0

[drupal-devel] [bug] comment preview "Required" is easily bypassed
by Jeremy 09 Sep '05

09 Sep '05

Issue status update for http://drupal.org/node/28420 Post a follow up: http://drupal.org/project/comments/add/28420 Project: Drupal Version: cvs Component: comment.module Category: bug reports Priority: normal Assigned to: Jeremy(a)kerneltrap.org Reported by: Jeremy(a)kerneltrap.org Updated by: Jeremy(a)kerneltrap.org Status: patch (code needs review) Any feedback on this patch? I have been running it on my website for a couple of weeks, and it has completely stopped the most persistent auto-spam scripts that had been posting poker type comments constantly to my site. Jeremy(a)kerneltrap.org Previous comments: ------------------------------------------------------------------------ Mon, 08 Aug 2005 01:55:34 +0000 : Jeremy(a)kerneltrap.org Setting "Preview comment" to "Required" does not strictly require that the comment be previewed first. This is being abused by spammers to quickly and efficiently post spam comments. I discovered this after I added a new feature to my new spam module [1] to auto-blacklist spammer IP addresses, allowing me to block comment spammers when they preview a comment and thus preventing them from ever inserting their spam into my database. I configured my comment module to "require" comment previews, and yet found that the comments were slipping past my filter. I finally realized what the spammer is doing is setting $_POST['op'] to 'Post comment', effectively bypassing the preview phase. I'm currently looking for a clean solution to this. At the moment the only idea I have is to generate a token at the preview phase, and validate the token at the post phase. Unfortunately the token would have to be stored in the databse between the preview and the post, which adds overhead. Alternatively, I've considered using a time-based hash which would constantly update depending on the time of day. This could easily be validated without storing anything in the database. If too long has gone between the preview and the post, an additional preview step would be required... The down side here is that the time-based hash would be publically available, and thus the spammer could easily duplicate it in their script. A private key could solve for that, but increases the complexity as it adds a configuration step. I have the feeling I'm missing a simpler, cleaner solution. Suggestions? [1] http://kerneltrap.org/jeremy/drupal/spam/ ------------------------------------------------------------------------ Mon, 08 Aug 2005 02:26:21 +0000 : moshe weitzman even if you get this fixed, won't these bots just add a preview step? this 'preview required' feature is designed to maintain high quality submissions by forcing users to proof read. it isn;t designed for security. i think you want to hook into comment_validate(). just add a hook here - there is already a hook_comment() waiting for you to add an operation. ------------------------------------------------------------------------ Mon, 08 Aug 2005 08:49:30 +0000 : Eaton I posted a patch a few days ago (http://drupal.org/node/28255) that adds validation and form construction hooks for comments. It's similar to the one that the captcha module uses, though it adds comment form_pre and form_post hooks instead of a single comment form hook. ------------------------------------------------------------------------ Mon, 08 Aug 2005 13:30:34 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_11.patch (2.5 KB) > even if you get this fixed, won't these bots just add a preview step? Eventually, yes, but it drastically changes their ability to fling spam at a site. As is, they simply have a script that shoots data out at high speed without having to wait for messages to return from the server. It is the server that is doing all the work, thus making it simple for a spammer to DoS a site. If "preview required" really meant "preview required", they would be forced to first automate clicking "preview", and then wait for a response before clicking "submit". This requires more resources on their side, and allows us to add delays after clicking "preview" (if we detect that they are a spammer) further using their resources. > this 'preview required' feature is designed to maintain high quality > submissions by forcing users to proof read. it isn;t designed for security. Regardless of the intention, I was misled to believe that configuring my site to require previews would require that all comments were first previewed. As a site administrator, I would prefer to know that "required" really means "required". > i think you want to hook into comment_validate(). just add a hook here - > there is already a hook_comment() waiting for you to add an operation. Yes and no. Ultimately yes that will work and will allow my spam module to prevent the spam from ever being posted. But it still leaves the greatest burden on the web server, instead of on the spammer. The spammer can still use a very simple script that only pushes data, and thus can generate spam at an unbelievable rapid rate. Here is an example patch to enforce "preview required". It's one idea, I'm sure there are better ones. ------------------------------------------------------------------------ Mon, 08 Aug 2005 14:01:38 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_12.patch (1.4 KB) Here's a second version of the patch that doesn't require any manual configuration. ------------------------------------------------------------------------ Mon, 08 Aug 2005 16:27:27 +0000 : Jose A Reyero I like this idea, and the patch looks good Still, I think it misses something, like some timestamp related hash, because once you get the hash code you can post multiple comments with that. Another problem I can think of is, what happens when a cron run happens between the preview and the post?? I'm afraid comments would get lost For this second problem, I think a key generated only once after module activation could do. About the first one....mmm... I'll sit down for a while and think..... ------------------------------------------------------------------------ Tue, 09 Aug 2005 12:27:20 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_13.patch (1.33 KB) > I think it misses something, like some timestamp related hash, because > once you get the hash code you can post multiple comments with that. Using a timestamp will mean that the comment form "expires". That is, if you wait too long to preview your comment, it will generate an error when you try to post. Yes, technically a spammer could post one real comment, then based on what was in the session from that they could post the same identical comment over and over, so long as it was attached to the same node. But this is not what they do, they try and spread their spam throughout your webpage. Furthermore, the spam module is perfectly capable of detecting and preventing this. > Another problem I can think of is, what happens when a cron run happens > between the preview and the post?? I'm afraid comments would get lost The key is only generated once, that's what the first test is about. In any case, in the unlikely event that the key were to change between preview and post they would simply have to post a second time. My earlier patch wasn't quite right, I was testing the token in the wrong place. This patch fixes that. BTW: This is beneficial for maintaining high quality submissions too, as prior to this change someone could: 1) enter a comment 2) press preview 3) completely change their comment (introducing a mistake) 4) press submit and the comment (mistake and all) would go into the database unpreviewed After this change: 1) enter a comment 2) press preview 3) completely change their comment (introducing a mistake) 4) press submit and they get an error because they didn't preview their changes - forcing them to preview once after any change ------------------------------------------------------------------------ Wed, 10 Aug 2005 03:50:33 +0000 : Jeremy(a)kerneltrap.org FWIW: I've been getting slammed by spam attacks this whole week. Installing this patch has made a huge difference. Well over 100 spam attempts per minute (sometimes two and three times that) and I hardly notice the spammer, whereas before it was choking my database. (Granted, the spammer has not yet upgraded his script to first preview, then submit. But even if he did it wouldn't help him as testing has verified that the new spam module would prevent the comments from ever getting to the database.) Additionally, user and anonymous (nonspam) comments continue to show up at a normal rate. ------------------------------------------------------------------------ Tue, 16 Aug 2005 14:08:04 +0000 : Jeremy(a)kerneltrap.org I would love to see _any_ discussion on this. Drupal is currently too easy to spam, with little effort on the spammer's side, and lots of resources wasted on the Drupal side. A patch like this will greatly increase the spammer's burden, and make it possible to effectively block even the most aggressive spammer attacks. ------------------------------------------------------------------------ Wed, 17 Aug 2005 16:24:04 +0000 : Jose A Reyero Well, this patch is definitely better than what we have, and would save some spam for sure. But maybe keeping track, at the session level, of generated hashes for a user, and then removing them when the comment is sent, could do the work. This way we can forget about previewing comments or not, and also the "permission" to post the comment would expire when the session expires. Any randomly generated value could do for this, no need for complex hashes, but having nid and pid in the hash would add some extra security. ------------------------------------------------------------------------ Wed, 17 Aug 2005 19:58:02 +0000 : breyten Jeremy, a big +1 on the idea, but why not generate the private key when it is actually needed (Ie, when displaying the comment form), instead of wasting a _cron() hook on it? ------------------------------------------------------------------------ Thu, 18 Aug 2005 03:20:02 +0000 : Jeremy(a)kerneltrap.org > Well, this patch is definitely better than what we have, and would save some spam for sure. It is continuing to work very well on my site, which seems to be under nearly perpetual spam attacks from multiple sources. > But maybe keeping track, at the session level, of generated hashes for a user, and then > removing them when the comment is sent, could do the work. The catch is: the key has to be something unique to the server, not guessable or learnable from the outside Simply storing the hash data in the session alone is not enough, as then the spammer could create any random data and store it in the session. That said, the hash could be generated off something other than the text of the comment as it is now, so that a preview is not required. I'll look at doing something like that and submit another patch. > This way we can forget about previewing comments or not, and also the "permission" to > post the comment would expire when the session expires. Any randomly generated value > could do for this, no need for complex hashes, but having nid and pid in the hash would > add some extra security. nid and pid alone are worthless, as they are easy to learn. The pid can always be 0 (spam is rarely attached to a pre-existing comment). The nid is obtained in the path of where the spam is being posted. The solution is a "private-key", which is what my patch adds. Then sure, hash the private key plus the nid and the pid, and you've got enough protection to prevent most spammers. To make it even more secure, automatic rey-keying could be easily accomplished. ------------------------------------------------------------------------ Thu, 18 Aug 2005 04:09:10 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_15.patch (1.18 KB) The attached patch: 1) gets rid of the _cron() hook 2) no longer requires that comments be previewed Prior to this patch, comment spammers were able to send data to a Drupal server acting as though they'd filled out a comment form and pressed submit. As they didn't actually use the form, they could submit spam comments at an obscene rate. With this patch, comment spammers will have to actually load the form, enter text, and press submit. Yes, that can still be automated, but it takes much more work and slows them down, as they have to wait for the entry form to load each time. Unfortunately a spammer could manually submit one comment, then re-use that same session info over and over to attach repeated spam comments to the same node. Such an attempt would be detected and blocked by the spam module if enabled, but again such a session re-use attack could be done without loading the form each time. Fortunately there is much less gain for a spammer to submit 100 spam comments on the same page, versus submitting 100 spam comments each on a different page as they do now. Ideas to improve upon this concept include: - re-key every day or week, changing the private key regularly to be sure it couldn't ever be permanently cracked - add a key table, and generate a unique key for every comment form. essentially, upon comment form creation generate a random key which is stored both in a database table and in the session. when a comment is submited, look for the key from the session in the database table, if a match is found delete it from the database table and post the comment. this would prevent session re-use, but adds overhead. i don't know if it's worth it, perhaps as an external module if the hooks were available. ------------------------------------------------------------------------ Fri, 19 Aug 2005 18:55:11 +0000 : drumm <?php form_set_error('error', t('Validation error, please be sure cookies are enabled on your browser.')); ?> form_set_error [2]()'s fist argument should be the name of a form field, not 'error.' Using (..., 'error') would be better in this case. And the actual message needs work. Since this is a hidden field I don't think it has anything to do with cookies. [2] http://drupaldocs.org/api/head/function/form_set_error ------------------------------------------------------------------------ Fri, 19 Aug 2005 18:56:41 +0000 : drumm The unclosed link in my last update was supposed to say drupal_set_message(..., 'error') ------------------------------------------------------------------------ Sat, 20 Aug 2005 16:00:15 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_16.patch (1.11 KB) drupal_set_message(..., 'error') isn't sufficient to prevent the comment from being posted. I have instead updated the patch to set the error on the hidden 'token' form field. I have updated the message to read: "Unable to validate your comment, please try again. If this error persists, please contact the site administrator." If you don't like the error message, better suggestions are welcome.

1 0

[drupal-devel] [bug] "Only variables can be passed by reference" to file_check_directory()
by danielc 09 Sep '05

09 Sep '05

Issue status update for http://drupal.org/node/26033 Project: Drupal Version: 4.6.2 Component: base system Category: bug reports Priority: critical Assigned to: Anonymous Reported by: danielc Updated by: danielc Status: patch Attachment: http://drupal.org/files/issues/file_create_path.diff (2.4 KB) A client of mine just upgraded to the latest snapshot of PHP 5.0.5-dev and started getting errors when file_check_directory() is called: Fatal error: Only variables can be passed by reference in c:\drupal\modules\system.module on line 653 This patch fixes the problem by assigning the values to variables before passing them to file_check_directory(). Thanks. (BTW: I'm no longer on on drupal-devel due to time constraints.) danielc

4 7

[drupal-devel] [task] Improvments to Filter UI
by tag 09 Sep '05

09 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: tag Status: patch (code needs work) In my initial battle to figure out this module's (quite nice) functionality, what tripped me up the most was actually the nomenclature. The way I keep it straight in my head now is to translate "input format" to "filter set". If you view those (the current) screens with this in mind, I think it's a lot easier to understand what's what. It sort of describes this in the help text, but the terms "filter" and "input format" don't really convey the relationship between the two -- there's no way to intuit that an "input format" is a collection of "filters". Not to my mind at least... and well, we know nobody reads instructions... Are there any technicalities that make renaming "input format" to "filter set" not accurate? Or does someone have a better term/terms to better show the relationship of these elements? (I'm aware of filters in servlet architectures but I don't think there's much of a collision with that usage). Anyhow, I guess this doesn't quite address the UI controls and/or layout, but would still help a lot. tag 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'), ...). ------------------------------------------------------------------------ Thu, 08 Sep 2005 20:51:24 +0000 : m3avrck 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 :)

1 0

[drupal-devel] [bug] phptemplate.engine: is_int/is_numeric confusion
by wiz 09 Sep '05

09 Sep '05

Issue status update for http://drupal.org/node/27781 Post a follow up: http://drupal.org/project/comments/add/27781 Project: Drupal Version: cvs Component: theme system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: wiz Updated by: wiz Status: patch Attachment: http://drupal.org/files/issues/phptemplate_node.patch (720 bytes) I've wondered why $node is not defined anymore in my theme's page.tpl.php. The reason is a modified expression in phptemplate.engine, which now reads: if ((arg(0) == 'node') && is_int(arg(1))) { arg(1) may be the node number, but it is a string variable, and therefore this check is always false. is_int should be replaced with the code given in the patch. wiz

3 2