Attached are patches on HEAD and Drupal 6 for this watchdog error generated by RSS feeds with file attachments: "Undefined index: value in includes/common.inc on line 962."
This could also be fixed by adding some isset() logic in common.inc -- it depends what is desired here, strictness or flexibility of the Drupal API.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | xml-notice.patch | 742 bytes | mfb |
| #4 | xml-notice.patch | 741 bytes | mfb |
| #2 | upload-notice.patch | 783 bytes | mfb |
| d6-upload-notice.patch | 730 bytes | mfb | |
| d7-upload-notice.patch | 666 bytes | mfb |
Comments
Comment #1
mfbComment #2
mfbThis alternate patch seems better now that I look at it, as format_xml_elements() already has similar isset() logic for attributes.
Comment #3
dries commentedI've committed this to CVS HEAD.
I tried committing it to DRUPAL-6 but it failed to apply. I'm changing the version to 6.x and marking it 'code needs work' so it can be re-rolled.
Keep up the great work, mfb!
Comment #4
mfbComment #5
stevenpatzComment #6
mfbReroll to get rid of (offset 10 lines) warning.
Comment #7
webchickConfirmed this fixes the error in both rss.xml and taxonomy/term/blah/feed.
RTBC!
Comment #8
gábor hojtsyWhy not use !empty()?
Comment #9
webchickWell, great question! :D
Marking back up to 7.x-dev to fix there and then backport.
Comment #10
mfbBecause a value of 0 (which would be "empty") should still be printed?
Comment #11
mfbActually, would it just ignore 0 because that would evaluate as ''?
Comment #12
mfbOK the difference would actually be for '0' not 0. We probably do not want to ignore a string value of '0'? At least, we haven't been so far.
Comment #13
dman commentedHow come this never made it back into DRUPAL-6?
Comment #14
gábor hojtsy@dman: because as you can see above, it was still discussed.
I get that '0' should be allowed as a value, so this is indeed better. I'm not entirely fond of this not documented in the code (ie. someone could come around and 'simplify' this code, just like I suggested), but I've committed it as-is anyway.
Comment #15
dman commentedCool then, thanks! It looked to me like the fix was good enough to be put into Drupal 7 over a year ago but just stalled waiting to be backported to 6 :-) . So it needed a nudge ;-)
It just bit me this week, is all ( I saw it last year too, and did this patch by hand on that version) .
I've got E_ALL notices on, and some semantic browser plugin (I suspect) was pinging my rss every pageview (I have no idea what's up with that) and showing the notices all the time. Normally I'm guessing that RSS-triggered notices fade away into /dev/null, which is why nobody has noticed the bug was still there..
Triggered by upload.module and image_attach.module both.