Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spin out baked-in syndication feeds to forms #1761

Open
petecooper opened this issue Dec 11, 2021 · 15 comments
Open

Spin out baked-in syndication feeds to forms #1761

petecooper opened this issue Dec 11, 2021 · 15 comments

Comments

@petecooper
Copy link
Member

Currently we offer Atom and RSS syndication formats, they work just fine and the code hasn't been touched in a while. Any modification to the feed generator will trigger a notification in Diagnostics that core files have been modified. Some knowledge of PHP and SQL is required to modify the files.

If we spin out the generator to forms, we could easily extend scope to offer additional formats like JSON Feed, or leave as an exercise to the admin for any flavour-of-the-month and / or uncommon format. Similarly, the admin could enable or disable each syndication type, rather than a blanket on or off for syndication.

Considerations:

  • This can already be done manually in forms, but requires some knowledge, providing a boilerplate will make feed customisations more straightforward (e.g. https://forum.textpattern.com/viewtopic.php?id=51608).
  • URL rewriting for existing feed subscribers, ideally internally instead of .htaccess or similar…or perhaps consider a new URL schema to not break backwards compatibility.

I'll close #1536 since it's now covered under this issue.

@bloatware
Copy link
Member

We should also add file download forms here, but it would rather be in 4.(8.)9 for me.

@petecooper petecooper added this to the v4.9 milestone Dec 12, 2021
@petecooper petecooper changed the title Consider spinning out baked-in syndication feeds to forms Spin out baked-in syndication feeds to forms Dec 25, 2021
bloatware added a commit that referenced this issue Feb 4, 2022
@bloatware
Copy link
Member

I have tentatively added a file_download_header form (the name to debate) for altering the file download process. For example, putting there

<txp:header name="content-disposition" value="inline" />
<txp:header name="content-type" value="application/pdf" />

will try to display files as inline pdf rather than offering them to download. Actually, this is already possible via file_download callback + plugins, but it's only one line to add/remove. Comments welcome.

@Bloke
Copy link
Member

Bloke commented Feb 4, 2022

That's so simple and elegant. Love it.

I haven't actually tested this but won't it trigger a 'form not found' in debugging/testing mode? fetch_form() seems to throw one, and that's used by parse_form().

This whole thing about 'essential' forms is still lurking over our heads, and it'd be nice to one day be able to axe them all and have Txp gracefully fallback on built-in default content if the form doesn't exist. Is there any way we can work towards that?

@Bloke
Copy link
Member

Bloke commented Feb 4, 2022

My only reservation about this is that there's only one header form. Is there scope with what's available (globally, or passed in) to be able to do some conditional testing in the file_download_header form? So, for instance, if the downloadable content is in the category 'publications' then output the PDF header, if it's in 'images' then output a suitable header based on the file extension (e.g. image/jpeg or image/png). And so forth.

This concept would play very nicely with a possible use case that I'd like to bounce off you via email.

@bloatware
Copy link
Member

All buffers are cleaned on valid files output, so no warnings should be issued. Not in my tests, at least.

Yes, $thisfile data is available on this stage, so all file tags should work ok. Sidenote: <txp:header name="content-type" value="*" /> seems to make Firefox (at least) auto-detect the file type.

And yes, it would be cleaner, say, to have a pref containing possibly multiple file header forms. Absolutely nothing is set in stone, especially because it is more complex for rss/atom/etc output.

@Bloke
Copy link
Member

Bloke commented Feb 4, 2022

Interesting. All good options. Let's see how this plays out with syndication feeds and go from there.

@bloatware
Copy link
Member

Also, should it be at all a per theme thing? Because the forms are...

@Bloke
Copy link
Member

Bloke commented Feb 4, 2022

I'm inclined to say yes. There's no other reliable way to do it. parse_form() has theme support baked in so yeah, whatever current theme is in play is the one that's used at the point of download.

I keep going over in my head whether offering the ability to call a particular form from a particular theme is useful or not, and always talk myself out of it.

Using per-theme forms also means the possibility of offering different behaviour in different sections if you mix and match themes across your site, and is also useful for improving handling of content during live/dev workflow upgrades.

@petecooper
Copy link
Member Author

I keep going over in my head whether offering the ability to call a particular form from a particular theme is useful or not, and always talk myself out of it.

What're the downsides, out of interest? If we end up taking the Wordpress route of parent and child themes, that presumably becomes a legit use case for it.

@Bloke
Copy link
Member

Bloke commented Feb 4, 2022

The downside is mainly head-scratching and lack of portability. If you had this in a template somewhere to "call in" a form from a different theme:

<txp:output_form form="iron_man" theme="marvel" />

What happens if you export the current theme, bundle it up and make it available to the general populace? The end user gets broken theme access because that second theme isn't available in the destination system.

Also, what happens if you rename the theme? Bang. Or try and use it in a dev situation with a different version number?

Yes, things break if you rename a form but it's more localised and easier to debug.

@bloatware
Copy link
Member

I'm fine with per theme forms, the question is per theme file output. Say, if we want some files to be displayed inline, we need to create a corresponding form for every theme attached to default section.

@Bloke
Copy link
Member

Bloke commented Feb 4, 2022

I think that complicates things too much. UNLESS we can figure out a way of checking a theme-specific content location first and THEN falling back on the 'global' 'files area before issuing a 404.

Same with images. Per-theme images would be handy to bundle up if your theme relies on them (and we sort out the importer so it can inject assets into the DB without blatting existing content). Themers would need to be careful to reference images/files by name (assuming no clashes during import in the DB) and NOT by ID as there could be no guarantee that the IDs match when imported into someone else's database

Or are you talking about a way of adding a theme dropdown to the Files panel (and theme column in txp_file?) so the file can only be accessed in that theme? And if a file has no theme assigned to it, then it can be considered 'global' and accessible from anywhere?

@bloatware
Copy link
Member

I'm just saying that storing file output headers in forms complicates things. Say, you decide to display some pdf files inline rather than offering them for download (which is txp default behaviour). You create a form with attachment: inline headers etc, and it works fine. Now you decide to change the default theme, for whatever reason. But the new theme does not contain you custom inline form, so file downloads switch to the default behaviour...

I'd say that the file output format is rather a site owner choice than a theme dependent feature. But I have very little 'real world' web dev experience.

@Bloke
Copy link
Member

Bloke commented Feb 4, 2022

I concur. Leave it to the site admin to decide.

@bloatware
Copy link
Member

Where do we store it then, if not in a form (because forms belong to themes)? A pref, something else?

jools-r added a commit to jools-r/textpattern that referenced this issue Aug 21, 2022
bloatware added a commit that referenced this issue Jan 6, 2023
@petecooper petecooper modified the milestones: v4.9, v4.10 Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants