Skip to content

XRENDERING-802: Prepared macro content leads to duplicated IDs when inserted#374

Open
mflorea wants to merge 1 commit intomasterfrom
XRENDERING-802
Open

XRENDERING-802: Prepared macro content leads to duplicated IDs when inserted#374
mflorea wants to merge 1 commit intomasterfrom
XRENDERING-802

Conversation

@mflorea
Copy link
Copy Markdown
Member

@mflorea mflorea commented Apr 3, 2026

Jira URL

https://jira.xwiki.org/browse/XRENDERING-802

Changes

Description

  • Add IdGenerator#adaptId(String) to adapt an existing ID (make it unique in the scope of this ID generator)
  • Add XDOM#setIdGenerator(IdGenerator, boolean) to allow adapting the existing IDs when changing the ID generator of an XDOM
  • Adapt the existing IDs from the prepared macro content XDOM clone before inserting it into the target document
  • Add unit tests

Clarifications

  • For IdGenerator#adaptId(String) I kept the method name that @michitux introduced (I basically moved the private method from DocumentContentAsyncExecutor). I find the name to be OK, but I'm open to change it if you have better suggestions
  • XDOM#setIdGenerator(IdGenerator, boolean), I initially thought we could adapt existing IDs every time a different ID generator is set, but this breaks the XDOM cloning use case where we set on the clone a copy of the ID generator from the source XDOM, but we don't want to adapt the IDs, otherwise the clone would not equal the source.
  • I don't think we should expose XDOM#makeIdsUnique() because I don't see other use case for it than when you change the ID generator
  • The current implementation has a small limitation: IDs from the prepared content are sometimes adapted even if there is no need to. When the content is prepared it is parsed with an ID generator, so some IDs are generated. Then, when the prepared content is used we set a new ID generator (from the target document) and adapt the existing IDs. This new ID generator is most of the time a copy of the original ID generator, but it's still different, so all IDs are adapted. This is obviously better than having duplicate IDs, but it can cause confusion. To avoid this we could:
    • Stop using an ID generator when parsing the XDOM to be prepared, and generate the IDs afterwards when the prepared XDOM (clone) is used. This seems doable for macro content but requires more changes for document content that I'm not comfortable doing
    • Add an IdGenerator#equals() and use it to avoid adapting the existing IDs if the ID generators are equal; my worry with this is that we might have cases where the ID generators have the same generated IDs but thy were used on different content, so we might get duplicated IDs. I'm not sure if it's worth taking this risk just to have continuous IDs.
  • See XRENDERING-802: Prepared macro content leads to duplicated IDs when inserted xwiki-platform#5370 for the platform changes.

Expected merging strategy

  • Prefers squash: Yes
  • Backport on branches:
    • stable-17.10.x

…nserted

* Add IdGenerator#adaptId(String) to adapt an existing id (make it unique in the scope of this id generator)
* Add XDOM#setIdGenerator(IdGenerator, boolean) to allow adapting the existing ids when changing the id generator of an XDOM
* Adapt the existing ids from the prepared macro content XDOM clone before inserting it into the target document
* Add unit tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant