-
Notifications
You must be signed in to change notification settings - Fork 11
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
Improvements for com:pages #152
Conversation
We no longer fallback on the file locator when importing relative templates. Component and modules templates are always loaded through their respective locators.
- Allow to pass a template when loading from source - Add locations cache to store the url of source template - Qualify relative template during import - Only locate fully qualified templates - Only store the template url in the stack
- Allow to pass a template when loading from source - Qualify relative template during import - Only locate fully qualified templates - Only store the template url in the stack
- Remove getBasePath() and seBasePath() methods - Add qualify() method
This reverts commit 86659a3.
Handle normalisation of template urls by resolving references to /./, /../ and extra / characters in the input url and returns the canonicalized absolute url.
- Allow to get the basepath using getBasePath() method - Prevent directory traversal outside of the basepath - Improve find() implementation to increase flexibility
# Conflicts: # code/libraries/joomlatools/library/template/engine/markdown.php
…tcher instead of deprecated JDispatcher
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johanjanssens left you two comments
public function triggerContentPrepare($config = array()) | ||
{ | ||
// Can't put arguments through KObjectConfig as it loses referenced variables | ||
$attributes = isset($config['attributes']) ? $config['attributes'] : array(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johanjanssens $attributes
is not used below. Also an $attributes array with less than 2 elements will produce a warning because of the direct access to $config['attributes'][1]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ercanozkaya I have hardened the implementation. Requiring at least 2 attributes now at all times, the event needs a context and content to acted on, rest is optional.
array_push($this->_stack, $url); | ||
|
||
//Store the location | ||
$this->_locations[$url] = $file; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johanjanssens what happens if $url
is null here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, should only store the location if an url is provided. Hardened this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johanjanssens we are also pushing null
to the stack but since they are popped after render it should be fine right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, no problem there.
@ercanozkaya Can you re-review this. I'm not calling the onContentPrepare event directly and handling the result. For consistency I let the event handler also return the result which makes this much more use-full in a template context. |
An optional condition attribute can defined to define a more advanced condition as to when the placeholder should be rendered. Only if the condition evaluates to TRUE the modules will be rendered. Example <khtml:modules position="sidebar" condition="sidebar >= 2"> In this case the sidebar will be rendered only if at least two modules have been injected.
This reverts commit 2c89503.
- Use KTemplateExceptionError - Return the real file path
# Conflicts: # code/libraries/joomlatools/component/koowa/template/helper/event.php
Note: this branch has been merged in /develop instead of master. |
This PR includes improvements implemented in com:pages
1. Always locate a fully qualified template url
Refactored the way relative templates are located. Relative templates are now qualified during import and the fully qualified url is stored in the engine stack. As a result upon a subsequent import of a relative template the engine no longer jumps to the file locator.
Note: this can have impact on other extensions and needs to be tested completely.
2. Provide abstraction of onContentPrepare
Added an triggerContentPrepare method to the helper and provided abstraction through the trigger method.
Note: this change also changed the use of JDispatcher to JEventDispatcher as JDispatcher has been deprecated in Joomla.
3. Allow to exclude file types from rendering
Allow the template to exclude file types from rendering and just return the content of the file. By default only 'html' files are excluded. See: 7243c24
4. Add support for modules condition
An optional condition attribute can defined to define a more advanced condition as to when the placeholder should be rendered. Only if the condition evaluates to TRUE the modules will be rendered.
Example
<khtml:modules position="sidebar" condition="sidebar >= 2">
In this case the sidebar will be rendered only if at least two modules have been injected. See: a21dcfd