Uploaded image for project: 'XWiki Platform'
  1. XWiki Platform
  2. XWIKI-24781

In-place editing of the Image Styles administration properties is broken by undefined $xobject and $xclass variables

    XMLWordPrintable

Details

    • Unknown
    • N/A
    • N/A

    Description

      Problem

      Image/Style/Code/Administration.xml references two Velocity variables, $xobject and $xclass, that the document never defines. They look like leftovers from an object-sheet template, where they would have been bound by an enclosing loop over the document's xobjects.

      The document sets up its own variables:

      #set ($configurationDoc = $xwiki.getDocument('Image.Style.Code.Configuration'))
      #set ($configurationObj = $configurationDoc.getObject('Image.Style.Code.ConfigurationClass'))
      #set ($configurationClass = $configurationObj.xWikiClass)
      

      but then, for each of the two properties, uses $xobject and $xclass instead:

      #set ($xobjectPropertyReference = $xobject.getPropertyReference('defaultStyle'))
      <dt class="editableProperty"
          data-property="$escapetool.xml($services.model.serialize($xobjectPropertyReference))"
          data-property-type="object">
        ## This must match the id generated by the $doc.display() method below.
        #set ($propertyId = "${xclass.name}_${xobject.number}_defaultStyle")
      

      Two consequences:

      • $xobjectPropertyReference is never assigned, so data-property is emitted as the literal string $escapetool.xml($services.model.serialize($xobjectPropertyReference)) on both <dt class="editableProperty"> elements.
      • $propertyId is computed from undefined variables and is then never used at all — the <label for=...> below it is hardcoded. It is dead code, and the comment above it ("This must match the id generated by the $doc.display() method below") is misleading because nothing matches anything.

      User visible consequence

      In-place editing of Default Style and Force Default Style in the Image Styles administration section does not work, and fails silently.

      editableProperty.js loads the editor with:

      $.get(XWiki.currentDocument.getURL('get'), {
          xpage: 'display',
          mode: 'edit',
          property: editableProperty.data('property'),
          type: editableProperty.data('propertyType'),
          ...
      })
      

      so it sends the literal Velocity string as the property parameter. display.vm passes it to #displayObjectProperty, which does $services.model.resolveObjectProperty($propertyReference) then $Unable to render embedded object: File (object.display(...)}}. The reference resolves to nothing, {{$object}} is null, and the {{$) not found. silences the result, so the response is HTTP 200 with an empty body. The client treats that as a success and injects an empty editor: the property simply disappears from the section, with no error notification and nothing in the logs.

      The form's own "Update the default image style" submit button is unaffected, so only in-place editing is broken.

      A second defect on the same two elements

      Fixing data-property alone is not sufficient. #displayObjectProperty reads the object off the current document:

      #macro (displayObjectProperty $propertyReference $displayMode)
        #set ($propertyReference = $services.model.resolveObjectProperty($propertyReference))
        #set ($object = $doc.getObject($propertyReference.parent))
        $!object.display($propertyReference.name, $displayMode)
      #end
      

      and editableProperty.js always requests XWiki.currentDocument, which in the administration is XWiki.XWikiPreferences. The property being edited lives on Image.Style.Code.Configuration, so $doc.getObject(...) returns null there too and the response is again 200 with an empty body. editableProperty.js offers no way to target another document.

      So this section cannot use editableProperty for a property stored on a different document without a change on one side or the other.

      How to reproduce

      On any instance, as a user with admin rights:

      curl -s -u <admin>:<password> "http://localhost:8080/xwiki/bin/admin/XWiki/XWikiPreferences?editor=globaladmin&section=image.style&basicauth=1" \
        | grep -o 'data-property="[^"]*"'
      

      Both occurrences print the raw Velocity string. The section's source document renders the same way and needs no admin rights, which makes it a convenient check:

      curl -s "http://localhost:8080/xwiki/bin/view/Image/Style/Code/Administration" | grep -c 'xobjectPropertyReference'
      

      That prints 2 when the bug is present and 0 when it is not.

      To observe the empty editor response directly, which is what the browser gets when the edit pencil is clicked:

      curl -s -G -u <admin>:<password> -w "http=%{http_code} bytes=%{size_download}\n" \
        --data-urlencode "xpage=display" --data-urlencode "mode=edit" \
        --data-urlencode "property=Image.Style.Code.Configuration^Image.Style.Code.ConfigurationClass[0].defaultStyle" \
        --data-urlencode "type=object" --data-urlencode "basicauth=1" \
        "http://localhost:8080/xwiki/bin/get/XWiki/XWikiPreferences"
      

      Verified on 18.8.0-SNAPSHOT.

      Suggested fix

      Both $configurationObj and $configurationClass are already in scope, so the markup can be built from them directly: use $configurationObj.getPropertyReference('defaultStyle') for data-property, and drop the unused $propertyId #set together with its stale comment. That fixes the malformed attribute.

      Making in-place editing actually work additionally requires addressing the current-document mismatch described above — either by teaching editableProperty.js to accept the document holding the property, or by not using editableProperty for these two fields.

      Measured evidence

      The four combinations below were run against a healthy 18.6.0-SNAPSHOT instance, replaying the request that editableProperty.js#loadEditor makes (xpage=display&mode=edit&property=…&type=object). They are recorded here so that whoever fixes this does not have to re-derive them:

      document requested property value response
      Image.Style.Code.Configuration correct reference 423 (edit confirmation) — confirms the probe shape is valid, the template did run
      Image.Style.Code.Configuration the malformed literal 423 (the confirmation fires before the property is resolved)
      XWiki.XWikiPreferences correct reference 200, 0 bytes
      XWiki.XWikiPreferences the malformed literal 200, 0 bytes

      The third row is the important one: XWiki.XWikiPreferences is what editableProperty.js actually requests in the administration, and it returns an empty editor even with a correct data-property. That is why fixing the attribute alone is not enough, and it is also why the failure is silent — a 200 is a success as far as the client is concerned, so it injects an empty editor and shows no notification.

      Possible approaches

      The two defects do not have a single obvious combined fix, so this needs a decision before coding:

      1. Fix the attribute and stop advertising in-place editing on these two properties — build data-property from $configurationObj, drop the dead $propertyId #set and its stale comment, and remove class="editableProperty" from the two <dt> elements. Smallest change, confined to the one XAR page, and it makes the UI honest: the section's own "Update the default image style" submit button already works and is unaffected. It does not deliver working in-place editing.
      2. Fix the attribute and make in-place editing work — additionally teach editableProperty.js and display.vm to accept the document that holds the property (for instance via a data-document attribute), since neither currently offers a way to name it. This delivers the feature but touches shared web resources used by many sheets, so it needs its own testing and would arguably be better tracked as a separate issue.
      3. Move the configuration xobject onto XWikiPreferences — would make the current-document assumption hold, but it is a data model change with migration implications. Mentioned only for completeness.

      Test coverage

      ImageStyleIT#imageStyleAdministration exercises this section but does not exercise in-place editing, and it passes with the bug present, which is why this went unnoticed. If approach 2 is taken, extending that test would need a new page object method on ImageStyleAdministrationPage.

      Unrelated but worth knowing when working on that test: it is a known flicker, tracked in XWIKI-24720 and XWIKI-24778. Since 1e5024c0ef8 its page object dumps the rendered section markup and the REST view of Image.Style.Code.Configuration on failure, so do not be surprised by the extra output.

      Origin

      Present since the section was written, in commit e3a48ce0df9 ("XWIKI-19459: Implement the images style administration backend", 2022-03-02), first released in 14.3.

      Attachments

        Activity

          People

            mleduc Manuel Leduc
            vmassol Vincent Massol
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: