August 2005 - Development - lists.drupal.org

[drupal-devel] anyone interested in participating in SpikeSource's Open Testing Contest
by Kai Mai 19 Aug '05

19 Aug '05

Hi, I am wondering if anyone interested in participating in SpikeSource's Open Testing Contest (http://www.spikesource.com/events/opentestingcontest/) I see this is more of an opportunity to bring automated testing coverage to Drupal. I've used SimpleTest to implemented some automated tests for a Drupal module I wrote. SimpleTest is pretty solid in doing unit testings and web-based black-box testing. I went to SpikeSource's headquarter for their test festival where I saw the demo of using phpCoverage(http://www.spikesource.com/projects/phpcoverage/) and SimpleTest on Mambo. It's really cool. It's getting harder and harder to manual test a big and growing system like Drupal. Automated testings may be hard to implement initially. But it gives quick feedback on changes made to a system. Drupal should make use of SpikeSource's Free Project Test service at http://spikesource.com/spikewatch/index.jsp. We can pick which stacks we deploy Drupal and do the testings. I am not affiliated with SpikeSource. I just really like their testing services for open source projects. Thanks. Kai

1 0

[drupal-devel] [bug] conf_init should strip out port numbers in bootstrap.inc
by Morbus Iff 19 Aug '05

19 Aug '05

Issue status update for http://drupal.org/node/19934 Post a follow up: http://drupal.org/project/comments/add/19934 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: chx Reported by: bwooster47 Updated by: Morbus Iff Status: patch (code needs review) Attachment: http://drupal.org/files/issues/_ports_0.patch (2.25 KB) Grr. Parens in the wrong place. Right patch attached. Morbus Iff Previous comments: ------------------------------------------------------------------------ Sun, 03 Apr 2005 23:07:58 +0000 : bwooster47 The latest Drupal from CVS (4.6.0) offers multisite configuration files, but currently the conf_init function in includes/bootstrap.inc will also include the :portnum URL specification in the directory name, which does not seem right (and the colon character may cause problems on Mac/Windows). Snippet from bootstrap.inc: * Example for a fictitious site installed at * http://www.drupal.org/mysite/test/ the 'settings.php' is * searched in the following directories: ..... If the site is: http://www.drupal.org:8080/mysite/test/ the same list of directories should be searched as listed in the example in the file, and the :8080 should not be added to the directory name (I think...) I don't know enough PHP to submit a patch, but this is probably easy for those who know this stuff :-) ------------------------------------------------------------------------ Fri, 08 Apr 2005 07:08:53 +0000 : chx Attachment: http://drupal.org/files/issues/conf_init_ports.patch (768 bytes) Here you are. ------------------------------------------------------------------------ Fri, 08 Apr 2005 11:16:52 +0000 : Dries Won't commit. Removing the port seems like a bad idea (I could have two drupal sites at two different ports but otherwise identical URLs) and adding regex' all over the map isn't advised. If ":" is an invalid character, then there might be more such characters. I suggest we replace them with '.' and that we document this behavior much like we do with $db_url. ------------------------------------------------------------------------ Mon, 11 Apr 2005 23:25:51 +0000 : chx Attachment: http://drupal.org/files/issues/conf_init_ports_0.patch (1.35 KB) ------------------------------------------------------------------------ Tue, 19 Apr 2005 17:03:46 +0000 : bwooster47 Regarding update #3 - does this mean the code has made it in, and will be available in the next release? Secondly: I follow the logic for the update, and it makes sense for non-standard HTTP port numbers, but not sure if the logic is valid when port 80 is used. For example, http://yourdomain.com/ and http://yourdomain.com:80/ both will go to the exact same page, but looks like the search for the conf.php will be different. Not sure if this matters... ------------------------------------------------------------------------ Mon, 01 Aug 2005 01:23:28 +0000 : killes(a)www.drop.org patch still applies, my testsite kept working (is not on a non-standard port, though). ------------------------------------------------------------------------ Mon, 01 Aug 2005 05:31:56 +0000 : Dries I think the PHPdoc is confusing. The second example URL is non-existing/invalid. Did you meant showing a configuration file? I agree with bwooster that this gets tricky with the default port '80'. Do we need to discuss this some more? ------------------------------------------------------------------------ Sat, 06 Aug 2005 07:20:03 +0000 : Boris Mann I like the period (".") character as the delimiter for ports. If people really do tack on :80, then site admins can symlink domain.com.80 to domain.com (much as you would need to symlink www.domain.com to domain.com as well). I think this is largely a documentation issue. ------------------------------------------------------------------------ Fri, 19 Aug 2005 01:27:39 +0000 : Morbus Iff Attachment: http://drupal.org/files/issues/_ports.patch (2.25 KB) +1. This patch is absolutely required for my home development, which is behind an ISP that doesn't like webservers on port 80. I've attached a new patch that fixes the doc mistake from the last patch, as well as adds a note to the INSTALL.txt too. I'm with Boris on the :80 issue - if a developer is using it knowingly, he should be smart enough to put 2 and 2 together.

1 0

[drupal-devel] [bug] conf_init should strip out port numbers in bootstrap.inc
by Morbus Iff 19 Aug '05

19 Aug '05

Issue status update for http://drupal.org/node/19934 Post a follow up: http://drupal.org/project/comments/add/19934 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: chx Reported by: bwooster47 Updated by: Morbus Iff Status: patch (code needs work) Attachment: http://drupal.org/files/issues/_ports.patch (2.25 KB) +1. This patch is absolutely required for my home development, which is behind an ISP that doesn't like webservers on port 80. I've attached a new patch that fixes the doc mistake from the last patch, as well as adds a note to the INSTALL.txt too. I'm with Boris on the :80 issue - if a developer is using it knowingly, he should be smart enough to put 2 and 2 together. Morbus Iff Previous comments: ------------------------------------------------------------------------ Sun, 03 Apr 2005 23:07:58 +0000 : bwooster47 The latest Drupal from CVS (4.6.0) offers multisite configuration files, but currently the conf_init function in includes/bootstrap.inc will also include the :portnum URL specification in the directory name, which does not seem right (and the colon character may cause problems on Mac/Windows). Snippet from bootstrap.inc: * Example for a fictitious site installed at * http://www.drupal.org/mysite/test/ the 'settings.php' is * searched in the following directories: ..... If the site is: http://www.drupal.org:8080/mysite/test/ the same list of directories should be searched as listed in the example in the file, and the :8080 should not be added to the directory name (I think...) I don't know enough PHP to submit a patch, but this is probably easy for those who know this stuff :-) ------------------------------------------------------------------------ Fri, 08 Apr 2005 07:08:53 +0000 : chx Attachment: http://drupal.org/files/issues/conf_init_ports.patch (768 bytes) Here you are. ------------------------------------------------------------------------ Fri, 08 Apr 2005 11:16:52 +0000 : Dries Won't commit. Removing the port seems like a bad idea (I could have two drupal sites at two different ports but otherwise identical URLs) and adding regex' all over the map isn't advised. If ":" is an invalid character, then there might be more such characters. I suggest we replace them with '.' and that we document this behavior much like we do with $db_url. ------------------------------------------------------------------------ Mon, 11 Apr 2005 23:25:51 +0000 : chx Attachment: http://drupal.org/files/issues/conf_init_ports_0.patch (1.35 KB) ------------------------------------------------------------------------ Tue, 19 Apr 2005 17:03:46 +0000 : bwooster47 Regarding update #3 - does this mean the code has made it in, and will be available in the next release? Secondly: I follow the logic for the update, and it makes sense for non-standard HTTP port numbers, but not sure if the logic is valid when port 80 is used. For example, http://yourdomain.com/ and http://yourdomain.com:80/ both will go to the exact same page, but looks like the search for the conf.php will be different. Not sure if this matters... ------------------------------------------------------------------------ Mon, 01 Aug 2005 01:23:28 +0000 : killes(a)www.drop.org patch still applies, my testsite kept working (is not on a non-standard port, though). ------------------------------------------------------------------------ Mon, 01 Aug 2005 05:31:56 +0000 : Dries I think the PHPdoc is confusing. The second example URL is non-existing/invalid. Did you meant showing a configuration file? I agree with bwooster that this gets tricky with the default port '80'. Do we need to discuss this some more? ------------------------------------------------------------------------ Sat, 06 Aug 2005 07:20:03 +0000 : Boris Mann I like the period (".") character as the delimiter for ports. If people really do tack on :80, then site admins can symlink domain.com.80 to domain.com (much as you would need to symlink www.domain.com to domain.com as well). I think this is largely a documentation issue.

1 0

[drupal-devel] [bug] Call to undefined function check_plain in in xmlrpc.inc in drupal 4.5.5
by Uwe Hermann 19 Aug '05

19 Aug '05

Issue status update for http://drupal.org/node/29149 Post a follow up: http://drupal.org/project/comments/add/29149 Project: Drupal Version: 4.5.5 Component: other Category: bug reports Priority: normal Assigned to: Anonymous Reported by: underwaterguy Updated by: Uwe Hermann -Status: active +Status: patch (code needs review) Hi, thanks for the report. Please provide patches in a proper format, though, see http://drupal.org/patch. Uwe Hermann Previous comments: ------------------------------------------------------------------------ Thu, 18 Aug 2005 20:54:47 +0000 : underwaterguy Attachment: http://drupal.org/files/issues/xmlrpc.inc.underwaterguy.patch (342 bytes) I'm getting an error in my hourly cron run: Fatal error: Call to undefined function: check_plain() in /home/wadsworth/www/jeaun.com/includes/xmlrpc.inc on line 67 It appears that the new version of xmlrpc.inc in drupal 4.5.5 contains a call to the check_plain function, which I believe only exists in drupal 4.6. I'm guessing that those function calls should be changed to drupal_specialchars, since the two functions appear to be doing essentially the same thing. I've attached a patch that switches the function calls. I can't seem to upload the patch (although it's easy enough to make manually,) so here it is: 67c67 < return '<string>'. check_plain($xmlrpc_value->data) .'</string>'; --- > return '<string>'. drupal_specialchars($xmlrpc_value->data) .'</string>'; 80c80 < $return .= " <member><name>". check_plain($name) ."</name><value>"; --- > $return .= " <member><name>". drupal_specialchars($name) ."</name><value>"; ------------------------------------------------------------------------ Thu, 18 Aug 2005 20:56:42 +0000 : underwaterguy Okay, so the patch was uploaded. The preview function wasn't working for some reason, so I couldn't see that it was already there.

1 0

[drupal-devel] [task] Extend db_query()
by Cvbge 18 Aug '05

18 Aug '05

Issue status update for http://drupal.org/node/17656 Post a follow up: http://drupal.org/project/comments/add/17656 Project: Drupal Version: cvs Component: database system Category: tasks Priority: normal Assigned to: killes(a)www.drop.org Reported by: killes(a)www.drop.org Updated by: Cvbge Status: patch (code needs review) The part I was objecting previously looks ok now. I haven't tested the code though. I still think it'd be nice to change php NULL to real SQL NULLs, but I don't have how to do it (at least not with current approach). The best would be if there was (s)printf flag that would just ignore it's argument... Cvbge Previous comments: ------------------------------------------------------------------------ Mon, 21 Feb 2005 12:48:30 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db-query.patch (2.16 KB) We should make our database abstraction layer more robust and ensure that module authors can use it without string manipulations inside the query. Several queries use implode() to get their arguments into the query. This is undesirable as we rely on the module author to check the keys and values of such arrays for exploitation attempts. I have created the attached patch which shouldbe able to allow us to not use implode anymore. A minor problem is that all inserted values will be treated as strings. This might be a problem with PostgreSQL at least. However, the same strategy is already used in Drupal core without any complaints I know of. Summary: This patch will alow us to simplify some code in node.module, user.module, taxonomy.module and probably others. ------------------------------------------------------------------------ Mon, 21 Feb 2005 17:03:58 +0000 : killes(a)www.drop.org It's a patch. ------------------------------------------------------------------------ Mon, 21 Feb 2005 17:19:13 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db-query_0.patch (2.07 KB) Squeezed out two lines of code after consultation with Karoly. Adds only 10 loc (plus some docs). ------------------------------------------------------------------------ Mon, 21 Feb 2005 17:23:08 +0000 : chx Do I need to say +1? ------------------------------------------------------------------------ Thu, 03 Mar 2005 00:15:10 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db-query_1.patch (2.69 KB) After some discussion with Adrian at Drupal Con we found out that we do not know why node_save currently works with pgsql. It currently assumes that all db columns are strings. It seems to work but we should not rely on it. Here is a patch that checks for the type of field that is inserted. It needs testing. ------------------------------------------------------------------------ Tue, 26 Jul 2005 01:17:04 +0000 : drumm +1 for making this into an API. I've seen too many hacked together query builders in Drupal and Contrib. I have not tested. ------------------------------------------------------------------------ Wed, 27 Jul 2005 11:30:29 +0000 : Bèr Kessels untested. a big +1 for the feature ------------------------------------------------------------------------ Wed, 27 Jul 2005 11:41:52 +0000 : killes(a)www.drop.org the patch still applies. the new patch here updates node_save to use it. Untested. ------------------------------------------------------------------------ Wed, 27 Jul 2005 11:42:24 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/node-%a.patch (926 bytes) the patch still applies. the new patch here updates node_save to use it. Untested. ------------------------------------------------------------------------ Wed, 27 Jul 2005 11:45:17 +0000 : Cvbge "After some discussion with Adrian at Drupal Con we found out that we do not know why node_save currently works with pgsql. " Well, it does not work. Real life exampless of not-working include flexinode (my experience) and forum module (as someone reported). Probably also others. The bug occurs when a normal user (but with necessarily rights) adds a node and he has no controls (the 'moderated', 'sticky', 'published' etc). The, in node_load() $node->sticky, $node->moderated (and maybe others) are set to FALSE (or TRUE, but in this case it works). When doing printf("%s", FALSE) the FALSE is change to empty string. The sticky and moderated db fields are numeric and postgresql do not accept '' (empty string) as a value of integer type. The result is for example such error: warning: pg_query(): Query failed: ERROR: invalid input syntax for integer: "" in ..../includes/database.pgsql.inc on line 45. user error: query: INSERT INTO node (title, uid, type, teaser, status, moderate, promote, sticky, body, comment, created, changed, nid) VALUES('xx', '2', 'flexinode-1', '<div class="flexinode-body flexinode-1"><div class="flexinode-image-3"><div class="form-item"> <label>Zdjęcie:</label><br /> <img alt="xx" src="..../pliki/" /><br />Get original file (28KB) [1] </div> </div></div>', '1', '', '1', '', '<div class="flexinode-body flexinode-1"><div class="flexinode-image-3"><div class="form-item"> <label>Zdjęcie:</label><br /> <img alt="xx" src="..../pliki/" /><br />Get original file (28KB) [2] </div> < in ..../includes/database.pgsql.inc on line 62. [1] http://drupal.org/..../pliki//tmp/male.jpg [2] http://drupal.org/..../pliki//tmp/male.jpg ------------------------------------------------------------------------ Wed, 27 Jul 2005 13:03:14 +0000 : Cvbge Attachment: http://drupal.org/files/issues/node.module_5.diff (500 bytes) Here's a quick fix for 4.6 ------------------------------------------------------------------------ Wed, 27 Jul 2005 13:10:54 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/node-a.patch (1.8 KB) the last patch wasn't good. ------------------------------------------------------------------------ Mon, 08 Aug 2005 08:19:14 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db_api_taxonomy.patch (3.5 KB) Here is an additional patch for taxonomy.module it removes the two custom functions that are used for inserts and updates. Needs testing. @Cvbge: shoduld we have a better test than just is_numeric in _db_argument_type to fix the pgsql problems? We could also try to pass $value by reference and cast it to int. this function is only used for updates/inserts so we can afford an additional check. ------------------------------------------------------------------------ Mon, 08 Aug 2005 08:37:25 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db_query.patch (2.74 KB) Here's a revised version fo the original patch. _db_argument_type now gets the $value argument by reference and we check for bools too. numerics and bools are cast to int. ------------------------------------------------------------------------ Mon, 08 Aug 2005 10:34:36 +0000 : Bèr Kessels I cannot say anything about performance for I do not know how to benchmark this. But I tried it, And rewrote some queries in a custom module, to use it, and like it a lot. The code becomes cleaner, and better readable. But above all, i see this idea going into a very interesting and good direction: that of 'more' query builders in core'. a +1! ------------------------------------------------------------------------ Mon, 08 Aug 2005 10:51:14 +0000 : Cvbge I'll be talking about postgres db only. The patch won't work as expected in some cases. Let's say we have a text/char/other character column and we want to insert a text that looks like a number. The _db_argument_type() will recognize the value as a numeric/number etc and will use $key = %d format. Even not considering printf() numeric conversion this can lead to incorrect value inserted into db. Examples of such problematic texts are: 01 (leading 0), 2e2 (scientific notation), probably also text with trailing/leading white spaces. When inserting such data into db without quotes the postgres db will think this is a number and will change 010 to 10, 2e2 to 200 . Here is an example: piotr=> CREATE TABLE t(c CHAR(10)); piotr=> INSERT INTO t VALUES (01); piotr=> INSERT INTO t VALUES (2e2); piotr=> INSERT INTO t VALUES (-01); piotr=> INSERT INTO t VALUES (000); piotr=> SELECT * from t; c ------------ 1 200 -1 0 (4 rows) I think it's safe to use '%s' everywhere. I could not find it writtent directly in postgres documentation (although I remember reading it somewhere some time ago). Postgres will convert the string to correct integer/numeric/bool value. ------------------------------------------------------------------------ Sat, 13 Aug 2005 01:14:41 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db_query_0.patch (2.69 KB) Ok, the patch now does only distinguish between integerss and the rest. This is needed to decide whether we need quotes. ------------------------------------------------------------------------ Sat, 13 Aug 2005 09:40:59 +0000 : Cvbge I'm sorry, but I see no reason for this integer check. Can you explain why there is a need to differentiate between integers and other types? The only problem I see is with converting bool(FALSE). This value is converted to empty string ('') and when trying to insert it into numeric, float, bool, date (probably any column that is not text type) etc you get error. One solution would be to check for bool(FALSE) and convert it to 0. This would work for numeric-like and bool fields. But it would not work for DATA columns (would produce error). Also, entering '0' into text column might not be what the author wanted. But this is more a problem with the coder - if you have integer/date etc field, insert integer/date/etc data type, not bool! Side note: the integer-version, the one without '', will be used only for integers (i.e. ..., -2, -1, 0, 1, 2, ... see http://php.net/manual/en/language.types.integer.php) ------------------------------------------------------------------------ Sat, 13 Aug 2005 09:43:19 +0000 : Cvbge "DATA" should be "DATE" ------------------------------------------------------------------------ Sun, 14 Aug 2005 00:53:34 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/db_query_1.patch (2.71 KB) Ok, another attempt. There is indeed no real reason to use %d for integers, they work fine as strings and are converted by sprintf to strings independend of %d or %s. But it is the Right Thing(tm), so I kept it. Booleans, however, need the %d formatter in order to be converted to 0 and 1. TRUE to 1 always works, but FALSE to 0 only works with %d. All tests on php 4.3.x.

1 0

[drupal-devel] [feature] Query optimization in taxonomy_node_form
by fgm 18 Aug '05

18 Aug '05

Issue status update for http://drupal.org/node/29151 Post a follow up: http://drupal.org/project/comments/add/29151 Project: Drupal Version: 4.6.3 Component: taxonomy.module Category: feature requests Priority: minor Assigned to: fgm Reported by: fgm Updated by: fgm Status: patch (code needs review) Attachment: http://drupal.org/files/issues/taxonomy.module.463.patch (630 bytes) The current query on which taxonomy_node_form is based is of the form: SELECT v.*, n.type FROM [...] But the code does not make use of any of the results except v.vid, using DB server resources uselessly. Patch reduces the query to SELECT v.vid FROM [...] This reduces the size of the result set and, to some extent, the work to be performed by the DB server. fgm

1 0

[drupal-devel] [task] Query optimization in taxonomy_node_form
by fgm 18 Aug '05

18 Aug '05

Issue status update for http://drupal.org/node/29150 Post a follow up: http://drupal.org/project/comments/add/29150 Project: Drupal Version: cvs Component: taxonomy.module Category: tasks Priority: minor Assigned to: fgm Reported by: fgm Updated by: fgm Status: patch (code needs review) Attachment: http://drupal.org/files/issues/taxonomy.module.cvs.patch (614 bytes) The current query on which taxonomy_node_form is based is of the form: SELECT v.*, n.type FROM [...] But the code does not make use of any of the results except v.vid, using DB server resources uselessly. Patch reduces the query to SELECT v.vid FROM [...] This reduces the size of the result set and, to some extent, the work to be performed by the DB server. fgm

1 0

[drupal-devel] A Gallery of UI elements
by Adrian Simmons 18 Aug '05

18 Aug '05

There's a lot of talk about Drupal's UI these days, so I thought you might like take a look at this site I just came across - images and discussion of UI elements from various OS's. Not web applications I know, but it might provide some inspiration: http://www.guidebookgallery.org/ HTH -- adrinux (aka Adrian Simmons) <http://adrinux.perlucida.com> e-mail <mailto:adrinux@gmail.com> AOL/Yahoo IM: perlucida, Microsoft: adrian(a)perlucida.com

1 0

[drupal-devel] [task] Load as little as possible for autocomplete
by chx 18 Aug '05

18 Aug '05

Issue status update for http://drupal.org/node/29136 Post a follow up: http://drupal.org/project/comments/add/29136 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: chx Reported by: chx Updated by: chx Status: patch (code needs review) Attachment: http://drupal.org/files/issues/autocomplete_12.patch (11.08 KB) This is a patch which changes significantly the work of autocomplete. Instead of loading the full Drupal and using the menu system to find out the correct handler, we only bootstrap to the PAGE_CACHE phase, and load the necessary module only. Autocomplete functions need to check for permissions themselves! chx

3 6

[drupal-devel] FYI
by Dries Buytaert 18 Aug '05

18 Aug '05

http://www.phpbb.com/phpBB/viewtopic.php?f=14&t=316489 -- Dries Buytaert :: http://www.buytaert.net/

6 6