How to handle IDs in prepared XDOM

Development
1

Hi everyone,

We have this problem Loading... :

  • when we prepare the content (of a document or a macro) we pass an ID generator to the parser, most of the time, so the prepared content has some IDs generated for heading and image blocks
  • when using the prepared content we have to fix the IDs that were generated on parse in order to ensure they are unique in the context where the prepared content is inserted (i.e. in the scope of a new ID generator)

I created two PRs to fix this:

I simply followed the same approach as @MichaelHamann when fixing XRENDERING-6: Id are not unique when included document or macro conte… · xwiki/xwiki-platform@ec21f5f · GitHub , moving his code to rendering, adding 2 new public APIs:

  • IdGenerator#adaptId(String) to be able to adapt existing IDs in the scope of this ID generator
  • XDOM#setIdGenerator(IdGenerator, boolean) to be able to set a new ID generator and also adapt existing IDs to be unique in its scope

The result is that IDs are now unique BUT, very often, the generated IDs are not contiguous because some IDs are adapted even when there is no need to. To give an example, suppose you have a page with this content:

= Heading =

{{info}}
= Heading =
{{/info}}
  • when the content of the page / macro is prepared, both headings get a generated ID (HHeading, HHeading-1)
  • then, when the page is displayed:
    • the prepared macro content is cloned, and the cloned XDOM gets a copy of the original ID generator
    • we set the context ID generator on the prepared content and adapt the existing IDs

If the page is displayed alone, then the context ID generator is the same as the one used to prepare the content, so there’s no need to adapt the IDs. But we do it, so we get HHeading, HHeading-1-1 (could be HHeading-2, the point is that the values are not contiguous).

If the page is included in another page, then the context ID generator is not the same as the one used to prepare the content so we definitely must adapt the IDs.

The issue is that I don’t see a clean / easy way to detect if the IDs need to be adapted, and I’m worried that we might miss some cases in that check. It’s better to have unique but not contiguous IDs than having duplicate IDs. I have thought of:

  • 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 (have the same generated ID sets); my worry with this is that we might have cases where the ID generators have the same generated IDs, but they were used on different content, so we might get duplicated IDs

WDYT?

Is the approach I took the right one? Do you agree with the new public APIs I exposed? Do you think it’s acceptable to have unique but not contiguous IDs?

Thanks,
Marius