-
Notifications
You must be signed in to change notification settings - Fork 40
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
Remove PHP Filter module #19
Comments
http://drupal.org/node/1203886 .... DAT RTBC queue. |
Same for Views' PHP input. Separate issue? |
Considering we don't have Views in Backdrop yet, yes. :) Moving this issue to needs work, please relabel when the PR is updated (or a new one is filed). |
what is the reason for removing php filter functionality? Isn't it something that many non-elite developers rely on? would this require that a dev or implementer write modules to handle simple use-cases where they currently rely on php filter input in a node or block? |
http://drupal.org/node/1203886
Precisely the reason for removing it. Using it exposes security holes, performance implications, breaks the upgrade path, etc.
Yes, either that or still use it http://drupal.org/project/php . |
ok, I can see your point to an extent, however I am a bit confused. |
@ericfg there's no point in having a discussion that the same community already had, that's why you'll find many references to drupal.org, most (if not all) of us come from Drupal and there are some valid points in many discussions that we just can't ignore because we're starting a new project. The conclusions, in many cases, will be the same ones. +1 to remove the thing which should had been removed like 6 years ago. |
It's easy to understand the confusion. Considering we're taking Backdrop in a different direction than Drupal, we might not always reach the same conclusions. In this case, I think our reasoning might actually be different than Drupal, but we'd reach the same conclusion. Based on our current principals (or "goals" stated on backdropcms.org), we're basically moving into the product space. A product for end-users shouldn't even require people to know what PHP code is, and certainly not allow them to inject snippets of code they may not even understand into the system. If we're going to be a system for users specifically to avoid code in the first place, we shouldn't be exposing this to those users. And there's all the other reasons mentioned in the drupal.org issue (security, upgradability, irrecoverable site breakage, data loss, etc). So regardless of Drupal's decision, this is probably the right move for us also. The current PR is failing tests (despite Travis CI saying it's passing in the overview). Let's fix the tests and remove. |
quicksketch, thanks for that reply. Although I do think that many of the same issues exist when people code custom functions in a module (security, upgrade problems, etc.), I can see the logic of the decision. I also think that it is important to recognize that this decision will create a more complex learning curve for people in the group that backdrop has stated is one of the target audiences for the fork. It might be the best decision, but I think it is important to think about these issues before just saying "here's no point in having a discussion that the same community already had" when this community is not drupal; many people will come to this community because they are not happy with decisions that were made in the drupal community; how decisions are made in the drupal community and a lack of faith in the long-term direction of the drupal community. I strongly believe that we will see many of the long standing debates revisited here, and many of them might take different directions. In general I agree that executable code does not belong in the database, but I find php filter to be very useful when doing site building -- although in nearly all cases after initial prototypes are built my code goes into custom functions in modules and is not left in the db. So, I guess I'll have to put some energy into making sure that php filter is available via a contrib module for backdrop. luiselizondo: Can we all agree in the future to not attempt to shut down any discussion with "this decision was made in the drupal community so there's no valid reason to discuss it here"? If that was a valid bottom line, there would be no reason for this fork to exist. |
@ericfg I apologize if you thought I was trying to shut down any discussion, that was not my intention. Discussion is always a good thing, I was just making a point that regarding this issue, there's plenty of discussion and your points (which are 100% valid), are probably debated and answered on the Drupal.org issue mentioned. Many D8 decisions will have to be analysed here, by no means I was generalizing, just pointing out that this issue it's been around for a while now and from my point of view, the conclusion will be the same, PHP Filter in core is not a good idea. |
All concerns and considerations combined, the much more interesting question is whether it is possible to resemble the 80% use-case of the PHP code input filter with a solution that does not involve PHP code. That pretty much boils down to an input filter introducing a macro language that allows for conditional control structures and simple expressions. The abstraction from raw PHP code ensures security and eliminates the upgrade path problem space. And of course, a limited subset of operations can be explained in filter tips/help (whereas raw PHP is impossible to explain to end-users). Looking outside of the Drupal box, reality is that raw PHP code is not supported in any other CMS (natively). However, some web applications do support a macro language concept. (I've definitely seen this before, but unfortunately cannot remember where.) |
Merged backdrop/backdrop#81. |
sub-issue from #7
The text was updated successfully, but these errors were encountered: