Uploaded image for project: 'Magnolia'
  1. Magnolia
  2. MAGNOLIA-6309

AreaElement sets properties directly on AreaDefinition leading sometimes to unwanted behavior

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Neutral Neutral
    • 5.4.1
    • 5.4
    • None
    • None
    • Sprint 4 (Kromeriz)
    • 5

      I run into this when testing STK + MTK installed alongside (CE).

      • MTE installed first
      • STK installed second
      • opened travel demo (freemarker errors)
      • then opened STK demo .. this demo should work, but it didn't because:
        • opening the travel demo led to rendering of htmlHeader - AreaElement sets renderType to site and i18nBasename to info.magnolia.module.travel-demo.messages on the corresponding AreaDefinition
        • since we are setting these values on the AreaDefinition directly, everytime when we now obtain site's prototype, these values are there leading to incorrect behavior (wrong renderer resolved)
        • workaround to this problem is to open site we have configured in the site module as active

      Erroneous code (info.magnolia.templating.elements.AreaElement):

      // FIXME we shouldn't manipulate the area definition directly
// we should use merge with the proxy approach
if (areaDefinition instanceof ConfiguredAreaDefinition) {
    if (areaDefinition.getTemplateScript() == null) {
        ((ConfiguredAreaDefinition) areaDefinition).setRenderType("noscript");
    } else if (areaDefinition.getRenderType() == null) {
        ((ConfiguredAreaDefinition) areaDefinition).setRenderType(this.templateDefinition.getRenderType());
    }

    if (areaDefinition.getI18nBasename() == null) {
        ((ConfiguredAreaDefinition) areaDefinition).setI18nBasename(this.templateDefinition.getI18nBasename());
    }
}

      As the FIXME comment suggest, we should use merging utility instead of setting the values directly

      Possible fix (worked for me):

      ConfiguredAreaDefinition definition = new ConfiguredAreaDefinition();
if (areaDefinition instanceof ConfiguredAreaDefinition) {
    if (areaDefinition.getTemplateScript() == null) {
        definition.setRenderType("noscript");
    } else if (areaDefinition.getRenderType() == null) {
        definition.setRenderType(templateDefinition.getRenderType());
    }

    if (areaDefinition.getI18nBasename() == null) {
        definition.setI18nBasename(templateDefinition.getI18nBasename());
    }
}
definition = BeanMergerUtil.merge(areaDefinition, definition);
// Use definition instead of areaDefinition

        Acceptance criteria

              jsimak Jaroslav Simak
              jsimak Jaroslav Simak
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Created:
                Updated:
                Resolved:

                  Bug DoR
                  Task DoD

                    Estimated:
                    Original Estimate - Not Specified
                    Not Specified
                    Remaining:
                    Remaining Estimate - 0d
                    0d
                    Logged:
                    Time Spent - 1d 4.75h
                    1d 4.75h