- Development - lists.drupal.org

[drupal-devel] [bug] fix empty form elements
by chx 24 Apr '05

24 Apr '05

Issue status update for http://drupal.org/node/18663 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: chx Reported by: killes(a)www.drop.org Updated by: chx Status: patch Attachment: http://drupal.org/files/issues/fix_form_1.patch (4.97 KB) Recursive version. chx Previous comments: ------------------------------------------------------------------------ March 10, 2005 - 02:51 : killes(a)www.drop.org If you select nothing from a set of checkboxes an empty string is save to the variables table. This is a problem if the calling code assumes, it would get an array. We should make it save an empty array. The problem is the form_hidden field that we add. even if we replace form_hidden($name, 0) by form_hidden($name, array()) it isn't going to work, because the empty array is eaten by htmlspecialchars. redLED found this bug and he does not like it: 02:53 < redLED> i just like things saying 'CRITICAL' in red 02:53 < redLED> it gives this warm impending doom feeling 02:53 < redLED> and is generally associated with nuclear reactors in people's subconsciousness Suggestions welcome. ------------------------------------------------------------------------ March 12, 2005 - 19:47 : Steven There is no way to fix it in form_checkboxes() itself afaik, because even without htmlspecialchars, it would still come out as "Array". You can only create arrays indirectly through form submissions, by naming your values "foo[]". But you can not create empty arrays this way. So it is really up to the module which uses form_checkboxes() to handle this. A simple is_array() should do the trick. ------------------------------------------------------------------------ March 28, 2005 - 17:27 : chx Attachment: http://drupal.org/files/issues/checkboxes.patch (1.71 KB) system_settings_save can not really cope with this situation. This makes a problem with empty node options. And the fact that we have a check everywhere we use form_checkboxes is error prone, too. Here is a patch. If this accepted I'll comb the core for dead code. For eg. the first three lines of taxonomy_save_vocabulary and so. ------------------------------------------------------------------------ March 28, 2005 - 17:32 : chx Attachment: http://drupal.org/files/issues/checkboxes_0.patch (1.71 KB) Oh NO! One strayed space! I fixed it before steven beheads me.... ------------------------------------------------------------------------ March 28, 2005 - 18:36 : chx Attachment: http://drupal.org/files/issues/checkboxes_1.patch (1.64 KB) Among other changes, I have moved the hidden field form_checkboxes[] out of $edit. I have included HTML code, yes, but this is a very special thing, so it does not merit changing form_input just 'cos of this. And you won't need to theme a hidden field... ------------------------------------------------------------------------ April 8, 2005 - 08:19 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_2.patch (1.9 KB) killes did not like the previous version. ------------------------------------------------------------------------ April 12, 2005 - 00:15 : chx Dries? ------------------------------------------------------------------------ April 12, 2005 - 20:15 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_3.patch (1.64 KB) ------------------------------------------------------------------------ April 12, 2005 - 20:39 : jhriggs I have to agree with Steven that it is up to the calling code...and I hardly think this is critical. Code that is expecting an array from form_checkboxes() should ensure that it is dealing with an array (i.e. is_array()). As for dealing with variables set through things like system_settings_save(), the same thing applies. The code that is retrieving the variable should ensure that it is working with an array. With regards to your patch, what's the difference if the calling code has to remember to check via is_array() or remember to call fix_checkboxes()? This method is no less error prone as it's just as easy to forget this call as it is to check for an array. That is unless you are suggesting that fix_checkboxes() gets called for every page load which seems rather crufty. I wrote the original patch to add form_checkboxes() -- which didn't exist -- and if I remember, this was the desired behavior if nothing was selected. Isn't this the same behavior one would see using a multiple form_select() if the user chooses nothing? They should behave the same. A single select widget is the same as a radio group, and a multiple select widget is the same as a group of checkboxes. They should behave the same; the select types just take up less realestate. ------------------------------------------------------------------------ April 12, 2005 - 22:23 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_4.patch (2.06 KB) Previous patch had index.php missing. jhriggs, the current problem is that a form_checkboxes expected to be an array. Currently, empty form_checkboxes is a string. That's bad. ------------------------------------------------------------------------ April 12, 2005 - 23:51 : jhriggs -1 on this patch. I still don't understand the problem. What do you mean by "form_checkboxes expected to be an array"? Is there code in core that assumes it is an array but it is not checked with is_array()? If so, let's add the missing checks. The same goes for contrib. This seems like swatting a fly with a sledge hammer to me. If we do go ahead and make this change, however, then we will need to change form_select() also (when multiple = TRUE). Radios and single select should be consistent, as should checkboxes and multiple select. ------------------------------------------------------------------------ April 23, 2005 - 12:35 : chx Attachment: http://drupal.org/files/issues/fix_form.patch (4.71 KB) See Gordon's complaint http://lists.drupal.org/archives/drupal-devel/2005-04/msg00934.html ------------------------------------------------------------------------ April 23, 2005 - 12:38 : chx Forgot to change the title. ------------------------------------------------------------------------ April 23, 2005 - 12:46 : chx It seems that it's not obvious why this is necessary. The calling code can not always handle itself the situation if the save is done by system_save_settings. The edge case is: I set up something, so variable_get no longer falls to its default value, later I decide to empty it. If we do not do something like this patch, system_save_settings can not know there is something to be set to 0/array() because it is just not in $_POST. ------------------------------------------------------------------------ April 23, 2005 - 14:23 : chx Attachment: http://drupal.org/files/issues/fix_form_0.patch (4.72 KB) Small bugfix. ------------------------------------------------------------------------ April 23, 2005 - 15:53 : chx doh. still has serious problems when the name is an array. stay tuned.

1 0

[drupal-devel] What do users do on my site?
by Gerhard Killesreiter 24 Apr '05

24 Apr '05

Hi all! Above question is interesting for all site admins. I found a nice tool called pathalizer: http://pathalizer.bzzt.net/ that attempts to answer the question by looking at the Apache logs and generates nice graphics through graphviz. Steven made the drupal.org logs available and here is the result: http://killes.drupaldevs.org/tmp/drupal.org.log.pdf Note that only the most important paths are shown. Maybe this can give us some ideas. Cheers, Gerhard

5 5

Re: [drupal-devel] Talk with Rasmus
by Gabor Hojtsy 23 Apr '05

23 Apr '05

>> I don't think there is a config option for this in PHP. At least it is >> not documented. They have the liberty to do whatever with PHP at >> Yahoo. If there is a such a setting, it should be documented. Rasmus? >> >> Goba > > There is no config option for this. And there is no real reason for > Drupal to fix this, other than it is rather silly to send completely > useless information like this over the wire. > > -Rasmus Other than assuming that some correct default data will get set, which is in itself a false assumption given that the source can be some intelligent script or scraper, or whatever... Goba

1 0

[drupal-devel] [bug] fix empty form elements
by chx 23 Apr '05

23 Apr '05

Issue status update for http://drupal.org/node/18663 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: chx Reported by: killes(a)www.drop.org Updated by: chx Status: patch doh. still has serious problems when the name is an array. stay tuned. chx Previous comments: ------------------------------------------------------------------------ March 10, 2005 - 02:51 : killes(a)www.drop.org If you select nothing from a set of checkboxes an empty string is save to the variables table. This is a problem if the calling code assumes, it would get an array. We should make it save an empty array. The problem is the form_hidden field that we add. even if we replace form_hidden($name, 0) by form_hidden($name, array()) it isn't going to work, because the empty array is eaten by htmlspecialchars. redLED found this bug and he does not like it: 02:53 < redLED> i just like things saying 'CRITICAL' in red 02:53 < redLED> it gives this warm impending doom feeling 02:53 < redLED> and is generally associated with nuclear reactors in people's subconsciousness Suggestions welcome. ------------------------------------------------------------------------ March 12, 2005 - 19:47 : Steven There is no way to fix it in form_checkboxes() itself afaik, because even without htmlspecialchars, it would still come out as "Array". You can only create arrays indirectly through form submissions, by naming your values "foo[]". But you can not create empty arrays this way. So it is really up to the module which uses form_checkboxes() to handle this. A simple is_array() should do the trick. ------------------------------------------------------------------------ March 28, 2005 - 17:27 : chx Attachment: http://drupal.org/files/issues/checkboxes.patch (1.71 KB) system_settings_save can not really cope with this situation. This makes a problem with empty node options. And the fact that we have a check everywhere we use form_checkboxes is error prone, too. Here is a patch. If this accepted I'll comb the core for dead code. For eg. the first three lines of taxonomy_save_vocabulary and so. ------------------------------------------------------------------------ March 28, 2005 - 17:32 : chx Attachment: http://drupal.org/files/issues/checkboxes_0.patch (1.71 KB) Oh NO! One strayed space! I fixed it before steven beheads me.... ------------------------------------------------------------------------ March 28, 2005 - 18:36 : chx Attachment: http://drupal.org/files/issues/checkboxes_1.patch (1.64 KB) Among other changes, I have moved the hidden field form_checkboxes[] out of $edit. I have included HTML code, yes, but this is a very special thing, so it does not merit changing form_input just 'cos of this. And you won't need to theme a hidden field... ------------------------------------------------------------------------ April 8, 2005 - 08:19 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_2.patch (1.9 KB) killes did not like the previous version. ------------------------------------------------------------------------ April 12, 2005 - 00:15 : chx Dries? ------------------------------------------------------------------------ April 12, 2005 - 20:15 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_3.patch (1.64 KB) ------------------------------------------------------------------------ April 12, 2005 - 20:39 : jhriggs I have to agree with Steven that it is up to the calling code...and I hardly think this is critical. Code that is expecting an array from form_checkboxes() should ensure that it is dealing with an array (i.e. is_array()). As for dealing with variables set through things like system_settings_save(), the same thing applies. The code that is retrieving the variable should ensure that it is working with an array. With regards to your patch, what's the difference if the calling code has to remember to check via is_array() or remember to call fix_checkboxes()? This method is no less error prone as it's just as easy to forget this call as it is to check for an array. That is unless you are suggesting that fix_checkboxes() gets called for every page load which seems rather crufty. I wrote the original patch to add form_checkboxes() -- which didn't exist -- and if I remember, this was the desired behavior if nothing was selected. Isn't this the same behavior one would see using a multiple form_select() if the user chooses nothing? They should behave the same. A single select widget is the same as a radio group, and a multiple select widget is the same as a group of checkboxes. They should behave the same; the select types just take up less realestate. ------------------------------------------------------------------------ April 12, 2005 - 22:23 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_4.patch (2.06 KB) Previous patch had index.php missing. jhriggs, the current problem is that a form_checkboxes expected to be an array. Currently, empty form_checkboxes is a string. That's bad. ------------------------------------------------------------------------ April 12, 2005 - 23:51 : jhriggs -1 on this patch. I still don't understand the problem. What do you mean by "form_checkboxes expected to be an array"? Is there code in core that assumes it is an array but it is not checked with is_array()? If so, let's add the missing checks. The same goes for contrib. This seems like swatting a fly with a sledge hammer to me. If we do go ahead and make this change, however, then we will need to change form_select() also (when multiple = TRUE). Radios and single select should be consistent, as should checkboxes and multiple select. ------------------------------------------------------------------------ April 23, 2005 - 12:35 : chx Attachment: http://drupal.org/files/issues/fix_form.patch (4.71 KB) See Gordon's complaint http://lists.drupal.org/archives/drupal-devel/2005-04/msg00934.html ------------------------------------------------------------------------ April 23, 2005 - 12:38 : chx Forgot to change the title. ------------------------------------------------------------------------ April 23, 2005 - 12:46 : chx It seems that it's not obvious why this is necessary. The calling code can not always handle itself the situation if the save is done by system_save_settings. The edge case is: I set up something, so variable_get no longer falls to its default value, later I decide to empty it. If we do not do something like this patch, system_save_settings can not know there is something to be set to 0/array() because it is just not in $_POST. ------------------------------------------------------------------------ April 23, 2005 - 14:23 : chx Attachment: http://drupal.org/files/issues/fix_form_0.patch (4.72 KB) Small bugfix.

1 0

[drupal-devel] playing with google maps and drupal
by Anselm Hook 23 Apr '05

23 Apr '05

Here is a summary of some playing around I did today with google maps and drupal. This is pretty easy for anybody to replicate and improve: http://civicmaps.org/gmaps/about.html Thought I would fire this off before heading down to SF this weekend. I'd be very interested in hearing what people think - is google maps a nice way to visualize drupal content? Even if not legal ultimately - maybe the metaphor itself is nice; tile maps + javascript can be applied to vmap0 and other datasets (admittedly with a lot more work - for example compare the speed and fluidity of google maps to http://64.91.125.89/maps/ ( a more traditional mapserver ) ). For simple visualization and providing of 'context' - as opposed to editing - does google maps win? You're welcome to join the cmaps mailing list noted above as well; I'm hoping to gather discussion there on a variety of technology agnostic ways to do maps in civicspace. - a

2 1

[drupal-devel] [task] Consistent return 'passive voice' in form_set_error() and drupal_set_message()
by rivena 23 Apr '05

23 Apr '05

Issue status update for http://drupal.org/node/15595 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: stefan nagtegaal Reported by: bertboerland(a)www.drop.org Updated by: rivena Status: patch Have to agree with Djun. You're going to run into a lot of problems changing all the you's to me's. It'd be creepy if my drupal install started talking about itself. Anisa. rivena Previous comments: ------------------------------------------------------------------------ January 14, 2005 - 22:59 : bertboerland(a)www.drop.org *My* drupal not *Yours*! I think that when one addresses the user or the administrator of a site, it is best to do this outside in instead of inside out. So when in the documentation the coder is referring to the site of the administrator, this site should be called "your site" (/inside out/) but "my site" (/outside in/). There are numours examples where the text in referring to "your"where "my"would be apopriate [1] There used to be some "your" in the userinterface for the enduser as well, but that has been solved in 4.5. Before that you had "my account", but "track your postings". This has now all beens replaced by "my"and "mine". If everybody agrees, lets also corect this for the administrator as well; my administrator, not yours ;-) [1] examples: admin/logs/referrers "...that point to your web site." admin/settings "General configuration options for your site" ------------------------------------------------------------------------ April 20, 2005 - 01:36 : stefan nagtegaal We should consistently use the 'passive voice', when returning messages using form_set_error() and drupal_set_message(). ------------------------------------------------------------------------ April 22, 2005 - 21:05 : stefan nagtegaal Attachment: http://drupal.org/files/issues/drupal_set_message consistency part 1 of 2 (51.16 KB) first patch of 2 to make the drupal_set_message(), form_set_error() and watchdog() messages more consistent. please review/comment and apply.. ------------------------------------------------------------------------ April 22, 2005 - 21:05 : stefan nagtegaal Setting status to Patch. ------------------------------------------------------------------------ April 23, 2005 - 17:49 : puregin A big -1 from me on replacing "you" with "me" :) Regarding the use of the first person - I find it extremely awkward and confusing to see usage such as 'My Account', "Here are my recent posts", etc. From my perspective as a native speaker of English, I find that this kind of usage violates conventional "conversational context". To clarify, if I am communicating with another person *via* the medium of a program then I expect that person to follow the normal conventions of conversation, in particular, to say "me, my, I" when referring to him/herself, and "you, your, etc." when referring to me. If I am communicating with a program, I *also* expect it to follow conversational conventions. Particularly obsequious programs may always refer to themselves in an indirect second person: "This unworthy program was unable to execute your most excellent command, and will henceforth terminate itself" I have to say I really don't like this kind of interaction. I like a program that tells me straight up "I can't find my configuration files! Sorry, I'm giving up!" People like, understand, and are extremely good at direct conversational interaction. It's intuitively clear who is speaking and what the relation to the listener is. I might tolerate a page with title "My Account", but really, a message such as 'This page shows my site-wide referrer statistics. I can optionally view just the "external referrers" or the "internal referrers". Referrers are web pages, both local and on other sites, that point to your my site.' is just plain confusing and unclear. To quote Sendmail: "I refuse to talk to myself" Please don't inflict this gratuitous use of first person on us! Djun

1 0

[drupal-devel] [bug] fix empty form elements
by chx 23 Apr '05

23 Apr '05

Issue status update for http://drupal.org/node/18663 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: chx Reported by: killes(a)www.drop.org Updated by: chx Status: patch Attachment: http://drupal.org/files/issues/fix_form_0.patch (4.72 KB) Small bugfix. chx Previous comments: ------------------------------------------------------------------------ March 10, 2005 - 02:51 : killes(a)www.drop.org If you select nothing from a set of checkboxes an empty string is save to the variables table. This is a problem if the calling code assumes, it would get an array. We should make it save an empty array. The problem is the form_hidden field that we add. even if we replace form_hidden($name, 0) by form_hidden($name, array()) it isn't going to work, because the empty array is eaten by htmlspecialchars. redLED found this bug and he does not like it: 02:53 < redLED> i just like things saying 'CRITICAL' in red 02:53 < redLED> it gives this warm impending doom feeling 02:53 < redLED> and is generally associated with nuclear reactors in people's subconsciousness Suggestions welcome. ------------------------------------------------------------------------ March 12, 2005 - 19:47 : Steven There is no way to fix it in form_checkboxes() itself afaik, because even without htmlspecialchars, it would still come out as "Array". You can only create arrays indirectly through form submissions, by naming your values "foo[]". But you can not create empty arrays this way. So it is really up to the module which uses form_checkboxes() to handle this. A simple is_array() should do the trick. ------------------------------------------------------------------------ March 28, 2005 - 17:27 : chx Attachment: http://drupal.org/files/issues/checkboxes.patch (1.71 KB) system_settings_save can not really cope with this situation. This makes a problem with empty node options. And the fact that we have a check everywhere we use form_checkboxes is error prone, too. Here is a patch. If this accepted I'll comb the core for dead code. For eg. the first three lines of taxonomy_save_vocabulary and so. ------------------------------------------------------------------------ March 28, 2005 - 17:32 : chx Attachment: http://drupal.org/files/issues/checkboxes_0.patch (1.71 KB) Oh NO! One strayed space! I fixed it before steven beheads me.... ------------------------------------------------------------------------ March 28, 2005 - 18:36 : chx Attachment: http://drupal.org/files/issues/checkboxes_1.patch (1.64 KB) Among other changes, I have moved the hidden field form_checkboxes[] out of $edit. I have included HTML code, yes, but this is a very special thing, so it does not merit changing form_input just 'cos of this. And you won't need to theme a hidden field... ------------------------------------------------------------------------ April 8, 2005 - 08:19 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_2.patch (1.9 KB) killes did not like the previous version. ------------------------------------------------------------------------ April 12, 2005 - 00:15 : chx Dries? ------------------------------------------------------------------------ April 12, 2005 - 20:15 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_3.patch (1.64 KB) ------------------------------------------------------------------------ April 12, 2005 - 20:39 : jhriggs I have to agree with Steven that it is up to the calling code...and I hardly think this is critical. Code that is expecting an array from form_checkboxes() should ensure that it is dealing with an array (i.e. is_array()). As for dealing with variables set through things like system_settings_save(), the same thing applies. The code that is retrieving the variable should ensure that it is working with an array. With regards to your patch, what's the difference if the calling code has to remember to check via is_array() or remember to call fix_checkboxes()? This method is no less error prone as it's just as easy to forget this call as it is to check for an array. That is unless you are suggesting that fix_checkboxes() gets called for every page load which seems rather crufty. I wrote the original patch to add form_checkboxes() -- which didn't exist -- and if I remember, this was the desired behavior if nothing was selected. Isn't this the same behavior one would see using a multiple form_select() if the user chooses nothing? They should behave the same. A single select widget is the same as a radio group, and a multiple select widget is the same as a group of checkboxes. They should behave the same; the select types just take up less realestate. ------------------------------------------------------------------------ April 12, 2005 - 22:23 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_4.patch (2.06 KB) Previous patch had index.php missing. jhriggs, the current problem is that a form_checkboxes expected to be an array. Currently, empty form_checkboxes is a string. That's bad. ------------------------------------------------------------------------ April 12, 2005 - 23:51 : jhriggs -1 on this patch. I still don't understand the problem. What do you mean by "form_checkboxes expected to be an array"? Is there code in core that assumes it is an array but it is not checked with is_array()? If so, let's add the missing checks. The same goes for contrib. This seems like swatting a fly with a sledge hammer to me. If we do go ahead and make this change, however, then we will need to change form_select() also (when multiple = TRUE). Radios and single select should be consistent, as should checkboxes and multiple select. ------------------------------------------------------------------------ April 23, 2005 - 12:35 : chx Attachment: http://drupal.org/files/issues/fix_form.patch (4.71 KB) See Gordon's complaint http://lists.drupal.org/archives/drupal-devel/2005-04/msg00934.html ------------------------------------------------------------------------ April 23, 2005 - 12:38 : chx Forgot to change the title. ------------------------------------------------------------------------ April 23, 2005 - 12:46 : chx It seems that it's not obvious why this is necessary. The calling code can not always handle itself the situation if the save is done by system_save_settings. The edge case is: I set up something, so variable_get no longer falls to its default value, later I decide to empty it. If we do not do something like this patch, system_save_settings can not know there is something to be set to 0/array() because it is just not in $_POST.

1 0

[drupal-devel] [bug] fix empty form elements
by chx 23 Apr '05

23 Apr '05

Issue status update for http://drupal.org/node/18663 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: chx Reported by: killes(a)www.drop.org Updated by: chx Status: patch It seems that it's not obvious why this is necessary. The calling code can not always handle itself the situation if the save is done by system_save_settings. The edge case is: I set up something, so variable_get no longer falls to its default value, later I decide to empty it. If we do not do something like this patch, system_save_settings can not know there is something to be set to 0/array() because it is just not in $_POST. chx Previous comments: ------------------------------------------------------------------------ March 10, 2005 - 02:51 : killes(a)www.drop.org If you select nothing from a set of checkboxes an empty string is save to the variables table. This is a problem if the calling code assumes, it would get an array. We should make it save an empty array. The problem is the form_hidden field that we add. even if we replace form_hidden($name, 0) by form_hidden($name, array()) it isn't going to work, because the empty array is eaten by htmlspecialchars. redLED found this bug and he does not like it: 02:53 < redLED> i just like things saying 'CRITICAL' in red 02:53 < redLED> it gives this warm impending doom feeling 02:53 < redLED> and is generally associated with nuclear reactors in people's subconsciousness Suggestions welcome. ------------------------------------------------------------------------ March 12, 2005 - 19:47 : Steven There is no way to fix it in form_checkboxes() itself afaik, because even without htmlspecialchars, it would still come out as "Array". You can only create arrays indirectly through form submissions, by naming your values "foo[]". But you can not create empty arrays this way. So it is really up to the module which uses form_checkboxes() to handle this. A simple is_array() should do the trick. ------------------------------------------------------------------------ March 28, 2005 - 17:27 : chx Attachment: http://drupal.org/files/issues/checkboxes.patch (1.71 KB) system_settings_save can not really cope with this situation. This makes a problem with empty node options. And the fact that we have a check everywhere we use form_checkboxes is error prone, too. Here is a patch. If this accepted I'll comb the core for dead code. For eg. the first three lines of taxonomy_save_vocabulary and so. ------------------------------------------------------------------------ March 28, 2005 - 17:32 : chx Attachment: http://drupal.org/files/issues/checkboxes_0.patch (1.71 KB) Oh NO! One strayed space! I fixed it before steven beheads me.... ------------------------------------------------------------------------ March 28, 2005 - 18:36 : chx Attachment: http://drupal.org/files/issues/checkboxes_1.patch (1.64 KB) Among other changes, I have moved the hidden field form_checkboxes[] out of $edit. I have included HTML code, yes, but this is a very special thing, so it does not merit changing form_input just 'cos of this. And you won't need to theme a hidden field... ------------------------------------------------------------------------ April 8, 2005 - 08:19 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_2.patch (1.9 KB) killes did not like the previous version. ------------------------------------------------------------------------ April 12, 2005 - 00:15 : chx Dries? ------------------------------------------------------------------------ April 12, 2005 - 20:15 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_3.patch (1.64 KB) ------------------------------------------------------------------------ April 12, 2005 - 20:39 : jhriggs I have to agree with Steven that it is up to the calling code...and I hardly think this is critical. Code that is expecting an array from form_checkboxes() should ensure that it is dealing with an array (i.e. is_array()). As for dealing with variables set through things like system_settings_save(), the same thing applies. The code that is retrieving the variable should ensure that it is working with an array. With regards to your patch, what's the difference if the calling code has to remember to check via is_array() or remember to call fix_checkboxes()? This method is no less error prone as it's just as easy to forget this call as it is to check for an array. That is unless you are suggesting that fix_checkboxes() gets called for every page load which seems rather crufty. I wrote the original patch to add form_checkboxes() -- which didn't exist -- and if I remember, this was the desired behavior if nothing was selected. Isn't this the same behavior one would see using a multiple form_select() if the user chooses nothing? They should behave the same. A single select widget is the same as a radio group, and a multiple select widget is the same as a group of checkboxes. They should behave the same; the select types just take up less realestate. ------------------------------------------------------------------------ April 12, 2005 - 22:23 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_4.patch (2.06 KB) Previous patch had index.php missing. jhriggs, the current problem is that a form_checkboxes expected to be an array. Currently, empty form_checkboxes is a string. That's bad. ------------------------------------------------------------------------ April 12, 2005 - 23:51 : jhriggs -1 on this patch. I still don't understand the problem. What do you mean by "form_checkboxes expected to be an array"? Is there code in core that assumes it is an array but it is not checked with is_array()? If so, let's add the missing checks. The same goes for contrib. This seems like swatting a fly with a sledge hammer to me. If we do go ahead and make this change, however, then we will need to change form_select() also (when multiple = TRUE). Radios and single select should be consistent, as should checkboxes and multiple select. ------------------------------------------------------------------------ April 23, 2005 - 12:35 : chx Attachment: http://drupal.org/files/issues/fix_form.patch (4.71 KB) See Gordon's complaint http://lists.drupal.org/archives/drupal-devel/2005-04/msg00934.html ------------------------------------------------------------------------ April 23, 2005 - 12:38 : chx Forgot to change the title.

1 0

[drupal-devel] [bug] fix empty form elements
by chx 23 Apr '05

23 Apr '05

Issue status update for http://drupal.org/node/18663 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: chx Reported by: killes(a)www.drop.org Updated by: chx Status: patch Forgot to change the title. chx Previous comments: ------------------------------------------------------------------------ March 10, 2005 - 02:51 : killes(a)www.drop.org If you select nothing from a set of checkboxes an empty string is save to the variables table. This is a problem if the calling code assumes, it would get an array. We should make it save an empty array. The problem is the form_hidden field that we add. even if we replace form_hidden($name, 0) by form_hidden($name, array()) it isn't going to work, because the empty array is eaten by htmlspecialchars. redLED found this bug and he does not like it: 02:53 < redLED> i just like things saying 'CRITICAL' in red 02:53 < redLED> it gives this warm impending doom feeling 02:53 < redLED> and is generally associated with nuclear reactors in people's subconsciousness Suggestions welcome. ------------------------------------------------------------------------ March 12, 2005 - 19:47 : Steven There is no way to fix it in form_checkboxes() itself afaik, because even without htmlspecialchars, it would still come out as "Array". You can only create arrays indirectly through form submissions, by naming your values "foo[]". But you can not create empty arrays this way. So it is really up to the module which uses form_checkboxes() to handle this. A simple is_array() should do the trick. ------------------------------------------------------------------------ March 28, 2005 - 17:27 : chx Attachment: http://drupal.org/files/issues/checkboxes.patch (1.71 KB) system_settings_save can not really cope with this situation. This makes a problem with empty node options. And the fact that we have a check everywhere we use form_checkboxes is error prone, too. Here is a patch. If this accepted I'll comb the core for dead code. For eg. the first three lines of taxonomy_save_vocabulary and so. ------------------------------------------------------------------------ March 28, 2005 - 17:32 : chx Attachment: http://drupal.org/files/issues/checkboxes_0.patch (1.71 KB) Oh NO! One strayed space! I fixed it before steven beheads me.... ------------------------------------------------------------------------ March 28, 2005 - 18:36 : chx Attachment: http://drupal.org/files/issues/checkboxes_1.patch (1.64 KB) Among other changes, I have moved the hidden field form_checkboxes[] out of $edit. I have included HTML code, yes, but this is a very special thing, so it does not merit changing form_input just 'cos of this. And you won't need to theme a hidden field... ------------------------------------------------------------------------ April 8, 2005 - 08:19 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_2.patch (1.9 KB) killes did not like the previous version. ------------------------------------------------------------------------ April 12, 2005 - 00:15 : chx Dries? ------------------------------------------------------------------------ April 12, 2005 - 20:15 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_3.patch (1.64 KB) ------------------------------------------------------------------------ April 12, 2005 - 20:39 : jhriggs I have to agree with Steven that it is up to the calling code...and I hardly think this is critical. Code that is expecting an array from form_checkboxes() should ensure that it is dealing with an array (i.e. is_array()). As for dealing with variables set through things like system_settings_save(), the same thing applies. The code that is retrieving the variable should ensure that it is working with an array. With regards to your patch, what's the difference if the calling code has to remember to check via is_array() or remember to call fix_checkboxes()? This method is no less error prone as it's just as easy to forget this call as it is to check for an array. That is unless you are suggesting that fix_checkboxes() gets called for every page load which seems rather crufty. I wrote the original patch to add form_checkboxes() -- which didn't exist -- and if I remember, this was the desired behavior if nothing was selected. Isn't this the same behavior one would see using a multiple form_select() if the user chooses nothing? They should behave the same. A single select widget is the same as a radio group, and a multiple select widget is the same as a group of checkboxes. They should behave the same; the select types just take up less realestate. ------------------------------------------------------------------------ April 12, 2005 - 22:23 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_4.patch (2.06 KB) Previous patch had index.php missing. jhriggs, the current problem is that a form_checkboxes expected to be an array. Currently, empty form_checkboxes is a string. That's bad. ------------------------------------------------------------------------ April 12, 2005 - 23:51 : jhriggs -1 on this patch. I still don't understand the problem. What do you mean by "form_checkboxes expected to be an array"? Is there code in core that assumes it is an array but it is not checked with is_array()? If so, let's add the missing checks. The same goes for contrib. This seems like swatting a fly with a sledge hammer to me. If we do go ahead and make this change, however, then we will need to change form_select() also (when multiple = TRUE). Radios and single select should be consistent, as should checkboxes and multiple select. ------------------------------------------------------------------------ April 23, 2005 - 12:35 : chx Attachment: http://drupal.org/files/issues/fix_form.patch (4.71 KB) See Gordon's complaint http://lists.drupal.org/archives/drupal-devel/2005-04/msg00934.html

1 0

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

23 Apr '05

Issue status update for http://drupal.org/node/18663 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: chx Reported by: killes(a)www.drop.org Updated by: chx Status: patch Attachment: http://drupal.org/files/issues/fix_form.patch (4.71 KB) See Gordon's complaint http://lists.drupal.org/archives/drupal-devel/2005-04/msg00934.html chx Previous comments: ------------------------------------------------------------------------ March 10, 2005 - 02:51 : killes(a)www.drop.org If you select nothing from a set of checkboxes an empty string is save to the variables table. This is a problem if the calling code assumes, it would get an array. We should make it save an empty array. The problem is the form_hidden field that we add. even if we replace form_hidden($name, 0) by form_hidden($name, array()) it isn't going to work, because the empty array is eaten by htmlspecialchars. redLED found this bug and he does not like it: 02:53 < redLED> i just like things saying 'CRITICAL' in red 02:53 < redLED> it gives this warm impending doom feeling 02:53 < redLED> and is generally associated with nuclear reactors in people's subconsciousness Suggestions welcome. ------------------------------------------------------------------------ March 12, 2005 - 19:47 : Steven There is no way to fix it in form_checkboxes() itself afaik, because even without htmlspecialchars, it would still come out as "Array". You can only create arrays indirectly through form submissions, by naming your values "foo[]". But you can not create empty arrays this way. So it is really up to the module which uses form_checkboxes() to handle this. A simple is_array() should do the trick. ------------------------------------------------------------------------ March 28, 2005 - 17:27 : chx Attachment: http://drupal.org/files/issues/checkboxes.patch (1.71 KB) system_settings_save can not really cope with this situation. This makes a problem with empty node options. And the fact that we have a check everywhere we use form_checkboxes is error prone, too. Here is a patch. If this accepted I'll comb the core for dead code. For eg. the first three lines of taxonomy_save_vocabulary and so. ------------------------------------------------------------------------ March 28, 2005 - 17:32 : chx Attachment: http://drupal.org/files/issues/checkboxes_0.patch (1.71 KB) Oh NO! One strayed space! I fixed it before steven beheads me.... ------------------------------------------------------------------------ March 28, 2005 - 18:36 : chx Attachment: http://drupal.org/files/issues/checkboxes_1.patch (1.64 KB) Among other changes, I have moved the hidden field form_checkboxes[] out of $edit. I have included HTML code, yes, but this is a very special thing, so it does not merit changing form_input just 'cos of this. And you won't need to theme a hidden field... ------------------------------------------------------------------------ April 8, 2005 - 08:19 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_2.patch (1.9 KB) killes did not like the previous version. ------------------------------------------------------------------------ April 12, 2005 - 00:15 : chx Dries? ------------------------------------------------------------------------ April 12, 2005 - 20:15 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_3.patch (1.64 KB) ------------------------------------------------------------------------ April 12, 2005 - 20:39 : jhriggs I have to agree with Steven that it is up to the calling code...and I hardly think this is critical. Code that is expecting an array from form_checkboxes() should ensure that it is dealing with an array (i.e. is_array()). As for dealing with variables set through things like system_settings_save(), the same thing applies. The code that is retrieving the variable should ensure that it is working with an array. With regards to your patch, what's the difference if the calling code has to remember to check via is_array() or remember to call fix_checkboxes()? This method is no less error prone as it's just as easy to forget this call as it is to check for an array. That is unless you are suggesting that fix_checkboxes() gets called for every page load which seems rather crufty. I wrote the original patch to add form_checkboxes() -- which didn't exist -- and if I remember, this was the desired behavior if nothing was selected. Isn't this the same behavior one would see using a multiple form_select() if the user chooses nothing? They should behave the same. A single select widget is the same as a radio group, and a multiple select widget is the same as a group of checkboxes. They should behave the same; the select types just take up less realestate. ------------------------------------------------------------------------ April 12, 2005 - 22:23 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_4.patch (2.06 KB) Previous patch had index.php missing. jhriggs, the current problem is that a form_checkboxes expected to be an array. Currently, empty form_checkboxes is a string. That's bad. ------------------------------------------------------------------------ April 12, 2005 - 23:51 : jhriggs -1 on this patch. I still don't understand the problem. What do you mean by "form_checkboxes expected to be an array"? Is there code in core that assumes it is an array but it is not checked with is_array()? If so, let's add the missing checks. The same goes for contrib. This seems like swatting a fly with a sledge hammer to me. If we do go ahead and make this change, however, then we will need to change form_select() also (when multiple = TRUE). Radios and single select should be consistent, as should checkboxes and multiple select.

1 0