Skip to content

Conversation

@CybotTM
Copy link
Contributor

@CybotTM CybotTM commented Jan 4, 2026

Summary

Fixes broken permalink reference in Documentation-rendertest/Confval/Index.rst that causes test failures with --minimal-test flag.

Discovery

While working on PR #1143 (performance optimizations), CI tests failed with:

Reference confval-case-array could not be resolved in Confval/Index

This was causing make test-rendertest to exit with code 1 because --minimal-test sets failOnError('warning').

Technical Analysis

The broken line was:

Permalink to confval: `array of cObjects  <https://docs.typo3.org/permalink/rendertest:confval-case-array>`_.

Why it fails

The permalink URL rendertest:confval-case-array is processed by ReplacePermalinksNodeTransformer which creates:

  • ReferenceNode with targetReference='confval-case-array'
  • linkType='std:label' (default)

But the actual confval anchor (defined in ConfvalTrees.rst with :name: case-array) is registered as:

  • anchor='case-array' (without the confval- prefix)
  • linkType='std:confval'

The reference resolver looks for:

$internalLinkTargets['std:label']['confval-case-array']

But the target exists at:

$internalLinkTargets['std:confval']['case-array']

They don't match! The permalink format doesn't support specifying link types.

The correct approach

Using the proper :confval: text role:

Permalink to confval: :confval:`case-array` (defined in ConfvalTrees.rst).

This creates a ReferenceNode with linkType='std:confval' and anchor='case-array', which resolves correctly.

Question for maintainers

This broken reference was introduced in PR #975 (commit c310707) on May 4, 2025 by @linawolf.

Was this intentional? Possible interpretations:

  1. Typo/mistake: The author expected confval-case-array to work like :confval:case-array`` but they're fundamentally different
  2. Intentional test case: Perhaps this was meant to test error handling for broken permalinks?
  3. Missing feature: Should the permalink format support typed references like rendertest:confval:case-array?

If this was intentional as a "broken link" test case, please let me know and I'll adjust the fix accordingly (perhaps adding a comment explaining it's intentionally broken, or moving the test).

Test plan

  • Verify make test-rendertest passes
  • Verify the confval link renders correctly in generated HTML

The permalink URL `rendertest:confval-case-array` cannot resolve because:
- It creates a ReferenceNode with linkType='std:label' and anchor='confval-case-array'
- But the confval target is registered as linkType='std:confval' with anchor='case-array'
- The reference resolver looks in the wrong inventory section

Changed to use the proper `:confval:`case-array`` role which resolves correctly.

See PR for detailed technical analysis.
@CybotTM CybotTM marked this pull request as draft January 4, 2026 18:35
@CybotTM
Copy link
Contributor Author

CybotTM commented Jan 4, 2026

still investigating ...

@CybotTM
Copy link
Contributor Author

CybotTM commented Jan 4, 2026

Closing this PR - the original permalink format was correct and should work.

Root Cause Found

The actual bug was a priority ordering conflict in the feature branch's incremental rendering infrastructure:

Component Priority Function
CollectLinkTargetsTransformer 5000 Registers primary link targets
ExportsCollectorPass 4500 Collects internal targets for incremental cache
CollectPrefixLinkTargetsTransformer 4000 (was) Registers std:label entries for prefixed nodes like confval-*

Since higher priority runs first, ExportsCollectorPass was collecting targets before CollectPrefixLinkTargetsTransformer added the confval-* entries to std:label. This caused the permalink rendertest:confval-case-array to fail resolution.

Fix Applied

Changed CollectPrefixLinkTargetsTransformer.getPriority() from 4000 to 4900 in commit CybotTM@d4c3eeb9.

This ensures it runs before ExportsCollectorPass (4500) but after CollectLinkTargetsTransformer (5000).

Why Only Feature Branch?

ExportsCollectorPass is new incremental rendering infrastructure only present on the feature branch. Main doesn't have this component, so there was no priority conflict - explaining why main never showed this issue.

The original documentation was correct - no changes needed there.

@CybotTM CybotTM closed this Jan 4, 2026
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