This is the first check in of the new forms api code.

The system has been designed to co-exist with the current forms api, and is contained in a new
include file (includes/form.inc).

Forms are now defined in their component arrays, similar to how menu items are defined.

example :

 $form['body'] = array(type => 'textarea', default_value => $node->body, cols => 60, rows => 60);

Elements can also be nested, and the $edit follows this definition. For instance :

 $form['author'] = array(type => 'fieldset', title => t('Authoring information'), collapsible => TRUE, collapsed => TRUE, weight => -1);
 $form['author']['name'] = array(type => 'textfield', title => t('Authored by'), maxlength => 60, 
                                 autocomplete_path => 'user/autocomplete', default_value => $node->name, weight => -1);

All the properties used are defined as constants, and are documented for each of the elements, and individually.

CommentFileSizeAuthor
#149 watchdog.module.patch829 bytesTobias Maier
#147 contact-forms-api.patch19.52 KBasimmonds
#143 forms.tar_2.bz272.88 KBadrian
#140 form_5.inc33.27 KBchx
#137 forms.tar_1.bz272.16 KBadrian
#136 legacy.inc23.11 KBadrian
#135 form_4.inc33.22 KBadrian
#134 forms_7.patch411.59 KBadrian
#129 forms_6.patch406 bytesgordon
#126 form_render.module492 bytesgordon
#125 forms_5.patch435 bytesgordon
#122 drupal-head.throttle-form.junyor.patch4.61 KBjunyor
#121 drupal-head.update-php-form.junyor.patch1.09 KBjunyor
#118 Forms_2.rtf22.29 KBadrian
#117 Forms.ooutline66.43 KBadrian
#116 comment.module_29.patch58.74 KBeaton
#115 form.inc.txt28.7 KBadrian
#112 form_mega.patch196.88 KBeaton
#110 forms_4.patch118.54 KBhunmonk
#108 comment.module_28.patch59.92 KBeaton
#104 menu_forms.patch6.23 KBchx
#103 user-access_0.patch118.99 KBdries
#102 book.module_9.patch6.42 KBwebchick
#101 user-access-rules.patch7.07 KBdries
#98 forms.patch_2.txt122.36 KBadrian
#96 forum_forms.patch11.06 KBchx
#92 Forms_1.rtf22.05 KBadrian
#91 forms.patch_1.txt112.89 KBadrian
#88 issue29465_watchdog.module.patch1.8 KBTobias Maier
#87 forms.patch_0.txt105.93 KBadrian
#82 forms_system.module.patch29.24 KBBèr Kessels
#71 Forms.html10.02 KBadrian
#70 forms.tar_0.bz227.82 KBadrian
#66 Forms_0.rtf22.47 KBadrian
#65 forms.patch.txt108.18 KBadrian
#62 Forms.rtf14.02 KBadrian
#61 forms.tar.bz20 bytesadrian
#51 modif_Form_Inc.php2.52 KBnicopepe
#50 form_3.inc27.33 KBadrian
#44 forms_3.patch54.13 KBadrian
#43 form_2.inc26.76 KBadrian
#38 forms_2.patch47.72 KBjvandyk
#32 forms_1.patch46.21 KBchx
#24 forms_presentation.pdf78.63 KBadrian
#20 forms_0.patch37.68 KBadrian
#19 form_1.inc24.73 KBadrian
#17 form_0.inc21.01 KBchx
#16 forms_chx.patch15.59 KBchx
#15 form_defaults.patch2.39 KBmoshe weitzman
#2 forms.patch9.98 KBadrian
form.inc20.53 KBadrian

Comments

adrian’s picture

adrian commented

A patch for node.module, blog.module and taxonomy.module that changes them to use the new form format. This patch is very far from complete, but I wanted to get the code out so that i'm not working alone anymore.

adrian’s picture

adrian commented
StatusFileSize
new forms.patch9.98 KB

The actual patch =)

I forgot to mention, this adds a new hook .. namely hook_elements, which allows us to define the defaults for the elements (ie : cols and rows for textareas) meaning they don't have to be defined for each of the elements.

chx’s picture

chx commented

A few notes from my conversation with adrian. valid => array('integer', 'uid') for this to work you need function valid_integer($element) and valid_uid($element). $extra for form_select is legacy and really needed.

fago’s picture

fago
Primary language German
Location Vienna
commented

i really like this approach.

further i'd like to see the possibility to define an additional class to a form element, which is currently not working. so we 'd have to bring _form_get_class() and drupal_attributes() together.

adrian’s picture

adrian commented

that works already.

  $form[attributes]['class'] = 'someclass';

Although I am considering just adding a class property ...

ie:

  $form[class][] = 'someclass';

The fact that this is done via arrays, it means that the developer can add classes as he or she sees fit.

fago’s picture

fago
Primary language German
Location Vienna
commented

really?
i don't think so.

e.g.

 $checkbox = '<input type="checkbox" class="'. _form_get_class('form-checkbox', $element[required], _form_get_error($element[name])) .'" name="'. $element[name] .'" id="'. $element[id].'" value="'. $element[return_value] .'"'. ($element[value] ? ' checked="checked"' : '') . drupal_attributes($element[attributes]) .' />'

so we will end up with two class attributes, which won't work and isn't standard compliance.
your css property idea would be ideal imho.

nevets’s picture

nevets commented

Minor point on #5 and #6, when accessing an associated array like

$form[class][] = 'someclass';

if the key is a string it should be enclosed in quotes, i.e.

$form['class'][] = 'someclass';

(This is from the PHP documentation.)

moshe weitzman’s picture

moshe weitzman commented

Does this API affect form validation also? Thats the vague impression I had in my head, but I don't see any validation changed in node or taxonomy modules. perhaps that part is coming next.

There are reasons to love this patch. But one thing I don't like is the movement toward arrays and away from functions. Modern editors and IDE's offer function tips and function completion. These are huge time and brain savers. They are great for newbies and for experts. It is so helpful to just type 'form_sel', press tab, and have form_select('title', 'name', 'value', 'options') printed for you, with all the arguments. When you define forms in an array instead of functions, as proposed in this patch, you lose a lot of developer productivity and friendliness for newbies. Developers are also more prone to mistakes this way since the editor can't guide them along.

This is the sort of advantage that means nothing to the many people who just use a plain editor, and means everything to IDE users. Maybe someone can think of a way to keep the flexibility without losing IDE productivity.

adrian’s picture

adrian commented

The api has a drupal_validate_form() function, which does the following validation :

It steps through each of the elements, and executes any of the valid properties. An example would be valid => 'username'.
It then calls valid_username($element), which can check for errors.

It then calls $form_id_validate() , which can check for errors between form elements.

It then (optionally) calls $callback_validate(), which allows you to have unique form id's , similar to how the example does the node form.

You could create a function $type_node_form_validate(), to validate only that form, or a theme_$type_node_form() to theme that form differently.

An example of where this would be used is for CCK, where it will have a single callback for all nodes created by it.

Errors are flagged using form_error($element). It's different from form_set_error, in that it also sets the error property of the element, which I think is more practical than using the globals.

Regarding the IDE discussion, I am on the fence about that, but definitely leaning towards preferring the arrays over the function calls. I think that the menu system has proven itself, and that it's better to be consistent.

The plan for 4.7, is to leave the current form api in , so that all contrib modules don't need to be ported, but to switch core over to the new form system anyway, so for the time being .. the old functions will still exist.

All this would be a non-issue if php supported named parameters, which is essentially what we are reproducing using arrays.

Thomas Ilsche’s picture

Thomas Ilsche commented

I agree with moshe, and I think for day to day use the current forms api does a good job - however on complicated constructions i consider this to be really useful.
The problem i see is to keep the whole forms api consistent and easy to learn, any ideas?
I'd be against deprecating the current functions.

About the keyword definitions. I think it should be consistent with for hook_menu and all its "named parameter" friends, and to at least not confuse it more define the keywords without the leading underscore.

adrian’s picture

adrian commented

I'd prefer to make the menu system properties be consistent with the form system. actually.

I know the conventional logic is that all constants being uppercase, and the first versions of the form code did stick to that, but the end result
of the lowercase constants was far more readable code (the underscores however are a necessity to allow for nesting.)

What i was thinking was, that we could use the conventional form api as constructors for the form array.

ie: 

$group .= form_textfield(t('File system path'), 'file_directory_path', $directory_path, 60, 255, t('desc here'), null, etc)

turns into

 $form['files']['file_directory_path'] = form_textfield(t('File system path'), $directory_path, 60, 255, t('desc here'), null, etc);
 $form['files']['file_directory_path'][valid] = 'directory'; // any other properties that aren't in constructors.

instead of

  $form['files']['file_create_path'] = array( type => 'textfield', title => t('File system path'), default_value => $directory_path, maxlength => 255, valid => 'directory', description => t('desc here')  );

and form_textfield turns into 

  function form_textfield($title, $value, $size, $maxlength, $description = NULL, $attributes = NULL, $required = FALSE) {
    return array( title => $title, size => $size, maxlength => $maxlength, description = $description, attributes => $attributes, required => $required);
  }

Benefits :
1) easier to port
2) the ide thingy

Drawbacks :
1) more than one way to do something.
2) all forms will need to be upgraded, since the core form api will change. ie: breaks all of contrib.
3) Constructors could become unwieldy trying to tend to most of the parameters that can be set (weight, valid, validation_arguments, etc)

dries’s picture

dries commented

There should only be one way to build forms. Simplicity and uniformity is king. For now, I leave it up to Adrian to decide what this "one way" is going to look like. (I like his initial approach. The code is shorter which saves time too.)

chx’s picture

chx commented

I second Dries. The old form API be gone. The IDE is going to be a problem, yes. A possible approach: the default array have all keys possible set to NULL or so.

Bèr Kessels’s picture

Bèr Kessels commented

I prefer the One Way too. having more ways to do something normally results in two half-witted ways, instead of one way that works As Best As Possible.

I like the arrays approach. I love it, in fact. I have a feeling that the more code in drupal adopts the Array Way [tm], the more power AND uniformity we get. Just look at the success of array based menus: powerfull, yet simple to develop with.

But, I have a few hesitations: one is the lowercase CONSTANTS. I know, this is good for readability, so I lean towards the side of: then just use lowercase constants. But still, something does not feel right about it. I think this needs some more though, or comments of others.

Another thing I dislike is the way we use parameters to construct, IMO, completely different widgets. We should try to not think in terms of HTML, but in terms of usage and display. In HTML a collapsible form is similar -or nearly- to a noncollapsible. Same goes for a multi-select and none-multiselect select.
But where I really think we should have different APIs is for autofills. They are IMHO not textfields, but a complete separate widget. Thus they should get a separate API.

And last about the IDEs: allthough I do understand the problem, i beleive it is a very bad habit to let your code/application/product be limited, because of the tools you use. If your tools cannot handle libraries/snippets/etc beyond some function
calls, IMO you should get anoter IDE :). But surely we should not let us be held back by these limitations in these IDEs.

moshe weitzman’s picture

moshe weitzman commented
StatusFileSize
new form_defaults.patch2.39 KB

Um, snippits and macros are not a substitute for function complete. Snippits and macros are static entities which you manually create in the IDE and are then available as needed. If syntax changes for an given snippit, you have to manually change it. Thats just annoying enough to make you not use these feature at all. By contrast, function autocomplete just works the second you open a file. If you start working on a Contrib module you never seen before, the IDE introspects and is immediately ready for action. So all that nice PHPDoc that Adrian has written would be nicely used. Not so with snippits or macros.

If you guys want to see what the fuss about IDE is, download the free trial of Zend Studio (http://www.zend.com/store/products/zend-studio/) or Komodo (http://www.activestate.com/Products/Komodo/ - note the small link for the OSX alpha. I've tried the alpha and it works).

IDEs take a little while to get configured and get comfortable ... Create a project and start typing some common functions. See how the params show up and all our PHPDoc is there for your use. You can also quickly navigate your project via function names, and skip to the spot where a given function is defined. It is a tremendous time saver, and bug preventer. The IDEs above also offer a debugger which lets you step through your code and set breakpoints. Want to know the current value of a variable - just hover over it in your debugger. Once you've gotten the hang of this, you will never debug using print statements again.

I see nothing wrong with designing Drupal so that it uses those PHP language features that are friendly to IDEs. Namely, functions, classes, constants, etc. Arrays are very flexible, but that flexibility comes at a price.

chx’s picture

chx commented
StatusFileSize
new forms_chx.patch15.59 KB

Like we have core.php for hooks we can have form.php for form API and let that help your hand with any IDE.

Here is an updated version of Adrian's patch. User login block reworked. I move on to other parts of user.module. I also created a theme_password() function, so I will post form.inc as well.

chx’s picture

chx commented
StatusFileSize
new form_0.inc21.01 KB
gordon’s picture

gordon commented

+1 I like this patch a lot. but 2 things.

  1. The testarea hook is not supported. This needs to be implemented.
  2. the existing form_*() need to be changed to use the theme_*() so that there is only one place that form items can be created.

I think this will be great for theme developers and it will be easier to build forms for module developers.

Great job.

adrian’s picture

adrian commented
StatusFileSize
new form_1.inc24.73 KB

Here is an updated version of the patch.

1) admin / settings has been rewritten
2) Added form_radios and form_checkboxes and form_select and a few others
3) Very strict validation now .. the drupal_get_form function automatically validates any input .. no way to skip that. If it doesn't produce errors, it calls $form_id_execute(), or the optional $callback_execute().
4) Started the first part of the implementation of the multiple keyword, which is when an element can have multiple values. An example of this would be the 'files' element from upload.module , where more files get added, and also the primary / secondary links configuration of phptemplate.

Still needs to be done : The clean_url validation, and any other module with a _settings hook. I haven't integrated chx' user module work at this point.

adrian’s picture

adrian commented
StatusFileSize
new forms_0.patch37.68 KB

Here's the patch.

I'm also still busy with the filter format, which has unique requirements and needs to be a filter_format element type (gets rid of the in-line html)

Dave Cohen’s picture

Dave Cohen commented

One thing I like about the current API is that you can inject arbitrary HTML in the middle of a form if you see fit. It's not obvious to me how to do that with the new API. Is there a way?

Could I, for example, arrange my form fields in a table with two or three columns?

naudefj’s picture

naudefj commented

I would also like to be able to display forms in HTML tables. Here's a great article explaining how forms can be styled using tables - http://www.cs.tut.fi/~jkorpela/forms/tables.html

adrian’s picture

adrian commented

EVERY form will now have a theme function, as every form now has it's own (unique) form-id.

You can create a theme_my_form_id() function if you are a developer, and require the form to be differently layed out (like for instance, adding all the element outputs to a table).

You can create a mytheme_my_form_id() if you are a theme developer, that can override the form layout. (as is shown by the current node_form)
[?
function theme_node_form($form) {
$output .= '

';
$output .= '
';
$output .= '
';
$output .= form_render($form['author']);
$output .= '

';

$output .= '

';
$output .= form_render($form['options']);
$output .= '

';
$output .= '

';
$output .= '

';
$output .= form_render($form_render);
$output .= '

';
$output .= '

';
return $output;
}

?]
You can create a phptemplate stub to load it from a template.

function phptemplate_node_form($element) {
  return _phptemplate_default('node_form', $element);
}

and the current node_form template being :
[?

print form_render($form['author'])
print form_render($form['options'])
print form_render($form)

?]

NOTE: As a developer, you can even remix forms you didn't design. Infact, it gives you complete themeability over everything.

adrian’s picture

adrian commented
StatusFileSize
new forms_presentation.pdf78.63 KB

I hate the php filter ..
here's the theme_ function ..
i don't have time to escape the template right

function theme_node_form($form) {
  $output .= '<div class="node-form">';
    $output .= '<div class="admin">';
      $output .= '<div class="authored">';
      $output .= form_render($form['author']);
      $output .= '</div>';
      
      $output .= '<div class="options">';
      $output .= form_render($form['options']);
      $output .= '</div>';
    $output .= '</div>';
    $output .= '<div class="standard">';
    $output .= form_render($form_render);
    $output .= '</div>';
  $output .= '</div>';
  return $output;
}

I am attaching my presentation from drupalcon in portland.It has all the examples, although stuff like the validation has changed slightly.

adrian’s picture

adrian commented

Call for help :

Code freeze for Drupal 4.7 is coming very quickly , and a lot of work is still needed to get this functionality in before the code freeze happens.
As this patch is incredibly important (even if only considered from a security point of view), we need people to help us port all the forms in
Drupal at the moment.

If anyone is interested in helping, could you please contact me, so we can coordinate.

killes@www.drop.org’s picture

killes@www.drop.org commented

You can assign me a module to conver as soon as my revisions patch is in core. I think I'd like to convert profile.module.

Dave Cohen’s picture

Dave Cohen commented

Regarding #24:

Now I see why you keep the [printed] flag during node_render.

Should this line:
$output .= form_render($form_render);
read:
$output .= form_render($form);
??? (That is, I don't see where variable $form_render is coming from).

Regarding #25:

Must every form be converted in order to include this? Drupal 4.7 will still support the old forms API, yes?

I see that this was discussed early in the thread. For those of us with custom modules life would be a lot easier if the old form api sticks around until say a 5.0 release.

killes@www.drop.org’s picture

killes@www.drop.org commented

@yogadex: To those of us who roll out the actual security releases till 4am in the morning the disappearance of the old api with 4.7 will provide a lot more sleep. We win, backwards compatibility sucks.

adrian’s picture

adrian commented

The old form api will remain in core for a maximum of one release.

another 6-9 months to convert your modules is more than enough time... plus most of the new uber features (cck etc) are going to require you to upgrade to the new system.

Meanwhile, new functionality like the install wizards are going to require you to use this form api.

adrian’s picture

adrian commented

And regarding the form_render question .. it's not a variable, it's a small recursive function.

the theme function is passed the entire form tree. The designer can choose to remix the form however he chooses. Since elements are nested, and for instance the fieldsets, are named .. you can use form_render($form['elementname']) , which will print that element, and set the printed flag so it won't be printed again.

Like in the example where the author and options fieldsets are seperated out from the form and displayed seperately. form_render($form) in turn will print anything that hasn't been printed yet.

if you wanted to seperate out the author field on the page, you could use form_render($form['author']['name']) , and it would print that element wherever, and when you then called form_render($form['author']) or form_render($form); , it would not be printed again.

Dave Cohen’s picture

Dave Cohen commented

@killes: I certainly don't want you losing sleep ;) I'm glad to hear the new and old will coexist for at least one release. (Unless there's a security issue - that's another story)

@adrian: I understand the form_render() function and it's nifty. But there is a typo in both your attached presentation and #24 above, where "$form_render" is written and it should be "$form", if I am reading correctly. Thanks for explaining.

chx’s picture

chx commented
StatusFileSize
new forms_1.patch46.21 KB

I merged and rerolled against current HEAD.

m3avrck’s picture

m3avrck commented

Minor include bug in the node.module patch:

 define('NODE_NEW_LIMIT', time() - 30 * 24 * 60 * 60);
 
+include_once 'includes/form.inc';

that include_once is missing the './' part.

ax’s picture

ax commented

that second

+include_once 'includes/form.inc';

shouldn't be changed but *removed*, because it is a duplicate of 8 lines above.

m3avrck’s picture

m3avrck commented

ah yes, an even better catch than mine, touche!

chx’s picture

chx commented

Not so. The first should be removed 'cos it is hard to patch against the cvs id :)

ax’s picture

ax commented

allright, ok, of course, the first one.

what would be even better would be a working patch. the ones attached (last one and before) don't include form.inc, generate some duplicate functions that result in "PHP Fatal errors: Cannot redeclare functions()" (system_elements(), theme_node_form()), and "PHP Warning: Call-time pass-by-reference has been deprecated - argument passed by value"s. chx: are you working on this?

i would much like to help testing the new forms api and getting it into 4.7 because thats exactly what i need for a bigger project at work where we are considering using Drupal as base framework. the one showstopper is the extensability of Drupal forms ...

thanks for all the work done up to now, anyway.

jvandyk’s picture

jvandyk commented
StatusFileSize
new forms_2.patch47.72 KB

Here's an updated patch against HEAD that is mostly working. Karoly provided the updated node.module. For me, there seems to be an issue in form.inc in the recursive _form_builder() function where it is recursing even when $element is not an array but is a scalar.

m3avrck’s picture

m3avrck commented

Not to nitpick, but in node.module, the patch has +include_once 'includes/form.inc'; but should read +include_once './includes/form.inc'; for consistency and performance :)

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

I don't know if this is just the Drupal newbie in me talking, but I am getting quite a few errors with this patch (Drupal HEAD, up-to-date as of about 5 minutes ago). I've uploaded the form.inc from #19 and applied the forms_2.patch from #38, and I'm getting errors like the following:

admin/settings:
warning: Invalid argument supplied for foreach() in $DRUPAL_ROOT/includes/form.inc on line 511.

admin/comment/configure, admin/settings/user:
warning: Missing argument 2 for system_settings_form() in $DRUPAL_ROOT/modules/system.module on line 733.

node/add/[anything]:
warning: Cannot use a scalar value as an array in $DRUPAL_ROOT/includes/form.inc on line 406.

warning: Invalid argument supplied for foreach() in $DRUPAL_ROOT/includes/form.inc on line 403.

warning: Cannot use a scalar value as an array in $DRUPAL_ROOT/includes/form.inc on line 411.

warning: Cannot use a scalar value as an array in $DRUPAL_ROOT/includes/form.inc on line 406.

warning: Invalid argument supplied for foreach() in $DRUPAL_ROOT/includes/form.inc on line 403.

warning: Cannot use a scalar value as an array in $DRUPAL_ROOT/includes/form.inc on line 411.

warning: Cannot use a scalar value as an array in $DRUPAL_ROOT/includes/form.inc on line 406.

warning: Invalid argument supplied for foreach() in $DRUPAL_ROOT/includes/form.inc on line 403.

warning: Cannot use a scalar value as an array in $DRUPAL_ROOT/includes/form.inc on line 411.

warning: Cannot use a scalar value as an array in $DRUPAL_ROOT/includes/form.inc on line 406.

warning: Invalid argument supplied for foreach() in $DRUPAL_ROOT/includes/form.inc on line 403.

warning: Cannot use a scalar value as an array in $DRUPAL_ROOT/includes/form.inc on line 411.

warning: Cannot use a scalar value as an array in $DRUPAL_ROOT/includes/form.inc on line 406.

warning: Invalid argument supplied for foreach() in $DRUPAL_ROOT/includes/form.inc on line 403.

warning: Cannot use a scalar value as an array in $DRUPAL_ROOT/includes/form.inc on line 411.

warning: Invalid argument supplied for foreach() in $DRUPAL_ROOT/includes/form.inc on line 435.

warning: Cannot use a scalar value as an array in $DRUPAL_ROOT/includes/form.inc on line 448.

warning: Invalid argument supplied for foreach() in $DRUPAL_ROOT/includes/form.inc on line 435.

warning: Cannot use a scalar value as an array in $DRUPAL_ROOT/includes/form.inc on line 448.

warning: Invalid argument supplied for foreach() in $DRUPAL_ROOT/includes/form.inc on line 435.

warning: Cannot use a scalar value as an array in $DRUPAL_ROOT/includes/form.inc on line 448.

warning: Invalid argument supplied for foreach() in $DRUPAL_ROOT/includes/form.inc on line 435.

warning: Cannot use a scalar value as an array in $DRUPAL_ROOT/includes/form.inc on line 448.

warning: Invalid argument supplied for foreach() in $DRUPAL_ROOT/includes/form.inc on line 435.

warning: Cannot use a scalar value as an array in $DRUPAL_ROOT/includes/form.inc on line 448.

---

Any ideas?

jvandyk’s picture

jvandyk commented

That's why I said "mostly working." :)

m3avrck, sorry, I diffed against the copy where I hadn't corrected the ./ thing.

adrian’s picture

adrian commented

Good morning guys.
I am looking at those errors now, and i will upload an updated forms.inc in a short while.

adrian’s picture

adrian commented
StatusFileSize
new form_2.inc26.76 KB

Here is an updated patch and includes/form.inc.

Changes :
1 ) Introduce a global $form_variables array, which works exactly like $_POST['edit'] used to, except the values in it are filtered through the form system and only values that have form elements will be in it. Each form being processed on a page resets this array, so they won't interfere with each other.
2) introduce a form_id hidden that gets submitted with the page. This means it can intelligently decide which form to process.
3) Fix the bug above. I wrapped the offending lines in if is_array() , HOWEVER .. at the point I call that code I am fairly certain I am only working on arrays, hence I need to look it over more
4) fixed the user login form on /user. The user login block still doesn't work
5) Added a default css class (and some temporary css in drupal.css) for everything rendered with the form api. All other forms will have a thick red border if they have not been ported.
6) fixed a bug with element_info where it was setting the wrong defaults and then messing up the first form element because of it.
7) finished the filter module 'filter_format' element. Filter forms are printing now.
8) Added the 'weight' element. Could be cool to theme this with something ajaxy, right ?
9) Fixed some bugs with admin/system
10) Updated the following modules : Book, Page, Story, Blog, Node, Taxonomy and a fixed user form.
11) Moved the inclusion of form.inc to it's rightful place in drupal_bootstrap_full.
12) Added a bunch of comments to places. We will still have to go through all the comments and make sure they all 'parse' =)

The node form is not actually working or validated at present, and if someone could take a look at that , I'd be much obliged. It's important to get used to the new (strict) workflow for forms. It generally means having to shift some code around, and it makes it harder to write 'bad code'.

adrian’s picture

adrian commented
StatusFileSize
new forms_3.patch54.13 KB

here's the patch

dries’s picture

dries commented

introduce a form_id hidden that gets submitted with the page. This means it can intelligently decide which form to process.

Does this mean people could change the ID before they submit the form? I'm not sure that is a good idea.

adrian’s picture

adrian commented

The access check is tied to the page, not to the form.

If the page has 2 forms on the page, this check is to see which of the two he submitted.

Changing the form_id in the hidden will do nothing but cause the form not to execute, at all.

m3avrck’s picture

m3avrck commented

Running this patch I'm getting this error Fatal error: _drupal_bootstrap_full() [function.require]: Failed opening required './includes/form.inc'

Didn't see the file anywhere in the patch, unless I missed something...

adrian’s picture

adrian commented

drupal.org renames the files to avoid conflicts.

it's attached as form_2.inc

asimmonds’s picture

asimmonds commented

Should there be "return_value" and "delta" property constants defined in form.inc?

adrian’s picture

adrian commented
StatusFileSize
new form_3.inc27.33 KB

Well spotted.

Added those defines.

nicopepe’s picture

nicopepe commented
StatusFileSize
new modif_Form_Inc.php2.52 KB

Hello
I am a newsbe and i am vey concern about the form project. I want first to congratulaed for all this nice work. I propose this code in the attached file. Tell me what you think, and i'll finished it.

nicopepe’s picture

nicopepe commented

Hello again,
I see no reaction to my proposition. Maybe not interresting code ? My aim with that code is :

- Open all the possibilities of attribute in an input balise.
- Not multiply the number of constant (The key of an element is an attribute name, define by the programmer)
- Check within drupal_attributes function if these attributes are correct in an INPUT Balise.
- Include a javascript code insertion for form client validation.

In fact i would like to create later on, a module with generic javascript for client validation (Date checking, required fields, ...). I have this in mind when i proposed this code.

Any answer this time ?

ezheidtmann’s picture

ezheidtmann commented

nicopepe, it is difficult to tell what your changes are because you have not submitted a patch. Take a look at http://drupal.org/patch. Your code might get a better response that way.

dries’s picture

dries commented

As an aside, it might be worth looking into the XForms. They have a short introduction for HTML authors in case you don't feel like reading the entire specification. A friend of mine is a member of the XForms working group and told me someone was working on a JavaScript implementation of XForms.

More importantly, XForms have (client-side) validation build-in. For example, to specify a valid range you use:

<range ref="volume" start="1" end="10" step="0.5"/>

Similarly, XForms have the ability to state that a value must be supplied before the form is submitted, have a notion of typed values (eg. you can specify that you expect an URL, integer or date to be entered). XForms also include stuff to introduce wizard-like behavior of filling in a form in stages, include functionality to dynamically add fields (eg. like when you run out of textfields for poll questions), etc. A lot of the stuff we have been talking about. :-)

It might be useful to check if our developer API would map gracefully on XForms. It would impose that we are XForms-ready (future proof). It's especially interesting because Mozilla is working on a XForms implementation. (XForms 1.0 support will be included in Firefox 1.1. You can test it using the nightly Firefox builds.)

nicopepe’s picture

nicopepe commented

As a newsbe, i have not install any patch module Yet. So i will do it.
For Dries, i will have a look on this Xforms projet, but i just wonder if it is MS compatible (80% of market) ?

Maybe before adding this code I should have write my ideas. I think this new form API should consider having an easy way to perform database update as well as new records adding. So in my view, 2 specials “events” are needed :

- call an user defined procedure BEFORE the Form is Loaded (non existing yet ?) to read database and set value to the form.
- call an user defined procedure AFTER the Form is validate (drupal_execute_form)to write to the database update or new record.

I agree with Adrian to use arrays. They allowed flexibility. I think using $form and $element arrays with others keys than theses defined with property constants is a good way. Property constants are used for regular Drupal core functionality, and user defined keys values used for extra functionalities. This means that the $form array should have a well documented structure to be able to extend it.

I see here three examples:

- Use all JS events (as my code suggest) for client validation. examples :
$form['onSubmit'] = "js function name or code...";
$element['onkeydown'] = "js function name or code...";

- Create an $element[‘NonValidationMessage’] Key used in drupal_validate_form user function to return to the user when the validation of an element is not OK (This message could also be used in JS in client validation).

- Possibility for a programmer to input in the $element array the name of the database field and database table to store the input value in case of correct validation. Exemple :
$form['tablename'] = "mytable";
$form['idValue'] = "value";
$form['idName'] = "myid";
$element['Databasefieldname'] = "myfieldname";
Then create an function to create automatic SELECT STATMENT to read the value before display the form ( When we need to modify value stored in a database) or INPUT SQL statement after validation,

I hope i am clear with my ideas. Maybe some of them could be used already in the core.

dries’s picture

dries commented

nicopepe: I'm not saying we should implement XForms at this point. I meant to say that (i) XForms looks like an emerging standard and that (ii) the XForms API tries to solve the same problems we are trying to solve with the new form API. It is worth investigating XForms. Being able to generate XForms-compliant code with minimal changes is a nice extra, but certainly not a must at this point. Hope that clarifies my comment.

junyor’s picture

junyor commented

Dries: It'd probably be more interesting to look at Web Forms 2: http://www.whatwg.org/specs/web-forms/current-work/. It's geared toward the Web, whereas XForms isn't. Web Forms 2 is also more likely to see native implementations.

killes@www.drop.org’s picture

killes@www.drop.org commented

Dries: In my book we are trying to solve a securoty issue woth this patch: the case of inserting bogus fields into our html forms. Client side validation is noce from a usability pov but we are trying to do server side.

dries’s picture

dries commented

Gerhard: it's obvious that client-side validation is no replacement for server-side validation. :-)

Anyway, instead of looking years ahead, let's focus on the patch at hand.

nicopepe’s picture

nicopepe commented

Client side validation will never replace server side validation for security reason !

I do understand that client side is not yet a priority, but please let doors open for module developper if they want to implement Client side validation in their module.

adrian’s picture

adrian commented
StatusFileSize
new forms.tar.bz20 bytes

Here is an updated, and more complete version of this patch.

I had a bit of a SNAFU with the multiples code, and needed to refactor it to work properly.

Changes in this version :
1) Introduced a 'process' property. This is used by the radios and checkboxes elements, to call 'expand_radios' and 'expand_checkboxes' functions which turn the single element into a set of elements.
2) admin/themes screen done.
3) deprecated fix_checkboxes, since it was no longer needed. Checkboxes and radios work perfectly now ( they were broken previously).
4) The way to access the form value is through a global $form_values array, that is set up to the structure of the form. It's no longer flattened, and it works perfectly.
5) Introduced an array_walk_recursive function (copied from PHPCompat in PEAR) that allows you to step through the values and do something with each key. (admin/settings uses this property).
6) Moved the old form api into a legacy.inc
7) Did a bunch more forms.

In this tarball you will find the 2 new files (legacy.inc and form.inc) and the current patch.

adrian’s picture

adrian commented
StatusFileSize
new Forms.rtf14.02 KB

Here's the status document of all the forms (rtf export from omnioutliner)

It's incomplete, as I haven't finished cataloging all the forms yet.

m3avrck’s picture

m3avrck commented

No patch in that zip :)

m3avrck’s picture

m3avrck commented

Also, adrian, is there a way to add events or set variables in javascript, if the submit button on a form has been clicked? Need this functionality for a patch about to (hopefully!) make its way into HEAD soon.

adrian’s picture

adrian commented
StatusFileSize
new forms.patch.txt108.18 KB

I'm sure it's possible, but I think we prefer attaching events and the like onto forms using the id attribute.

here's the patch file.

adrian’s picture

adrian commented
StatusFileSize
new Forms_0.rtf22.47 KB

Here's a complete list of the forms in drupal that still need changing.
Everything that has already been done is not on this list (ie: all the node modules, and the like)

Some of the forms mix normal output, and form output in the same page, and switch out depending on certain requirments (the aggregator_list_page was an example of this), making it a fairly intensive process to seperate the two pieces of functionality into clean code.

If something is rated simple, it means it only involves changing the functions to arrays, and either returning the array, or returning the final form using drupal_get_form('form_id', $form);

If we want any chance of getting this into core before 4.7, I am going to need some volunteers to handle tackling these forms for me. Once again, I am looking for a few good men (where men == female OR men == male .. obviously. =) ).

I am going to be concentrating on the really complex forms, and the theme('confirm') form, which is required by a fair amount of things.

jeremy’s picture

jeremy commented

I thought I'd give this patch a quick try, but ran into a couple problems. First is very minor, it makes some changes to settings.php that it shouldn't -- something to clean up before the final version.

Second, it adds includes for form.inc and legacy.inc, but the files themselves are not actually included in the patch.

adrian’s picture

adrian commented

They are in the tarball i uploaded earlier.

jeremy’s picture

jeremy commented

I tried that last night too, but the earlier tarball was 0 bytes and contained no files:
Attachment: forms.tar.bz2 (0 bytes)

adrian’s picture

adrian commented
StatusFileSize
new forms.tar_0.bz227.82 KB

That is the weirdest thing ..

locally :
Benton:~/dev/forms adrian$ ls -l forms.tar.bz2
-rw-r--r-- 1 adrian adrian 28489 Sep 12 15:30 forms.tar.bz2

adrian’s picture

adrian commented
StatusFileSize
new Forms.html10.02 KB

Html version of the status list.

dries’s picture

dries commented

Guys, please take a look at the status page. If we want the new form API in Drupal 4.7, we'll have to work together pretty much non-stop to complete it! Care to help? Pick an item, migrate to the new form API and upload a patch.

Bèr Kessels’s picture

Bèr Kessels commented

Can we not exclude this from the code freeze? I mean that we now (last days) focus on all these outstanding patches for 4.7. and then have acollaborative effort on this forms patch. That will at least give us the benefit that we can aim on a stable target. Instead of junting on a still changing core. From what I see a lot of people are focussing very hard at their babies in the queue, to try to push them into 4.7 (that counts for me anyway) So they will not spend any time on the form api yet.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

For what it's worth, I agree with Bèr. I've been picking away at comment.module to make it compatible with the forms API, but it's very difficult since the patch needed to enact all these changes differs from day to day (today I applied the .tar'ed version and still am getting errors with a few hunks which I need to troubleshoot first in order to make sure my changes aren't causing errors).

Once things are more "settled" with the rest of core, it seems like it would be much easier to make a more "stable" patch as a starting point, which would thus ease efforts on the part of the rest of us to get the changes made. This patch represents a HUGE change, and I worry about the idea of trying cramming it in at the last minute.

Souvent22’s picture

Souvent22 commented

I totally agree with the above comments. With so many chagnes happening so fast, we wake up to a new core each morning, and must make changes accordingly. I don't know what the core will get 'stable' enough before the freeze to really hack through what needs to be done in an efficient manner. We all seem to agree this needs to happen, but perhaps a better plan of 'how' it should happen since it is such a big change would make things easier, more stable, and not make it seem like we're trying to cram a square block through a round hole.

m3avrck’s picture

m3avrck commented

I agree with Ber and Souvent. I think we should all focus on our pet patches for the Drupal 4.7 freeze (which I think should be Friday give us that one day ;). After that, we should have a "sub-freeze" where *only* FORM related API patches go in, set this for 3-4 days after the 4.7 freeze. Then, once that is complete, *then* get back into the regular bug/performance/usability freeze over all.

dries’s picture

dries commented

Let's do it the other way around. If you help with the form API, you get a 3-day extension for your other patches. The form API is far more important than so it needs to hit core first. I already granted a 2 week extension, and unless this is near completion, I won't extend the current development cycle. Sorry.

chx’s picture

chx commented

Dries, we have a deal. Contacted Adrian, will start working on filter module 90 minutes from now.

dries’s picture

dries commented

I volunteer to tackle the statistics.module as a starter.

matt westgate’s picture

matt westgate commented

Put a mark next to my name for path module.

adrian’s picture

adrian commented

I am tackling the system module, and (god forbid) node_admin_node (which is the single most complex form i have come accross).

I am desperately looking for someone who uses localization to help port locale.module and locale.inc

Bèr Kessels’s picture

Bèr Kessels commented
StatusFileSize
new forms_system.module.patch29.24 KB

quick rerolled system module patch.

which rasies the question: how to we work on this? send in patches per file, i assume?

Bèr Kessels’s picture

Bèr Kessels commented

my hand goes up for blog.module. to get warmed up.

Tobias Maier’s picture

Tobias Maier commented

i want to start with watchdog as my first.

Bèr Kessels’s picture

Bèr Kessels commented

hmm; make that block.module. my mistake. I am on bloCK.module now.

m3avrck’s picture

m3avrck commented

hold on the filter.module ... really nice patch about to make it's way in to redo that area: http://drupal.org/node/27364 ... talked to Dries, he said he'll commit this filter patch before the FORMs API rewrite team swoops in so we don't waste our times doubling coding effort :)

adrian’s picture

adrian commented
StatusFileSize
new forms.patch_0.txt105.93 KB

quick update of the patch.
Also, the filter module is not being worked on right now, as we are going to be redoing most of it before we port it.

Tobias Maier’s picture

Tobias Maier commented
StatusFileSize
new issue29465_watchdog.module.patch1.8 KB

hello,

I know I'm doing the simplest form of all - the watchdog.module
but i'm running in some errors.

I hope you can help me

here are the important code snipets which are bugging me:

function watchdog_overview() {

[...]

  if (empty($_SESSION['watchdog_overview_filter'])) {
    $_SESSION['watchdog_overview_filter'] = 'all';
  }

  $form['filter'] = array(
    type => 'select',
    title => t('Filter by message type'),
    options => $names,
    default_value => $_SESSION['watchdog_overview_filter']
  );
  $form['submit'] = array(type => 'submit', value =>t('Filter'));

  $header = array(

[...]

  return drupal_get_form('watchdog_form_overview', $form) . theme('table', $header, $rows);
}

[...]

function theme_watchdog_form_overview($form) {
  return '<div class="container-inline">'. form_render($form) .'</div>';
}

function watchdog_form_overview_execute($form_id, $form) {
  global $form_values;

  if ($_POST['op'] == t('Filter') && isset($form_values['filter'])) {
    $_SESSION['watchdog_overview_filter'] = $form_values['filter'];
  }
}

the value of $_SESSION['watchdog_overview_filter'] and $form_values['filter'] remain 'all' the whole time...
why?
I tried a lot.

I found out, that "..._execute" will run when drupal_get_form is called.
So all places where $_SESSION['watchdog_overview_filter'] were called before wont get the current value...

Interesting is <select name="edit[edit[filter]]" id="edit-filter"><option value="all" selected="selected">all messages</option><opt..., too
is this edit[edit[filter]] really wanted?

I dont know how to solve this...
please give me some hints.
Thanks

the attached patch is not working at the moment!!

Bèr Kessels’s picture

Bèr Kessels commented

I give up on block.module. I have been up and coding for nearly 48 hours now, it is just too much. I am even thinking in arrays ATM :) array('bed' => array('top'=>'sheets')) and so on :)

So , release the 'lock' on block.module, hereby.

Tobias Maier’s picture

Tobias Maier commented

I have to give up for today too.
Maybe I have time again tomorrow afternoon

cya

adrian’s picture

adrian commented
StatusFileSize
new forms.patch_1.txt112.89 KB

Tobias : Thank you kindly, I have integrated your code, and you found a bug in the select element, and a 1 line fix later .. it was working.

It's been integrated. Thanks.

Attached is the latest patch , which also includes the new version of the theme('confirm') dialog, which has been rewritten and called confirm_form()

An example for porting any of the confirm screens :

function _forum_confirm_delete($tid) {
  $term = taxonomy_get_term($tid);

  $extra = form_hidden('tid', $tid);
  $output = theme('confirm',
                  t('Are you sure you want to delete the forum %name?', array('%name' => theme('placeholder', $term->name))),
                  'admin/forums',
                  t('Deleting a forum or container will delete all sub-forums as well. This action cannot be undone.'),
                  t('Delete'),
                  t('Cancel'),
                  $extra);
  return $output;
}

Turns into : 

function _forum_confirm_delete($tid) {
  $term = taxonomy_get_term($tid);

  $form['tid'] = array(type => 'hidden', value => $tid);

  return confirm_form('forum_confirm_delete', $form, 
                  t('Are you sure you want to delete the forum %name?', array('%name' => theme('placeholder', $term->name))),
                  'admin/forums', t('Deleting a forum or container will delete all sub-forums as well. This action cannot be undone.'), 
                  t('Delete'), t('Cancel'));
}

I changed the parameter order to be inline with the rest of the form system, ie: a form_id, and then a $form array (which has the same use
as the $extra parameter had before).

To have something happen, just create a $form_id_execute($form_id, $form) function, provided it passes the validation.

The function has also been moved to system.module (since it has nothing to do with themes anymore).

adrian’s picture

adrian commented
StatusFileSize
new Forms_1.rtf22.05 KB

Attached is an updated document with what the progress is.

chx’s picture

chx commented

form.inc , theme_select has an extra edit[ ...]

chx’s picture

chx commented

story (and maybe others) has $form['taxonomy'] = taxonomy_node_form('blog', $node);

chx’s picture

chx commented

Last update from me this night. function system_elements() has two checkbox definition and I like the first one better.

chx’s picture

chx commented
StatusFileSize
new forum_forms.patch11.06 KB
adrian’s picture

adrian commented

The taxonomy_node_form has been changed to return a $form array.

Here's an updated patch, including the forum module patch from Chx .. thanks man.

Dries has started working on the user module.

adrian’s picture

adrian commented
StatusFileSize
new forms.patch_2.txt122.36 KB

and the file *blush*

Tobias Maier’s picture

Tobias Maier commented

I applied the last patch on my drupal HEAD.
The watchdog.module does not work as expected :(

the html-output is <select name="edit[edit[filter]]" id="edit-filter"><option value="all" selected="selected">all m... again...

Tobias Maier’s picture

Tobias Maier commented

another error:

for example on /node/add/page

<div class="form-item">
 <label for="edit-edit[author-name]">Authored by:</label><br />
 <input type="text" maxlength="60" class="form-text form-autocomplete" name="edit[author][name]" id="edit-author-name"  size="60" value="Tobias Maier"  />
</div>

the label for-value has to be the same as the input id-value (see http://www.w3.org/TR/REC-html40/interact/forms.html#h-17.9.1)
this error is on every form...

dries’s picture

dries commented
StatusFileSize
new user-access-rules.patch7.07 KB

Converted the access rules checks form. Patch for user.module attached.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
StatusFileSize
new book.module_9.patch6.42 KB

Here should be book.module for the most part, with the exception of the admin link function which I've tagged as TODO after talking with adrian. I swapped my comment.module assignment with eaton and took this one instead.

dries’s picture

dries commented
StatusFileSize
new user-access_0.patch118.99 KB

Updated two more forms in user.module. Discard #101 and use this one instead. Could you double-check the use of default_value in _user_admin_access_form(). Would that be correct?

chx’s picture

chx commented
StatusFileSize
new menu_forms.patch6.23 KB

Here are the menu forms. I even tested it somewhat :)

chx’s picture

chx commented

Maybe others also have some hard time to figure out how to put form elements into a table.

You construct the form, with non-interactive elements ie. plain text going in as array(type => 'markup', value => ...). Then you write the appropriate theme function and call form_render in a loop. If you use foreach ($form as $key => $element) { for this loop be aware that you need to pass $form[$key][..] to form_render so that it can set that form element to rendered. Or you can unset($form[$key]); at the end of the row. All this is necessary because the usual ending is:
<?
$output = theme('table', $header, $rows);
$output .= form_render($form);
return $output;
?>

Where this form_render call adds buttons and everything else not yet rendered.

chx’s picture

chx commented

Eek! Let's try again:

because the usual ending is:

  $output = theme('table', $header, $rows);
  $output .= form_render($form);
  return $output;

Where this form_render call adds buttons and everything else not yet rendered.

chx’s picture

chx commented

Look into system.module and (soon) filter.module for examples on how to do this table render dance.

eaton’s picture

eaton commented
StatusFileSize
new comment.module_28.patch59.92 KB

Not as good as the 'finished product' but here's the very-close-to-finished patched version of comment.module.*

Currently broken:

  • Comment settings display, but changes don't seem to be saving.
  • The comment filter widgets ('show 50 comments in threaded view...') display and submit but don't seem to be ding the actual filtering.
  • For some reason, the $form = array_merge($form, filter_form($node->format)); line that merges in filter formats works when I'm an admin, but prevents the comment submission page from rendering at all when I'm logged in as a user, or not logged in. It's commented out atm as I figure this out.

I'm looking into these issues; if anyone wants to take a stab at them, it'd certainly be cool.

Currently working:

  • Viewing, posting, replying to, editing, and deleting comments as admin, user, and anonymous.

*As I discussed on #drupal, the formapi work I'm doing is based on the already-completed comment module patch at http://drupal.org/node/28255 ... it removes moderation for implementation in a separate module, and adds a set of nodeapi style hooks for comment enhancements (file uploading, captchas, etc).

eaton’s picture

eaton commented

Something I just realized a few minutes ago that tripped me up...

Forms with nested groups of elements have to be navigated as keyed arrays: $form['admin']['options'] for example. When a newly loaded object is prep'd for use using object2array($object), it flattens the properties to $form['options'] and so on. if you're not expecting it, it can cause all sorts of wackiness.

It'd be great if the API could account for this *somehow* -- as of right now it means that preview/validate code may have to do double-duty whenever an object is being edited, rather than created.

(then again, I could just be tired...)

hunmonk’s picture

hunmonk commented
StatusFileSize
new forms_4.patch118.54 KB

dries' latest patch had a failed hunk in modules/blog.module--i'm assuming that was because of a recently applied patch/adjustment. attached is a new patch that corrects this.

hunmonk’s picture

hunmonk commented

oops. ignore that patch i posted in #110--i wasn't clear on how we were tackling this.

eaton’s picture

eaton commented
StatusFileSize
new form_mega.patch196.88 KB

Ultra mega super patch. this one takes adrian's most recent patch and rolls in all the changes posted SINCE then. (including chx' menu.module, webchick's book.module, thehunmonkgroup's blog.module fix, and my comment.module changes)

eaton’s picture

eaton commented

er... AND dries' patch, too.

adrian’s picture

adrian commented

Here's my updated forms.inc file.

apparently i didn't post the select box fix.

adrian’s picture

adrian commented
StatusFileSize
new form.inc.txt28.7 KB

no wait.

HERE's my form.inc

=)

eaton’s picture

eaton commented
StatusFileSize
new comment.module_29.patch58.74 KB

Here's the latest copy of comments.module. the only thing not working at present is the stuff managed by the form_settings hook. Other than that, things seem to be flying.

adrian’s picture

adrian commented
StatusFileSize
new Forms.ooutline66.43 KB

latest status document.

If you see anything here incorrect, please get a hold of me on irc.

adrian’s picture

adrian commented
StatusFileSize
new Forms_2.rtf22.29 KB

sigh.

adrian’s picture

adrian commented

ok.
this sending of patches is silly.

i'm going to set up svn accounts on dev.bryght.com for anyone interested in helping. Contact me on irc if you want to help.

adrian’s picture

adrian commented

Wiki Page

http://dev.bryght.com/t/wiki/DrupalFormsApi

It has a publicly editable status document.
If you work on something, please mark and keep track of what's done.

If there's any documentation/tips you want to share, please put it there too..

The current source is on http://dev.bryght.com/t/browser/forms/

junyor’s picture

junyor commented
StatusFileSize
new drupal-head.update-php-form.junyor.patch1.09 KB

Adrian's still asleep, so here's a patch for update.php.

junyor’s picture

junyor commented
StatusFileSize
new drupal-head.throttle-form.junyor.patch4.61 KB

Here's throttle.module.

gordon’s picture

gordon commented

Due to the dropping of the hook_textarea() I have been looking at other ways that a WYSIWYG Editor can be hooked into drupal, but also maybe even a better method.

The idea that I have come up with is creating a new hook that is called from form_render() called hook_form_render(&$form) which will allow contributed modules to change the $form array before the html is generated.

This doesn't allow me to add any html to the form, but for adding a WYSIWYG editor like Xinha I do not have to.

What I can do which will be very clean is add to the selected textareas is add to the attributes something like 'editor' => 'xinha' and the do a drupal_add_js() to load Xinha, which will search all the textareas in the page which the attribute editor with a value of xinha. The means that I will be adding no js to the rendered page, just 1 include and an attribute.

I started looking at this but the problem is that form_render is a recusive command. I have an idea that I can get around it by changing the function to

function form_render(&$form, $root = true) {
}

and then doing the recusive call with false.

I beleive this would give extremely amount of control over forms from contributed modules.

hunmonk’s picture

hunmonk commented

we submitted a little quick reference guide for the new form API here. Just a start, but quite handy!

gordon’s picture

gordon commented
StatusFileSize
new forms_5.patch435 bytes

I have been working on my idea to create a new hook called form_render() which is called during drupal_get_form().

This is such a powerful hook, as it means that the entire form can be edited before it is displayed. You can change fields, add fields, delete fields without any problems. This makes the nodeapi form pre and form post more of less obsolete. You can do more here and it is available to all forms.

How does this replace hook_textarea()? In most cases people do not return any additional html, they just remember the textarea and then do some processing at the bottom, such as the case with htmlarea. With the new js stuff and the better method of implementing js is there is no js on the actual page. The elements are flagged and then the included js then searches for the relevant elements and then does what they need to.

So in the case of htmlarea I would add editor="xinha" to each required textarea and then search for that to display the page. Much much more elegant than the existing method.

Other use cases would be the event module. The date is quite ugly, I prefer to use The coolest DHTML/js calendar. S what I can do is create my own date time field and then in the hook_form_render() I can just rewrite the original date field created by the event module with my own one.

We need something like this for the WYSIWYG editors in contribution so they can attach to the textareas, and this will create a very clean method of doing this.

gordon’s picture

gordon commented
StatusFileSize
new form_render.module492 bytes

Also here is a small example module that adds "blah" to all the titles in the form. Just an example of ehat can be done.

adrian’s picture

adrian commented

Regarding the form_render hook, I am going to -1 that.

i would prefer to do the hook in a different way.

ie: pass the entire form array to a hook, and allow you to alter it in any way you want.

This would allow you to step through all the elements, if you so wish, but would also allow you to alter the form using the form id.

ie: in form_id = event_node_form override the weight of the title field to 100 to let it display at the bottom of the form.

And I also think there has to be a better way to do the htmlarea functionality.

adrian’s picture

adrian commented

hah!
seems like we had the same idea after all.

Perhaps we should call the hook form_alter or something, since you aren't actually affecting the html output, but the form data structure.
The other thing i want to do is enable you to specify 'interface levels' for each of the elements, meaning you can set the taxonomy to not display for certain groups of users, or possibly default the taxonomy selection/title based on certain requirements.

I am against changing form_render to have the root parameter though, as I don't believe it's neccesary.

gordon’s picture

gordon commented
StatusFileSize
new forms_6.patch406 bytes

After I implemented the change into the form_render function I found that it was being call multiple times. I did not want this. I only wanted the the hook to be called once.

I then moved the hook into the drupal_get_form() where all modules should call the formsapi. I think it would be rare and this should not matter too much.

I have renamed the hook form_alter as per your request. I am not linked to the name, I more want the functionality.

Because the hook is not being called from the form_render() there is no need for the $root to protect the recusion.

adrian’s picture

adrian commented

I've added the form_alter hook to my form.inc

will commit it soon.

hunmonk’s picture

hunmonk commented

ok, i've done an exhaustive search of all modules and .inc files, looking for all forms that still need to be converted. found some that weren't in adrian's initial list. we're in decent shape, but there's still more work to do! you can see the updated list here, and pick out the task of your choice... :)

m3avrck’s picture

m3avrck commented

What's the status? It seems like most forms are almost all done? Is this still going in? Really need this for a new module working on let me know what I can do! Thanks!

hunmonk’s picture

hunmonk commented

the forms upgrade is nearly complete. all forms have been converted, and at least 90% of the basic testing/debugging is done.

plan of action from here on out:

  • finish up the remaining basic debugging. chx and adrian should be able to handle what little is left
  • generate the patch and merge it w/ current HEAD. this will take some work probably, since we've been working out of a seperate svn repo to coordinate our efforts, and there have most likely been changes to HEAD that will affect our mega patch
  • commit
  • get people to test our work and find any remaining bugs, etc...
adrian’s picture

adrian commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new forms_7.patch411.59 KB

Ladies and gentlemen.

I present to you, the single largest patch ever contributed to Drupal.
In fact, it's a partial rewrite of the entire input layer of Drupal, with a final patch file that is a shocking 411K in size.

This project has been a testiment to what we can achieve as a community, and has also taught us a lot about coordination and
managing a project of this magnitude. I think we've proven to ourself that no task is too big, if we work together.

I would like to thank Chx, Chad (thehunmonkgroup), Dries, Killes, Webchick, Asimmonds, Junyor, Eaton and Gordon for their contributions,
without which this patch would not have been possible. Really guys, you should be proud of yourselves =)

The next big task is documenting it all. But, we do now have a large group of people who are comfortable with the
new API, so it shouldn't be too painful a task.

adrian’s picture

adrian commented
StatusFileSize
new form_4.inc33.22 KB

Here is the the form.inc required for the patch. Place in includes/

adrian’s picture

adrian commented
StatusFileSize
new legacy.inc23.11 KB

Included is the legacy.inc file containing the current form functions.

I would also like to apologize to Tobias for neglecting to thank him in my previous post. It was purely due to my lack of proofreading =).

adrian’s picture

adrian commented
StatusFileSize
new forms.tar_1.bz272.16 KB

And here's it all in a tarball.

m3avrck’s picture

m3avrck commented

I just want to say you guys have done an awesome job with this patch, and for Adrian for coordinating this massive patch :-)

I wish I had more free time earlier to help convert forms but alas, I was swamped with work :-/ But don't worry, I do plan to help test this extensively as I'm working on 2 drupal HEAD sites currently so you can be sure I'll be submitting a many bug reports and hopefully patches too, if I can figure out what's up.

Again, awesome work guys. Dries, let's get this committed so we can start testing this ASAP!!! Cheers!

Steven’s picture

Steven commented
Status: Reviewed & tested by the community » Needs work

I was going to commit this after a bunch of requests on #drupal, but I don't understand why it was marked ready to be committed as there are severe bugs that break Drupal's most basic features.

- Node previewing doesn't seem to work? Fields disappear and errors appear.

- Logging in from the sidebar block is impossible? That form also has two <form> tags, one inside the other.

- upload.module was horribly broken even without Javascript. The JS-part was completely ignored in the conversion: wrappers were not wrapping anything, marker input-hidden's were not marking anything anymore.

- The implementation of prefix/suffix is odd to say the least. The documentation says it is only used for 'markup' type form elements, yet it is used in a couple of places on other elements too (textfield, hidden, fieldset). But textfield never outputs the prefix or suffix. It seems logical for every theme_element() to start with the prefix and end with the suffix. Implementing it in only some cases (as is done now) is odd. Why not move them into form_render() itself? Keeping them in theme_element() is wasteful, and doesn't give you any benefit in themability as they are solid chunks of HTML anyway (which are required for styling or ajax in several cases).

I started doing code clean up and fixing JS upload, but the buggyness prevents me from getting anything done.

chx’s picture

chx commented
StatusFileSize
new form_5.inc33.27 KB

I apologize -- the version I had was really working. There were some last-time changes and those broke this and that. I can't svn up currently and have no clue how could I proceed. I defnitely do not want to reroll a 400K patch. A form.inc is attached however which fixes a lot of problems.

In user.module, the following fixes an infite loop if the wrong password is entered:


  // Display error message (if any):
  if ($error) {
    $form['error'] = array(type => value, 'value' => 1);
    drupal_set_message($error, 'error');
  }
....
function user_login_execute($form) {
  global $form_values;
  if (!isset($form_values['error'])) {
    return user_login($form_values);
  }
}
chx’s picture

chx commented

ah yes -- node preview and submit is broken badly. I _know_ they were working for me but not now and I do not have any more time to spend on this -- I spent already too much *sigh*.

adrian’s picture

adrian commented

Ok. i've almost fixed node_preview.

I moved it so that the preview is an element added to the form, meaning you can move it around in the display (ie: put it at the bottom if that's what suits you).

I'll be testing the fixed form_upload a bit , and i've already done the prefix / suffix move to form_render. Thanks for that.

adrian’s picture

adrian commented
StatusFileSize
new forms.tar_2.bz272.88 KB

New version of the patch.

Fixed the node preview thing and the file upload.

adrian’s picture

adrian commented
Status: Needs work » Reviewed & tested by the community

status ...

dries’s picture

dries commented

Applying the patch results in a parse error: the problem lies within the contact.module.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Active

Patch committed, setting to active! =)

asimmonds’s picture

asimmonds commented
Status: Active » Needs review
StatusFileSize
new contact-forms-api.patch19.52 KB

Attached is a patch to update contact.module

dries’s picture

dries commented

Committed.

Let's mark this fixed when the documentation is in place.

Tobias Maier’s picture

Tobias Maier commented
StatusFileSize
new watchdog.module.patch829 bytes

and here comes a patch which fixes watchdog

Tobias Maier’s picture

Tobias Maier commented

what I fixed:
if you filtered for a message type you had to reload the page again to see any effect

Tobias Maier’s picture

Tobias Maier commented

another error: the Theme configuration on the user/edit page is broken for me
errors on /admin/themes and /admin/block after changing the theme all the blocks where hidden
SQL error messages on /admin/block
"chameleon settings" is available on /admin/block but this theme is disabled

this errors are here for me since the last update

Tobias Maier’s picture

Tobias Maier commented

sorry the only thing broken through the patch is the look of the Theme configuration on /user/1/edit

it seems like that if you select a theme for the first time from the /user/*/edit page all the blocks are not available.
-->also there is way missing to set it back to the default value

Tobias Maier’s picture

Tobias Maier commented

I tried to fix the "Theme configuration"-problem but it was too advanced for me.

could please the same person who did the system_themes() look at system_user()

I found out that it is possible to assign a theme just to a part of a $form-array.
you could maybe use
$form['themes'][theme] = 'system_user_themes';
and create the function theme_system_user_themes()
I tried it since my last post but had problems to understand what happens at systme_themes() (its very similar to the other function)

hunmonk’s picture

hunmonk commented

just a heads up that i've been commissioned by Kieran to write doc for the forms api. it's a top priority for me, and my goal is to have a good first effort submitted by sunday evening. i'll be working w/ Adrian, the other folks who contributed heavily to the upgrade, and the docs team to accomplish this task.

kbahey’s picture

kbahey commented

Any changes done in admin/settings are not saved after the patch hit HEAD.

Details here http://drupal.org/node/33347

hunmonk’s picture

hunmonk commented

sepeck has been kind enough to create a nice workspace for the evolving forms API documentation:

http://scratch.blkmtn.org/

there you can read the bleeding edge of forms API doc, which is currently an outline roadmap for full documentation, and a quickstart guide.

if you find anything that needs to be changed, please register at that site and create a new book page detailing the change, or add a new comment to an already existing book page, and i'll work it into the doc. i prefer this method instead of people bombing me with emails and pm's... :)

ax’s picture

ax commented

did anyone actually performance-test this big change?

killes@www.drop.org’s picture

killes@www.drop.org commented

ax; not to my knowledge. But it hardly matters as forms are not found on every page.

ezheidtmann’s picture

ezheidtmann commented

killes, with the login block enabled, a form is on every page (at least for anonymous users).

deekayen’s picture

deekayen commented

4) Started the first part of the implementation of the multiple keyword, which is when an element can have multiple values. An example of this would be the 'files' element from upload.module , where more files get added, and also the primary / secondary links configuration of phptemplate.

I'm guessing this is the right issue to say the primary/secondary links don't update when I submit changes on /admin/themes/settings.

m3avrck’s picture

m3avrck commented

Also I think I found another bug as well. I have two forms on the same page both of which should be submitted seperately. One form has required fields but if I submit the other form, the first one always popups saying "required field" missing. Any ideas?

chx’s picture

chx commented
Status: Needs review » Closed (fixed)

This thread is way too long. Please do not post to it any more but use new issues.