[Documents] Existing Areablock editable data becomes disconnected after upgrading from Pimcore v12 to v2026.2 and saving a document - #3934
Draft
ValeriaMaltseva wants to merge 4 commits into
Conversation
…g child editables The areablock editable derived its value from the rendered editmode DOM and silently assigned new sequential keys to every area entry whose key attribute was missing. Saving such a document renumbered the areablock indices (e.g. content:12 -> content:9) while the child editables kept their stored names (content:12.*), permanently disconnecting them from their area entry. - transformValue now uses the backend-provided indices as the source of truth and only falls back to the DOM when no server value exists - AreablockManager now ignores area entries without a key attribute (classic editmode parity, same as BlockManager) instead of inventing keys for them Fixes pimcore/platform-version#248 Co-Authored-By: Claude <noreply@anthropic.com>
ValeriaMaltseva
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes areablock key corruption by prioritizing backend indices over incomplete editmode DOM markup.
Changes:
- Uses server-provided areablock values when available.
- Ignores unkeyed DOM entries instead of assigning keys.
- Adds regression tests for value transformation and key handling.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
dynamic-type-document-editable-areablock.tsx |
Prioritizes backend values. |
dynamic-type-document-editable-areablock.test.tsx |
Tests server and DOM value handling. |
areablock-manager.ts |
Filters unkeyed entries. |
areablock-manager.test.ts |
Tests key preservation and allocation. |
pimcore-deployments
marked this pull request as draft
July 24, 2026 11:54
Collaborator
|
🚫 CI & merge-readiness guardrail failed — this PR has been converted to draft. A PR can only be merged when it has no conflicts with the base branch and all CI checks pass.
When fixed, press Ready for review to re-run the checks. |
ValeriaMaltseva
marked this pull request as ready for review
July 24, 2026 11:57
…not the DOM Follow-up to the review on #3934: restricting queryElements() to keyed nodes also changed every edit operation, not just the DOM fallback. On upgraded markup without key attributes, add/paste/move/remove started from an empty DOM read, could reuse an existing server key, and saved without the backend-provided entries — disconnecting their children again. - getCurrentValue() uses the store value (backend indices) as the source of truth for operation payloads and key allocation; the DOM is only a fallback and is used for element lookup / node positioning - key allocation now considers both the value keys and the DOM keys, so a new entry can never collide with an existing server key - add/paste/move/remove/toggle build the new value by applying the operation to the authoritative array (pure helpers in areablock-utils), instead of re-reading the rendered DOM; handlePostOperation is gone - reload mode uses the same value math instead of a parallel DOM element list Covers add/paste/move/remove/reload against unkeyed wrappers plus the pure value helpers. Co-Authored-By: Claude <noreply@anthropic.com>
|
ValeriaMaltseva
marked this pull request as draft
July 24, 2026 13:26
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes pimcore/platform-version#248
Problem
The areablock editable derived its value from the rendered editmode DOM instead of the backend-provided indices.
AbstractBlockManager.ensureAllElementKeys()silently assigned new sequential keys to every.pimcore_area_entryelement whosekeyattribute was missing or unreadable (e.g. custom areabrick wrapper templates in projects upgraded from v12).Saving such a document — without touching anything — renumbered the areablock indices (e.g.
content:12→content:9) while the child editables kept their stored names (content:12.*), permanently disconnecting them from their area entry. The data remains indocuments_editablesunder the old names, so affected documents are recoverable once keys are remapped.The classic v12 editmode never had this problem: it selected only entries that already carry a key (
.pimcore_block_entry[data-name="…"][key]) and never invented keys. TheBlockManagerfor the plainblockeditable in this codebase already follows that pattern — the areablock manager was the outlier.Fix
transformValue()now uses the backend-provided indices as the source of truth and only falls back to the DOM read when no server value existsAreablockManager.getElementSelector()now requires thekeyattribute (classic parity, same asBlockManager), andgetAreablockValue()no longer invents keys viaensureAllElementKeys()— unmanaged entries are ignored instead of being renumberedNew keys are still only allocated by explicit user operations (
addArea,pasteArea) viacalculateNextKey().Tests
Regression tests written first (TDD): before the fix, the
transformValuetest reproduced the reported corruption exactly (server keys[5, 2, 12]came back as[1, 2, 3]). Full jest suite passes (219/219), ESLint clean.Forward-merge
Base is
2025.4(oldest affected maintained line) → forward-merge2025.4 → 2026.2 → 2026.x.🤖 Generated with Claude Code