|
| Description: |
h2. 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:
{code:none}
#set ($configurationDoc = $xwiki.getDocument('Image.Style.Code.Configuration'))
#set ($configurationObj = $configurationDoc.getObject('Image.Style.Code.ConfigurationClass'))
#set ($configurationClass = $configurationObj.xWikiClass)
{code}
but then, for each of the two properties, uses {{$xobject}} and {{$xclass}} instead:
{code:none}
#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")
{code}
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.
h2. 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:
{code:java}
$.get(XWiki.currentDocument.getURL('get'), {
xpage: 'display',
mode: 'edit',
property: editableProperty.data('property'),
type: editableProperty.data('propertyType'),
...
})
{code}
so it sends the literal Velocity string as the {{property}} parameter. {{display.vm}} passes it to {{#displayObjectProperty}}, which does {{$services.model.resolveObjectProperty($propertyReference)}} then {{$!object.display(...)}}. The reference resolves to nothing, {{$object}} is null, and the {{$!}} 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.
h2. A second defect on the same two elements
Fixing {{data-property}} alone is not sufficient. {{#displayObjectProperty}} reads the object off the *current* document:
{code:none}
#macro (displayObjectProperty $propertyReference $displayMode)
#set ($propertyReference = $services.model.resolveObjectProperty($propertyReference))
#set ($object = $doc.getObject($propertyReference.parent))
$!object.display($propertyReference.name, $displayMode)
#end
{code}
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.
h2. How to reproduce
On any instance, as a user with admin rights:
{code:none}
curl -s -u <admin>:<password> "http://localhost:8080/xwiki/bin/admin/XWiki/XWikiPreferences?editor=globaladmin§ion=image.style&basicauth=1" \
| grep -o 'data-property="[^"]*"'
{code}
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:
{code:none}
curl -s "http://localhost:8080/xwiki/bin/view/Image/Style/Code/Administration" | grep -c 'xobjectPropertyReference'
{code}
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:
{code:none}
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"
{code}
Verified on 18.8.0-SNAPSHOT.
h2. 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.
h2. 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.
h2. Possible approaches
The two defects do not have a single obvious combined fix, so this needs a decision before coding:
# *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.
# *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.
# *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.
h2. 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.
h2. 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.
|
| Assignee: |
Manuel Leduc |
|
