Details
-
Bug
-
Status: Resolved
-
Minor
-
Resolution: Fixed
-
0.5.0
-
None
-
None
Description
The content viewer does not display json content when the content-type string does not exactly match "application/json" for example: "application/json;charset=UTF8" or "application/Json" trigger the error message "No viewer is registered for this content type."
According to RFC 2045 there can be any number of parameters following the type and subtype values, so I believe these should not impact the viewer chosen to display this content.
Furthermore, the rfc states that type and subtype are to be treated case insensitive, which is also not given as all checks I found in the code are purely against lowercase.
As far as I can tell this is checked twice, the viewer for the content type is obtained from a Context object, if there is no viewer that matches exactly the error message stated above is shown.
After this was successful a string comparison is done in the StandardContentViewerController class:
if ("application/json".equals(contentType) || "application/xml".equals(contentType) || "text/plain".equals(contentType) || "text/csv".equals(contentType)) { ... }
So even if the Viewer was assigned to this content type if would fail here.
Proposed solution
- Add code to DownloadableContent.java to parse content type string at creation and store only lowercased type and subtype, which is retrieved by getter method, so no changes are necessary to downstream code
- Add map to object to store parameters that were contained in the original value
- Either make the entire parameters map object retrievable or create getter to retrieve parameter from map by parameter name
- Add method to retrieve original field value, I propose to build the value on request from stored type, subtype and parameters, this way we can implement some light error handling and ensure to always hand out properly formed strings, even if the input value may have had some small error (there is no need to pay attention to parameter order, as per rfc the order shouldn't matter)
Discussion points
- Anything that actually relies on access to parameters in the content type would break, this could be avoided by not changing the getType method but instead adding a new method to return type without parameters instead. Drawback to this is that current implementation of ContentViewerController would need to be changed to use this new function. I am unsure which aproach is preferable..
- If there are classes downstream that access the content type field not via DownloadableContent, this shouldn't break anything, as the status quo is preserved, but there is a small chance that you get two different content types, if you actually use both methods
I am happy to create a pull request to this effect if open points can be clarified and this is deemed useful.