[MGNLSITE-39] SiteAwareRendererWrapper fails to render optional areas Created: 18/Sep/15 Updated: 12/Feb/16 Resolved: 11/Feb/16 |
|
| Status: | Closed |
| Project: | Magnolia Site Module |
| Component/s: | api |
| Affects Version/s: | 1.0.2 |
| Fix Version/s: | 1.0.5 |
| Type: | Bug | Priority: | Neutral |
| Reporter: | Tobias Mattsson | Assignee: | Tobias Mattsson |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | 0d | ||
| Time Spent: | 35m | ||
| Original Estimate: | Not Specified | ||
| Template: |
|
| Acceptance criteria: |
Empty
|
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
| Bug DoR: |
[ ]*
Steps to reproduce, expected, and actual results filled
[ ]*
Affected version filled
|
| Sprint: | Kromeriz 30 |
| Story Points: | 2 |
| Description |
|
It fails with an NPE in SiteAwareRendererWrapper on line 84 where it tries to use the content. It appears that optional areas are rendered without a content node. In AreaElement.java we have this code:
private boolean canRenderAreaScript() {
if (!this.isAreaDefinitionEnabled) { // area script can be rendered only when area is enabled
return false;
}
if (this.areaNode != null) {
return true;
}
if (this.optional && this.getServer().isAdmin() && !MgnlContext.getAggregationState().isPreviewMode()) { // render script for optional areas when being in edit mode on author instance
return true;
}
return false;
}
So if areaNode is null and optional is true and we're in preview mode on an author instance then we go ahead and call the renderer without a content node. The template script is then rendered. This works fine in JspRenderer and FreemarkerRenderer but in SiteAwareRendererWrapper throws an NPE. Without a content node SiteAwareRendererWrapper can't get the current site, and can't decide if it's rendering a page (well it could look at the renderable definition id to see if it has pages/ in it but there's no point since it doesn't have a site to use anyway). SiteAwareRendererWrapper should just skip template prototype merging altogether and proceed with rendering when content is null. Basically assuming that when content is null its an area so there's nothing to do anyway. With this change the fix would be a simple null check. |