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

AreaElement sets properties directly on AreaDefinition leading sometimes to unwanted behavior

    XMLWordPrintable

Details

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

    Description

      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
      

      Checklists

        Acceptance criteria

        Attachments

          Issue Links

            Activity

              People

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

                Dates

                  Created:
                  Updated:
                  Resolved:

                  Checklists

                    Bug DoR
                    Task DoD

                    Time Tracking

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