Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #8173 (closed defect: fixed)

Opened 2 years ago

Last modified 1 year ago

[PATCH] Fix assert_select for XML documents using content_type

Reported by: dasil003 Assigned to: core
Priority: normal Milestone: 1.x
Component: ActionPack Version: edge
Severity: normal Keywords: assert_select xml
Cc:

Description

This is my proposed fix for the problem with assert_select raising warnings on xml documents. It simply checks to see if the response.content_type ends in xml, thereby catching the three common XML content types (application/xml, text/xml, and application/xhtml+xml).

This doesn't account for the common practice of sending XHTML as text/html for the benefit of Internet Explorer which doesn't understand proper mime types. However in practice parsing XHTML as HTML should not usually be a problem, whereas testing XML as HTML is very often a problem. I think using the ContentType is the right thing to do here, but I would also not discount the patch at http://dev.rubyonrails.org/ticket/1937 for making things more robust.

This would also negate the need for documentation at http://dev.rubyonrails.org/ticket/7042

Attachments

fix_assert_select_for_xml_content.diff (1.7 kB) - added by dasil003 on 09/17/07 22:37:20.
new_fix_assert_select_for_xml.diff (1.8 kB) - added by dasil003 on 09/17/07 22:50:10.

Change History

05/13/07 02:41:17 changed by dasil003

  • keywords changed from assert_select xml to assert_select xml tiny.

06/05/07 01:34:10 changed by josh

  • keywords changed from assert_select xml tiny to assert_select xml.
  • milestone changed from 1.2.4 to 1.x.

(Not so tiny)

07/14/07 18:39:17 changed by dasil003

Okay, I added a patch that looks up the mime type based on content_type per Koz's suggestion in rails-core. However after look at the details I believe the original patch is still the best way to go.

The reasoning is that there is no one type of XML content. There are many mime_types that are XML languages. So there is no single type to lookup. We still have to use a regexp to match a trailing "xml" on the mime type. This heuristic ought to be bulletproof.

The downside of looking up the mime type is threefold:

1) application/xhtml+xml is converted to text/html for purposes of IE compatibility. But this also means it would be impossible for someone to use assert_select in XHTML mode should they wish. The kind of developer who sets their content type to application/xhtml+xml is the kind who would want assert_select to be operating in XML mode. 2) If a mime_type is unknown to Rails it will not work with the lookup. The developer can set any content_type they want in their application, but adding mime types is considerably more opaque. Since the heuristic is quite solid, and the consequences of a false-positive are minimal, it seems better to support unknown types based on the standard mime type convention. 3) Given the previous two reasons, it seems wasteful to perform an extra lookup.

09/08/07 01:32:19 changed by nzkoz

  • status changed from new to closed.
  • resolution set to fixed.

(In [7417]) Fix assert_select for XML documents. Closes #8173. [dasil003]

09/17/07 22:37:20 changed by dasil003

  • attachment fix_assert_select_for_xml_content.diff added.

09/17/07 22:50:10 changed by dasil003

  • attachment new_fix_assert_select_for_xml.diff added.