I am using views_data_export to create a CSV download with the Exposed Filters form in a block. One of the exposed filters is a Select widget (for content type). I get an error message at the top of my file when I download it.
I think the problem is that the value of the Select widget is an array, and the code expects a string (as for a text box/area). It passes that value through check_plain(), which hands it off to htmlspecialchars(), generating the error.
I will supply a patch as soon as I have an issue number.
Update (2014-06-20):
The original patch (#1) was committed to the 7.x-3.x branch (#2) and backported to the 6.x-2.x branch (#4).
The original patch assumes that the option is a string or an array of strings. Later comments point out that it can get worse than that. The patch in #12 deals with more cases and was marked RTBC (#13, #14, #16). The patch in #15 aims to deal with any level of arrays, but (according to #16) has trouble with simple strings.
Comment | File | Size | Author |
---|---|---|---|
#15 | views_data_export-exposed_filter_token_bug-1888614-15.patch | 1.16 KB | michfuer |
#12 | vde-exposed-filter-1888614-12.patch | 1.49 KB | JvE |
#7 | vde-exposed-filter-1888614-7.patch | 1.48 KB | benjifisher |
#1 | vde-exposed-filter-1888614-1.patch | 1.1 KB | benjifisher |
Comments
Comment #1
benjifisherThe attached patch fixes the problem for me. I am not sure what effect it will have on other file formats, or if the Exposed Filter form is not in a block. My guess is that the file format will not matter, since add_http_headers() adds the filter information as tokens in the 'Content-Disposition' header, which (I guess) is independent of the format.
I am also not sure whether the option can be anything other than a string or an array. At least, I think this patch is a step in the right direction.
Comment #2
Steven Jones CreditAttribution: Steven Jones commentedThanks very much for the patch, fixed in 7.x-3.x.
Comment #3
Steven Jones CreditAttribution: Steven Jones commentedNeeds backport
Comment #4
Steven Jones CreditAttribution: Steven Jones commentedPushed in 6.x-2.x
Comment #6
sinn CreditAttribution: sinn commented#1 doesn't work with the between-date-exposed-filter because option in that case is like
Code:
causes error in that case: "Array to string conversion in views_data_export_plugin_style_export->add_http_headers() (line 330 of D:\xampp54\htdocs\specsavers.localhost\brand2\code\sites\all\modules\contrib\views_data_export\plugins\views_data_export_plugin_style_export.inc)."
https://drupal.org/node/1298814#comment-6870686 works in that case.
Comment #7
benjifisher@sinn, thanks for pointing this out. I guess if I had looked harder in the issues queue, I would have found #1298814: PHP warning when exposed filters contain non-scalar data. (now closed as a duplicate of this issue) instead of opening this one.
I am not too surprised that the patch in #1 fails to cover some cases: see my comment there.
The latest patch in the other issue has some funkiness:
Even if PHP allows it, I do not like replacing an array while iterating over it, but surely this snippet does not do what we want: the effect is to set
$filter_data
to$key . '-' . $data
for the last entry of the original array, discarding the results of earlier entries. Also (and the first thing I noticed) it violates Drupal coding standards:=>
should have a single space on either side.Please test the attached patch. It could be more concise, but I think it is pretty clear. I tested it on my site, but not with a date-range filter.
Note that my patch ignores
$key
unless$data
is an array. I do not feel strongly about this: if you think the key should be included, then I can roll a new patch.Are we confident that "array of array of strings" is as bad as it can get? Maybe we should do something with
array_walk_recursive()
in order to future-proof this bit of code.Comment #8
JvE CreditAttribution: JvE commentedI am using patch #7 on one of my sites now and it does fix the notice.
I do not know if the resulting $tokens string is useable though.
Example:
One date field "field_date" and one date-range field "field_date_range".
$tokens['%exposed'] with only range filled:
field_date_value_value----field_date_range_value_min-2011-7-9--max-2012-5-16
$tokens['%exposed'] with only date filled:
field_date_value_value-2012-6-10-field_date_range_value_min-----max---
$tokens['%exposed'] with neither filled:
field_date_value_value----field_date_range_value_min-----max---
Perhaps an array_filter() is needed on $data before implosion?
Comment #9
koppie CreditAttribution: koppie commentedPatch in #7 works for me. (Patch in #1 did not.) Note that the new patch needs to be run against the dev version of the module. I'm marking this RTBC and switching the version back to 7.x-3.x.
Any word on when we can expect to see a new stable version?
Thanks!
Comment #10
koppie CreditAttribution: koppie commentedWhoops sorry - our ticket updates crossed in the blogosphere. Can I leave this RTBC or does it need more work?
Comment #11
benjifisherIf I remember correctly, the token is used to create a default file name for the download. It is configurable to some extent. It is not useful for me.
If that is right, then the problem in #8 is an annoyance, not a serious problem. Unless you need to be able to parse the file name ...
Comment #12
JvE CreditAttribution: JvE commentedThe same patch from #7 with a filter for empty values.
$tokens['%exposed'] with only range filled:
field_date_value_value--field_date_range_value_min-2011-7-9--max-2012-5-16
$tokens['%exposed'] with only date filled:
field_date_value_value-2012-6-10-field_date_range_value_min---max-
$tokens['%exposed'] with neither filled:
field_date_value_value--field_date_range_value_min---max-
Comment #13
Jelle_SPatch #12 works for me
Comment #14
NancyDruSeems to take care of me.
Comment #15
michfuer CreditAttribution: michfuer commentedI feel like recursion is appropriate here since I never can anticipate the depth of a particular field/filter used by Views. This patch doesn't do anything the others don't, just takes a recursive approach.
Comment #16
drenton CreditAttribution: drenton commentedPatch #12 works for me.
#15 does not work for string only filters.
Comment #17
benjifisherI am updating the issue summary to summarize the recent activity.
@drenton, can you give steps to reproduce the problem using the patch in #15?
Comment #18
drenton CreditAttribution: drenton commentedAny string only exposed filter should reproduce the problem.
I created a filter for :
Content: Author uid (exposed)
Here is the view export :
Comment #19
Steven Jones CreditAttribution: Steven Jones commentedSetting back to needs work, as there seems to be some disagreement about whether it works or not.
Comment #20
JvE CreditAttribution: JvE commented#12 works in all cases so far reported.
#15 adds support for multidimensional select options but removes the check to see if $options is an array or not before treating it as one.
@michfuer: when is $option a multidimensional array? Could you provide an example view?