Needs review
Project:
Entity Browser
Version:
8.x-2.x-dev
Component:
Widget plugins
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
27 Jul 2022 at 08:14 UTC
Updated:
4 May 2026 at 14:49 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
connbi commentedchange patch file name.
Comment #3
randy tang commentedComment #4
randy tang commentedComment #5
keshavv commentedComment #6
randy tang commentedWhen the array_parents level does not have data after 3, there will be a warning
Comment #7
benstallings commentedClaude Code says:
Problems:
1. Code style throughout — Missing spaces after if/foreach (if(, foreach(), missing space after // in comment. Would fail phpcs.
2. Hardcoded array depth — $form[$array_parents[0]][$array_parents[1]] assumes the widget is always exactly 2 levels deep in the form. Entity reference fields nested inside paragraphs, inline entity forms, or layout builder will have different depths. This would either miss those cases or throw undefined index errors.
3. Unused variable — $key_value = '' is assigned and never used.
4. Overcomplicated traversal — The foreach ($array_parents as $k => $array_parent) loop manually walks the form array by reference. But $array_parents is already available — NestedArray::getValue($form, $array_parents) does the same thing in one clean call.
5. Unnecessary loop — The outer foreach iterates widget siblings but then checks in_array($key, $array_parents) — it's really just trying to find the triggering element's own target_id field. The loop is doing work that the $trigger['#array_parents'] already gives directly.
6. Wrong layer for deduplication — If duplicates are appearing in target_id, the root cause is in the selection/submission pipeline (the entity browser or the entities-selected JS event). Silently deduplicating in the AJAX callback masks the real bug and could hide other data integrity issues.
7. No test, no issue context in the code — The commit message says "3300147-5" but there's no explanation of what scenario produces duplicates or how to reproduce.
Verdict: This addresses a symptom, not a cause, and does so with fragile, style-noncompliant code. If duplicate entities are a real problem, the fix belongs where entities are added to the selection — not in the AJAX render callback.
Comment #9
benstallings commentedComment #10
anybodyCould we please have a test for these duplicates case to ensure it also works in the future (and proves was broken before)? Thanks!
Comment #11
benstallings commentedComment #12
benstallings commented