In the light of recent events (namely https://github.com/cmsimple-xh/memberpages/issues/14 and https://github.com/cmsimple-xh/memberpages/issues/16), I want to antedate the announcement of some recent considerable changes to the view concept of PFW_XH, which is now supposed to provide automatic HTML escaping, at the price of way less flexible templates which might be regarded as a Good Thing[tm], though.
More detailed (but slightly out-dated) info regarding the new view concept is contained in the commit message.
An examplary use-case of this new concept can be found in the WidgetController of Poll_XH, and the respective view template. Also the respective unit test of Pfw_XH may provide some insights.
It seems to me that both Memberpages issues could have been prevented by using this view concept. I want to encourage plugin developers to try it out, and to provide feedback. To do so, you don't need any Controllers; actually you can stick with mostly procedural code. Just install the latest master of Pfw_XH, create and prepare a HtmlView object in a plugin function, put a respective xyz.php in your plugin's views/ folder, and play with it.
Automatic HTML escaping in plugin views
Automatic HTML escaping in plugin views
Christoph M. Becker – Plugins for CMSimple_XH
Re: Automatic HTML escaping in plugin views
Ah nice that you keep on working on Pfw_XH .
In fact: I had some problems to create the "Cache-Info" view.
As you can see in CacheService.php - cacheInfo(), I create some HTML in that method. That's not correct if I'm right (or at least against the concept). And, to output the html, I have to use $view->info = new HtmlString($cache->cacheInfo()); in MainAdminController.php.
BTW: is / was the code from Exchange_XH up to date?
I do not know if I understood all the details but I can tell you that coding this way is much easier when it comes to generate HTML output!
I have used the respective classes from Exchange_XH when developing CnC. I thought that I understood the concept. But it would be nice if you could take a look at my code.cmb wrote:I want to antedate the announcement of some recent considerable changes to the view concept of PFW_XH, which is now supposed to provide automatic HTML escaping, at the price of way less flexible templates which might be regarded as a Good Thing[tm], though.
In fact: I had some problems to create the "Cache-Info" view.
As you can see in CacheService.php - cacheInfo(), I create some HTML in that method. That's not correct if I'm right (or at least against the concept). And, to output the html, I have to use $view->info = new HtmlString($cache->cacheInfo()); in MainAdminController.php.
BTW: is / was the code from Exchange_XH up to date?
I do not know if I understood all the details but I can tell you that coding this way is much easier when it comes to generate HTML output!
Re: Automatic HTML escaping in plugin views
I have to – all this copy&paste leads to such problems as https://github.com/cmb69/twocents_xh/issues/48 (most likely 5 individual plugins are affected).Holger wrote:Ah nice that you keep on working on Pfw_XH .
Up-to-date regarding the view concept? Not really. Besides my rather old drafts, I've developed three view concepts which are relatively closely related. The first one was that of Pfw_XH 0.1.0, where view data have been injected into the templates as local variables (unescaped) or as view properties (escaped). I didn't like this manual escaping, so in several other plugins (amongst them Exchange_XH) I've refined this to inject the data as view properties (unescaped) and view methods (escaped). The templates could have been checked with a linter (actually, I've developed a draft) so that all echoing went to a call to a $this method, which guaranteed proper escaping. (A drawback was that callables didn't work anymore, but this feature was of somewhat limited use, anyway.) When turning back to Pfw_XH I've noticed that injecting view data as members of View was prone to BC issues (if I'd ever wanted to introduce a new property or method, even a private one, existing views/templates could cause havoc). Therefore I've decided (current Pfw_XH master) to inject the data as local variables, but wrapped in objects so automatic escaping would be possible. The additional benefit: no need to call $this->escape() anymore, which could amount to quite some additional code. However, I'm not really sure whether this view concept is actually viable – there are severe restrictions on what is possible in a template now.Holger wrote:BTW: is / was the code from Exchange_XH up to date?
Ah, I see. A simple improvement could be done by moving the $html stuff at the end of the method into a view/template, and let the method return the View object, like so:cmb wrote:In fact: I had some problems to create the "Cache-Info" view.
As you can see in CacheService.php - cacheInfo(), I create some HTML in that method. That's not correct if I'm right (or at least against the concept). And, to output the html, I have to use $view->info = new HtmlString($cache->cacheInfo()); in MainAdminController.php.
Code: Select all
public function cacheInfo() {
// create $files as before
$view = new View('cache-info');
$view->count = $count;
$view->bytes = $bytes;
$view->files = new HtmlString($files);
return $view;
}
A next step could be to tackle $files. Instead of precomputing $files, the respective DirectoryIterator could be passed to the template, where another loop could be made:
Code: Select all
<?php foreach ($this->files as $file):?>
<?php if (!$fileInfo->isDot() && $fileInfo->isFile()):?>
<?=$this->escape($this->file->getFilenameName())?><br>
<?php endif?>
<?php endforeach?>
Another option would be to create a view helper class for this view/template, and to pass an instance of it to the template, so in the template you'd could access $helper->getFiles(), $helper->getSize() and $helper->getCount() (or similar). Seems to be overkill here, but still a valid option. And it might simplify unit testing.
Also consider to use generators instead of arrays. I've rarely used them so far, since they require PHP 5.5.0, but they appear to be a really nice feature. Therefore I've added explicit support for them in the new view concept (well, actually there is general support for Iterators, but I don't like to manually create Iterator classes, at least not until this could be done with anonymous classes, which would require PHP 7, though).
Lack of support for anonymous classes is also the reason why I wouldn't recommend to use a FilterIterator instead of the if clause of the DirectoryIterator loop. Having anonymous classes would allow to write:
Code: Select all
$files = (new class(new \DirectoryIterator('.')) extends FilterIterator {
public function accept() {
return !$this->current()->isDot() && $this->current()->isFile();
}
});
Code: Select all
function files() {
foreach (new \DirectoryIterator('.') as $fileInfo) {
if (!$fileInfo->isDot() && $fileInfo->isFile()) {
yield $fileInfo;
}
}
}
If you conclude that this concept simplifies the generation of HTML output, I think that you have understood the idea.Holger wrote:I do not know if I understood all the details but I can tell you that coding this way is much easier when it comes to generate HTML output!
Christoph M. Becker – Plugins for CMSimple_XH