Add core 'JSON' and 'Text' Page Types #732
Comments
|
In my eyes a solution which can be configured (like the Content Type Mappings extension) is ideal. I agree that it would be nice to have this in the core — but not hardcoded. |
|
I agree with @michael-e about this extension, IMO the core does the minimal and is extendable for use cases like this. I don't believe that the extensio is a workaround at all, but following the standard approach to adding functionality. JSON may be added to the core, but I don't think there will be any more... |
|
JSON? Why JSON? What about JavaScript and CSS (which I am geenrating dynamically for a client)? That is the point: It must be configurable to be flexible. Don't make assumptions. |
|
Use the extension. It's what it was built for :) |
HTML, XML and JSON are by far the popular output types of websites. On numerous occasions I've had to faff around to get plain text and JSON output too. These are common tasks, so should be handled by the core. We've already got an easy of doing this for XML, so I'm with David that these can be added as page type labels. I don't think it needs anything more complex, and those that need more edge cases (JS, CSS, and any other MIME types under the sun) would continue using the Content Type Mapping extension... |
|
I agree it's probably a grey area (I could not set a label to 'suggestion' and probably should have discussed this in the forum first). However: the fact that Symphony allows us to set a Page Type of This is confusing. Especially since XSL allows us to set the media-types in our templates. IMO There have been quite a few questions re: |
|
That's what I say: If you add anything to the core, then make it configurable — the extension already is! (I am against adding any more hardcoded options to the core.) EDIT: I was slow. My answer was for @designermonkey. |
|
See #698 |
|
We should continue to use the content-mapping extension and keep the core slim... |
|
Not to press this too much, I acknowledge and appreciate the vision of keeping the core slim, and I realize this might not be a high priority fix anyway, but I would like to come back to my question:
It's not like adding 1 extra |
|
@davidhund: No it's not like adding an if/else since I don't think libxsl offers text support... |
|
@nitriques I meant adding a |
|
Maybe, but this would be a false thing to do since libxml is only capable of outputing html or xml... This is why there are extensions, i.e. Content Type mapping extension |
|
@nitriques: This is false. libxml can output text as well. (As @davidhund mentioned above, there are three output methods.) |
|
No, it's not exactly false. There are serious quirks with how text is generated from libxslt (which is what we're talking about here, not libxml). It will incorrectly add loads of whitespace throughout any text generated, which is the whitespace between your XSL nodes in your stylesheet. For anyone trying to output text as a clean document will have problems. Yes it can output text, but not properly. This is a known problem that has never been addressed as of yet. |
|
Loads of whitespace? Do you have any proof for that? (I have generated JavaScript and CSS dynamically with Symphony, and I never experienced this issue.) |
|
@nitriques @designermonkey my question/suggestion was primarily aimed at removing the confusion regarding the Page Type functionality and adhering to the W3C spec re: XSL output methods. I am not aware of these whitespace quirks regarding libxslt's handling of the output method 'Text' but I'm sure it does handle it. Personally I don't feel the fact(?) that libxslt support has whitespace quirks should prevent us from implementing this functionality. As a matter of fact: there are similar quirks/issues with the libxslt handling of the output method 'HTML' (e.g. indenting) but that was no argument implementing the 'HTML' output method, was it? ;P </tongue-in-cheeck> |
|
@designermonkey, @michael-e: yeah the problem is with libxslt (my bad).... When the output is set to text, it gives me an error. @michael-e: I generated css and js too but the output was set to html.... @davidhund: Why is usign a extension is that bad ? |
|
I still stand by my thought that since we already have |
|
@nickdunn: well add it then... I thought that keeping the core slim was more important |
|
I agree, it would be really handy to have those few basic output methods supplied by the core. Also, I don't take the argument that it "bloats the core". Basic, yet flexible input and output is a task the core should solve. Catering for JSON or Text are two of those basic output situations. Wasn't there a plan to create a |
|
I think we should add JSON in the core as it is essential if users want to create their own JSON for use in the new Remote JSON datasource (#695), or for use in their site's Javascript. @nils-werner, that utility would be super useful for doing those tasks! I'm not as convinced about text though, what's the use case? How common is it? While its not bloat in a 'lines of code' way, I feel it's bloat in a 'it's there but not used' way. |
|
@brendo My use case was a text-only e-mail message. @nickdunn mentioned having 'faffed around' trying to get plain-text output. Not common maybe, but still… Also: am I the only one having issues with the confusion regarding expectations of the |
Page Type is only ever mapped onto the
|
|
The correct mimetype for JSON is |
|
@nickdunn true (of course)… The confusion is all mine. I own it :) @michael-e: yup. |
How would you specify the extension in this case? Perhaps we should not integrate the Content-Disposition part of the Content Type Mapping extension? |
|
To me I like the simplicity; After re-reading this thread (I'm sorry to confuse, was just trying to get through my task-list), I can see the benefit of having actual MIME types in the Page Types box for an advanced user, however we need to keep this simple and complex at the same time to cater for the full user base, which we know has a lot of novice users, which is why I am still going to do this. It adds abilities for a wide range of use cases, but is still very little overhead in code. |
I was only thinking in terms of simple MIME types, I didn't consider that! Hmm. In that instance, actually typing both |
|
Thank you ;o) Decision made. |
|
My head hurts. Did you decide to go with @remie's suggestion of both allowing the MIME type ( If so, that would be great since it offers most flexibility in a transparent matter: Simple Labels ("hidden", "nav"), MIME-types (as we know 'em), File Extensions (as we know 'em) |
|
Remie's suggestion was
Which is wrong. But, yes you are right in your understanding. Examples as follows:
There will be a default set of magic keywords to include: Note about (edited as I forgot css) |
ouch... :) |
|
Sorry, didn't mean to be blunt. |
|
Yes you did :D Thanks for the summary and it sounds great. My only concern is that it is a lot of features for a single text field with the obscure |
|
We can definitely document it with examples. As for the obscurity, yes I totally agree. Maybe we should look at this for the next minor release, maybe One quick point, did @Phoque's code ever get merged? I can't find the commit on any branch??? |
|
The Page Type field presently shows the supported values under the text input, as clickable tags. This is self-documenting. Would we be able to retain this with the new mime-type implementation? |
|
We should add the simple magic options, but not mime type, they should be documented as advanced. |
Maybe I'm stating the obvious here, but just to be sure: keep in mind that this list is actually dynamic. It also shows frequently used custom types, not only a static list of default options. |
|
I am not sure I am convinced by the plans. Firstly, setting a special MIME type is a slightly advanced topic so I would expect users who are using them to be able to Google the right one for their usecase. So I don't think using a full MIME type in the type field is too complex for developers (the most commonly used are displayed below the field, a click adds them). These clickable defaults make it obvious that MIME types are accepted. In contrast, if you only use "xml", "text" and "json" in the list nobody would expect that MIME types are allowed too. Secondly, I find the What are people thinking about using the page title for the final filename? It doesn't strictly have to be a handle (the URL is different to the filename anyways), seems to be of limited use in that case and seems more obvious in both configuration and on the page index table. My suggestion goes as follows:
|
I can't actually see this in any code or commits, are you sure it's actually there? |
|
Sorry, apparently I've deleted my pagetypes branch. I've rebased and reuploaded it again. |
I strongly agree with this point, which is why I suggested that we should separate the Content-Disposition feature from the MIME type value field. |
|
I've just installed your branch Nils, and I have to say it is confusing to look at. It doesn't say MIME type anywhere, so when looking at it, it is not obvious what the tags mean. I really think we should have the tags as keywords, use keywords in the input field, and then if the user wants to add any MIME types instead, they can do for advanced control. We really have to keep confusion out of this, and see it from the perspective of people who won't just 'get it' from first look. The consensus on the downloadable Content-disposition stuff tells me to drop it, and if that is the case, then so be it, but I am not budging on the simple + advanced (if they want it) approach. it just looks wrong to me the way it is in Nils branch, and what pain is it for us to just keep the keywords in? None. |
|
I've made the changes I described above, not including the Content-Disposition header, also keeping Nils' changes, but modified to fit my description. I really think this should be merged as is to satisfy all parties and cover all possibilities. The Content-Disposition can be discussed at a later date. |
What exactly do you mean? Will we have less features in the core than with the ancient extension? Sorry if I haven't followed the discussion precisely. It was rather long-ish... Maybe my question is silly. |
|
Only one feature less. I'm just fed up of discussions taking so long when there are other things that are more important for Symphony. Personally I would like to add it, but most seem against it. I'm quite miffed as it was decided by a group that included Allen, you and myself, and others. But that seems to be how things go around here now.
|
|
Guys, please don't decide to not do stuff based on my input :) |
|
I shouldn't rant, apologies due. @remie, you're opinion does count and matters! |
Don't take it personally :) The concept of merging extension functionality into the core has (and likely always will be) a contentious issue, for obvious reasons. Don't be put off by it — often these things genuinely require a lot of thought, and often several days or even weeks or months of being on the back-burner, mulling over in your head, or our collective heads, before the ideal implementation is found and committed. (Edit: errr, reading that back it sounds horribly bohemian and preachy, which wasn't my intention. It is late.) |
|
No dude, I get it. |
|
Ah, so this is the thread that added 30 notifications to my inbox ;) So lets go back for a second. The original reason behind this issue was because JSON datasources are going to exist in Symphony 2.3, so there should be a native way to create a page that will output a page with a JSON mimetype, (I'm purposely ignoring the 'Text' option because IMO it's even less likely to be used) The discussion then continued to say, if we add JSON, why don't we add 'x'? Because we didn't want to manage all possible mime types and continually be fielding requests to keep adding additional mimetypes, a possible solution was raised that the Page Type setting would just become a free text field, and if you added a mimetype to it, the Now the discussion has gone further to say, mimetypes are ugly/advanced, lets keep it the Symphony way and allow human readable types. Content Type Mappings extension solves this rather elegantly with a key => value array where the key is the type and the value is the mimetype, so I can see the reasoning in wanting to merge this into the core. However, Content Type Mappings also allows you to set Disposition headers which will give the browser the correct mimetype to download the page as a file. From the above comments, the confusion started when it was said that However, this isn't necessary functionality, it's bloat (arguably minimal) and it's obviously pretty contentious on how this is implemented. The solution? Lets just add |
|
Content Type Mappings was agreed to be deprecated at Symposium, in favour of the tiny minimal change to the core. If this is what is going to happen every time I have to do a task on my list, then I give up. |
|
Please don't give up, John. I think the problem is that half of the group thought this case was already closed, so somehow Symposium reopened the discussion. What about having a break for a day in this discussion? |
|
Agreed. Let's make it "virtually closed" until tomorrow. |
At the moment Symphony supports only 1 alternative to the default
text/htmlcontent-type, namelyXML. Setting aPage TypetoJSONorTexthas no effect on the XSL outputcontent-type.The XSL templates allow us to set a
xsl:outputmethodandmedia-typebut changing these to e.g.textandtext/plainwill not result in the correctcontent-typeoutput. This prevents users from e.g. easily creating plain-text e-mails or JSON output.There's a workaround in the form of the Content Type Mappings extension (and some elaborate utilities) but IMO adding both
TextandJSONto the 'core' options (and setting the correctcontent-typeheaders) would be quite easy to do and prevent confusion.I believe the only change would be to add the two
content-typeheaders to theif/elsein class.frontendpage.php:226The text was updated successfully, but these errors were encountered: