Posted by markj on February 5, 2009 at 10:48pm
5 followers
| Project: | Views Bonus Pack |
| Version: | 6.x-1.x-dev |
| Component: | Views Export |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
In XML exports, Views field labels that contain spaces create invalid XML (specifically, spaces in element names). For example, my Views field is called "Updated date" and the XML element that contains this value is exported as <Updated date>. Sample file attached (with .txt added since .xml is not attachable).
| Attachment | Size |
|---|---|
| view-my_stuff.xml_.txt | 924 bytes |
Comments
#1
I'm tentatively going to call this by design. I don't want to fudge with the values and cause unexpected changes to the values of the elements(even if it means causing invalid elements). I believe there is some responsibility on the user to set the titles correctly, even if that commonly means overriding the values.
#2
Your call, but since the XML that is exported is not well formed (it won't even display in a web browser), maybe it shouldn't be called an 'XML export'. Maybe 'marked up' or 'tagged'?
#3
Totally. We need people to stop abusing XML...
Get it right early on, and it will be future-proof. Allow quirks through and it will fail.
How about :
<node><title name="Title"><![CDATA[<a href="/subjectguides/content/testing-image-insertion">Testing image insertion</a>]]></title>
<field_updated_date name="Updated date"><![CDATA[June 24]]></field_updated_date>
<published name="Published"><![CDATA[Yes]]></published>
</node>
... actually, there's lots of room for improvement, like ....
<published name="Published" value="1" />... and is that title really supposed to have the href embedded?
#4
I think you miss understand my reason for saying this is by design. I understand that that XML is not well formed but it is only not well formed because the label you have set for the field doesn't make for well formed XML. You can change the label to something well formed simply by overriding the label for you XML output display.
Along those lines, the link could be removed by overriding the option on the field.
As far as doing more complicated things like attributes, I'd be open to investigating this in a feature request. However, I was mostly focusing on providing a basis for simple XML feeds to be built form and this might be something more fitting for Views Datasource.
#5
Sorry, I didn't know it was possible to override the labels for the XML output. How do I do that, using a custom field label in my XML feed's display configuration?
Thanks for pointing out Views Datasource, it looks useful.
#6
Just answering my own question -- yes, I was able to make all element names well formed by assigning XML-feed specific field labels. Sorry I didn't think of this before opening the issue.
#7
I get why you consider this to be by design, but having just spent a lot of time overriding the default labels of 100+ fields (often forgetting to click the 'override' button), could i persuade you to consider an option in the xml style settings like the "Transform spaces to dashes in URL" option in views arguments?
Let me know if you would consider this, and I'll try to roll patch for it.
#8
I'm always open for good ideas. Lets see how it'd work.
#9
ok, let me roll a rough patch and we'll see where it goes. thanks.
#10
Here's what I have so far, now I just have to figure out how to actually implement the string replace ;-)
#11
Ok, here's a full functioning implementation. There's a couple of ways I could have implemented the string replace, I did it the simplest way until I see how you feel about the patch.
#12
Looks pretty reasonable.
A couple of nitpicks. I'm pretty sure this options isn't translatable. Also, I'm always hesitant to roll out changes that affect the default functionality(ie defaulting no features that mangle output to true). I guess its probably ok in this case since it would only affect people outputting broken XML. Lets make sure we think it over though.
It also feels like maybe if we're going to do this, we should go all out and have a couple options. ie. '-', '_', and maybe camel. I'll leave how far you want to take the implementation in the patch up to you though.
#13
Good points. I also thought about adding options for '_' and camel but in the end took the easy way out, lol. I also thought about changing the actual application of the option to a preprocess function instead of in the template file, but I couldn't get it to work for some reason. I'll work on it some more and see how far I can get. And thanks for considering the patch!
#14
Yeah, you'll need to move that label "building" code out of the template and into the preprocess function in views_bonus_export.theme.inc creating a new "label" key in $vars
Probably have to do something like
<?phpforeach ($vars['header'] as $field => $label) {
$vars['label'][$field] = <magic process logic>;
}
?>
Then use this in the template:
<<?php print $label[$field]; ?>><?php print $content; ?></<?php print $label[$field]; ?>>All code totally out of my head and not tested, if that "magic process logic" didn't give it away.
#15
quick question-- which is more common in xml, camelCase or PascalCase?
#16
OK, a new patch with options for '-', '_', and PascalCase.
#17
and thanks for the hint about the preprocess function... that'll be in the next patch.
#18
re common usage: don't really know. I've come across both depending on who i'm developing for but I'd guess camelCase would be pretty important based on that fact RSS uses it(ie pubDate)
some nits
'translatable' => TRUE, should still be FALSE
you need to provide 'transform_type' in your options_definition function
Nice use of the dependent field :) Exactly how I'd want it.
#19
ok, this is for real-- should be ready to go. Let me know if i missed anything.
#20
meh... I missed your post. Let me make those changes.
#21
ok, for real for real this time, lol. Includes all comments so far.
#22
what the heck, I hate incomplete things, lets add camelCase:
#24
yet one last one-- somehow the original hunk for the tpl,php file keeps showing up, this one removes it (and fixes a bug with blank $labels).
#25
hmm... same problem. Trying one more time. If this doesn't work, just delete that hunk, lol.
#26
Awesome work, I'll try and give it an actual honest to god apply and look over review tonight or tomorrow. Today got spent with a spawn from this issue #70719: drupal_ucwords() implementation.. Though I don't really care to much about unicode support ATM so please don't worry about implementing that patch here :)
#27
sounds good.
#28
Looking to commit a slightly modified version. Will do it in the morning if you don't see anything glaringly wrong.
Changes:
#29
fyi there's some csv hunks in your patch but it looks great-- and I think both changes make sense.
#30
Committed without csv hunks :)
#31
awesome-- thanks!
#32
looks like the patch got mangled somehow. I just checked out the dev version, and the changes to views_bonus_plugin_style_export_xml.inc are repeated a few times in the file. I deleted the repeats, and otherwise it works great-- thanks again.
#33
weird. Removed and should be fine now. Thanks for catching that.
#34
Automatically closed -- issue fixed for 2 weeks with no activity.