Fix some PHP notices on mailbox edit form
mfb - August 27, 2008 - 00:49
| Project: | Mailhandler |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Description
The mailbox add/edit form has some PHP notices about undefined indexes and undefined variables. The attached patch fixes the issue.
Part of the problem is missing default values for the mailbox form. I took a shortcut and used schema API to grab some defaults, but alternatively some explicit defaults could be defined.
Btw another thing I was looking at but didn't patch is the submit function for this form could probably use drupal_write_record().
| Attachment | Size |
|---|---|
| mailhandler-admin.patch | 2.21 KB |
#1
The patch failed for me. Please re-roll.
Also, you might want to check out the 'cron' and 'security' radio buttons, which are stored as text, and so they are initialized to an empty string, while there should be something there.
Why did you choose to initialize the values through the DB, instead of setting them as part of the form?
#2
I just didn't know what the defaults should be (aside from what the schema defines). I can reroll it if/when I have more time to look at correct form defaults..
#3
Here's a reroll with explicit defaults.
#4
I'll try to put the same values as part of
$form[...]['#default_value']and will check it.#5
Hi Mark,
I tried to recreate the notices, but I don't see anything.
I'm using PHP Version 5.2.4-2ubuntu5.3 with apache2, and PHP 'error_reporting = E_ALL'
#6
You may need to be using the dev version of Drupal to see the notices? See http://cvs.drupal.org/viewvc.py/drupal/drupal/includes/common.inc?r1=1.7...
#7
I tried again, I changed common.inc as necessary, but still no errors.
hmmm... I must be missing something, but I don't see what it is.
#8
Do you have Drupal set to display errors on the page? If not they might just be getting logged.
#9
z put this into you index.php at the top:
error_reporting(E_ALL);ini_set('display_errors', TRUE);
ini_set('display_startup_errors', TRUE);
And you'll see loads of errors. Here are some more fixes, the patch includes the previous ones.
#10
eMPee584, you can't RTBC your own patch :-)... but I appreciate the effort!
It took me a while to get to this round of bug fixing, but I'm here at last.
Meanwhile I applied Mark's patch at #3, with few corrections. I would like to better inspect your proposed changes, eMPee584.