Merge lp:~thumper/lazr.restful/encode-entryfieldresource-xhtml into lp:lazr.restful

Proposed by Tim Penhey
Status: Rejected
Rejected by: Tim Penhey
Proposed branch: lp:~thumper/lazr.restful/encode-entryfieldresource-xhtml
Merge into: lp:lazr.restful
Diff against target: 12 lines (+1/-1)
1 file modified
src/lazr/restful/ (+1/-1)
To merge this branch: bzr merge lp:~thumper/lazr.restful/encode-entryfieldresource-xhtml
Reviewer Review Type Date Requested Status
Leonard Richardson (community) Needs Fixing
Review via email:

Description of the change

If the XHTML representation for a single field is unicode, we should utf-8 encode it.

To post a comment you must log in.
Revision history for this message
Leonard Richardson (leonardr) wrote :

The XHTML representation for a field will not always be Unicode. It depends on the implementation of IFieldHTMLRenderer. In particular, the default renderer, render_field_to_html, returns UTF-8, and double-encoding it will cause an error. This might explain the errors you were seeing after making this change, but I'm not sure.

I think it's fine to move all the UTF-8 encoding out of the IFieldHTMLRenderer and make it the IFieldHTMLRenderer's responsibility to return Unicode. Alternatively, it would be easy to make EntryFieldResource._representation accept either and return UTF-8.

Here's my branch based on this one, which I think is better:

review: Needs Fixing

Unmerged revisions

167. By Tim Penhey

Encode the EntryFieldResource._representation if it is XHTML.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/lazr/restful/'
2--- src/lazr/restful/ 2011-01-24 17:28:14 +0000
3+++ src/lazr/restful/ 2011-01-26 05:01:40 +0000
4@@ -1280,7 +1280,7 @@
5 elif media_type == self.XHTML_TYPE:
6 name, value = self.unmarshallFieldToHTML(
7, self.context.field)
8- return value
9+ return value.encode('utf-8')
10 else:
11 raise AssertionError(
12 "No representation implementation for media type %s"


People subscribed via source and target branches