Hi everyone,
while working on refactoring the macro configuration UI I discovered a mechanism I wasn’t aware of, which I don’t like much but which seems to be some kind of undocumented API. So the proposal if about removing it.
So the thing is that right now, when a macro parameter belongs to a group, we apparently set a property data-property-group with the name of the groups, see the code here: xwiki-platform/xwiki-platform-core/xwiki-platform-ckeditor/xwiki-platform-ckeditor-ui/src/main/resources/CKEditor/MacroService.xml at xwiki-platform-16.10.8 · xwiki/xwiki-platform · GitHub
Apparently, this value is currently used in XWiki Standard in 2 displayers: entityreference and entitytype, specifically here: xwiki-platform/xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-templates/src/main/resources/templates/html_displayer/entityreferencestring/edit.vm at 45467ab4c5551c53c8cc5952c4405485e730809e · xwiki/xwiki-platform · GitHub and here: xwiki-platform/xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-templates/src/main/resources/templates/html_displayer/entitytype/edit.vm at 45467ab4c5551c53c8cc5952c4405485e730809e · xwiki/xwiki-platform · GitHub and in the associated javascript code: xwiki-platform/xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-war/src/main/webapp/resources/uicomponents/suggest/suggestEntities.js at 45467ab4c5551c53c8cc5952c4405485e730809e · xwiki/xwiki-platform · GitHub
If you check the code, you can notice that we actually never check the value of the property, we only check its presence and assume that we must perform actions based on that.
My understanding is that this mechanism was put in place for handling the group of macro parameters dealing with references in the Include macro. But not testing the value of the group is a bug: we could have macro parameter of type entitytype belonging to a group which has nothing to do with the UC of defining a reference.
So the usecase of having to be able to deal with custom displayers is real, but for me for such usecase we should define a custom type which would have its own displayer (which could reuse standard code if needed).
Now the current implementation of the displayers in XWiki Standard goes beyond the macro parameter UC since it’s a type displayer: so this data-property-group check could also be used in other UC. The thing is that I haven’t found any other usage of this property key in XS, and neither in xwiki-contrib.
So my proposal is to:
- remove assigning this property key in MacroService
- remove the if checks and related code in the standard displayers
- provide custom types for Include macro and the related displayer reusing what was done in the standard displayers
wdyt?