Related lookup popup doesn't close after selecting an existing item

[Matthias Kestenholz]

​https://github.com/django/django/commit/91bebf1adb43561b54bac18e76224759dc70acb3 broke the functionality by accessing window.relatedWindows instead of the module-local relatedWindows variable; window.relatedWindows is only defined when running the tests but not when running in the browser.

This affects all projects using raw_id_fields to select existing objects.

The patch attached here fixes the problem and the unit test, but it's obviously really ugly.

The problem wasn't discovered by me but by ​https://github.com/yoshson/ . I'm reporting this here and now because of the expected release date of Django 5.2.

[Natalia Bidart]

I can reproduce, the bug is that once an item is selected in the raw_id_fields popup, the popup is not automatically closed. We need to investigate a solution, potentially a revert.

Marking as patch needs improvement because we need to find a cleaner solution.

[Matthias Kestenholz]

Thanks! Yep, I totally agree.

The RelatedObjectLookups.js exposes many functions directly on window and exposing the additional variable doesn't seem worse than what we have now; I borrowed the variable name from React to make it very obvious that this is a hack and I don't like it much.

I think the cleanest solution would be to rewrite the related object lookups test from QUnit to selenium and keeping relatedWindows as a module-level const variable without exposing it. We could avoid exposing relatedWindows so that QUnit can access it. This will need some work though!

[Natalia Bidart]

Matthias, if you are already able to run the JS tests (I'm setting my env up), could you verify if this solution fixes the issue? My point being: I know the django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js does fix this bug report, but I wonder if, without this change, the test properly fails if window.relatedWindows is not defined as it's now. Basically my goal is to ensure that the test does not rely on a fabricated window.relatedWindows. Does this make sense?

django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js

@@ -206 +206 @@
     window.dismissChangeRelatedObjectPopup = dismissChangeRelatedObjectPopup;
     window.dismissDeleteRelatedObjectPopup = dismissDeleteRelatedObjectPopup;
     window.dismissChildPopups = dismissChildPopups;
+    window.relatedWindows = relatedWindows
 
     // Kept for backward compatibility
     window.showAddAnotherPopup = showRelatedObjectPopup;

js_tests/admin/RelatedObjectLookups.test.js

@@ -8 +8 @@
             <input type="text" id="test_id" name="test" />
             <input type="text" id="many_test_id" name="many_test" class="vManyToManyRawIdAdminField" />
         `);
-        window.relatedWindows = window.relatedWindows || [];
     }
 });
 

[Matthias Kestenholz]

Yes, that does fix the issue both when running browser tests and also when using the raw_id_fields popup in the administration interface.

I also tested the following patch which replaces window.relatedWindows usage in the admin JavaScript with relatedWindows and it works as well and seems a tiny little bit cleaner:

diff --git i/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js w/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js
index 5395386087..0522ba60b5 100644
--- i/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js
+++ w/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js
@@ -58,9 +58,9 @@
             elem.value = chosenId;
         }
         $(elem).trigger('change');
-        const index = window.relatedWindows.indexOf(win);
+        const index = relatedWindows.indexOf(win);
         if (index > -1) {
-            window.relatedWindows.splice(index, 1);
+            relatedWindows.splice(index, 1);
         }
         win.close();
     }
@@ -206,6 +206,7 @@
     window.dismissChangeRelatedObjectPopup = dismissChangeRelatedObjectPopup;
     window.dismissDeleteRelatedObjectPopup = dismissDeleteRelatedObjectPopup;
     window.dismissChildPopups = dismissChildPopups;
+    window.relatedWindows = relatedWindows;
 
     // Kept for backward compatibility
     window.showAddAnotherPopup = showRelatedObjectPopup;
diff --git i/js_tests/admin/RelatedObjectLookups.test.js w/js_tests/admin/RelatedObjectLookups.test.js
index 722aa7ae7b..0d71d88f2a 100644
--- i/js_tests/admin/RelatedObjectLookups.test.js
+++ w/js_tests/admin/RelatedObjectLookups.test.js
@@ -8,7 +8,6 @@ QUnit.module('admin.RelatedObjectLookups', {
             <input type="text" id="test_id" name="test" />
             <input type="text" id="many_test_id" name="many_test" class="vManyToManyRawIdAdminField" />
         `);
-        window.relatedWindows = window.relatedWindows || [];
     }
 });
 

The additional hunks in dismissRelatedLookupPopup change the code to more closely resemble all other functions which also access relatedWindows directly and not through the window object.

[Natalia Bidart]

Thank you Matthias for your prompt and complete response! I will cleanup my patch and propose a PR, I will include you as co-author.

I used our GH actions to verify and I can also confirm that the proposed test change, without the corresponding code fix, fails as expected. From the javascript test run log:

2025-04-01T15:14:56.5596976Z admin.RelatedObjectLookups > dismissRelatedLookupPopup closes popup window...ERROR
2025-04-01T15:14:56.5598964Z >> Message: Died on test #1: Cannot read properties of undefined (reading 'indexOf')
2025-04-01T15:14:56.5600137Z >>     at file:///home/runner/work/django/django/js_tests/admin/RelatedObjectLookups.test.js:14:7
2025-04-01T15:14:56.5601118Z >> Actual: undefined
2025-04-01T15:14:56.5601369Z >> Expected: undefined
2025-04-01T15:14:56.5601767Z >> TypeError: Cannot read properties of undefined (reading 'indexOf')
2025-04-01T15:14:56.5602680Z >>     at dismissRelatedLookupPopup (file:///home/runner/work/django/django/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js:61:45)
2025-04-01T15:14:56.5604267Z >>     at Object.<anonymous> (file:///home/runner/work/django/django/js_tests/admin/RelatedObjectLookups.test.js:23:12)
2025-04-01T15:14:56.5659666Z admin.RelatedObjectLookups > dismissRelatedLookupPopup removes window from relatedWindows array...ERROR
2025-04-01T15:14:56.5660853Z >> Message: Died on test #1: Cannot read properties of undefined (reading 'push')
2025-04-01T15:14:56.5661535Z >>     at file:///home/runner/work/django/django/js_tests/admin/RelatedObjectLookups.test.js:27:7
2025-04-01T15:14:56.5661982Z >> Actual: undefined
2025-04-01T15:14:56.5662235Z >> Expected: undefined
2025-04-01T15:14:56.5662610Z >> TypeError: Cannot read properties of undefined (reading 'push')
2025-04-01T15:14:56.5663318Z >>     at Object.<anonymous> (file:///home/runner/work/django/django/js_tests/admin/RelatedObjectLookups.test.js:33:27)
2025-04-01T15:14:56.6129232Z admin.RelatedObjectLookups > dismissRelatedLookupPopup triggers change event for single value field...ERROR
2025-04-01T15:14:56.6130896Z >> Message: Died on test #2: Cannot read properties of undefined (reading 'indexOf')
2025-04-01T15:14:56.6133119Z >>     at file:///home/runner/work/django/django/js_tests/admin/RelatedObjectLookups.test.js:39:7
2025-04-01T15:14:56.6133918Z >> Actual: undefined
2025-04-01T15:14:56.6134960Z >> Expected: undefined
2025-04-01T15:14:56.6135655Z >> TypeError: Cannot read properties of undefined (reading 'indexOf')
2025-04-01T15:14:56.6137289Z >>     at dismissRelatedLookupPopup (file:///home/runner/work/django/django/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js:61:45)
2025-04-01T15:14:56.6139511Z >>     at Object.<anonymous> (file:///home/runner/work/django/django/js_tests/admin/RelatedObjectLookups.test.js:55:12)
2025-04-01T15:14:56.6149250Z admin.RelatedObjectLookups > dismissRelatedLookupPopup triggers change event for many-to-many field...ERROR
2025-04-01T15:14:56.6155074Z >> Message: Died on test #2: Cannot read properties of undefined (reading 'indexOf')
2025-04-01T15:14:56.6156351Z >>     at file:///home/runner/work/django/django/js_tests/admin/RelatedObjectLookups.test.js:59:7
2025-04-01T15:14:56.6157125Z >> Actual: undefined
2025-04-01T15:14:56.6157699Z >> Expected: undefined
2025-04-01T15:14:56.6158736Z >> TypeError: Cannot read properties of undefined (reading 'indexOf')
2025-04-01T15:14:56.6160612Z >>     at dismissRelatedLookupPopup (file:///home/runner/work/django/django/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js:61:45)
2025-04-01T15:14:56.6162516Z >>     at Object.<anonymous> (file:///home/runner/work/django/django/js_tests/admin/RelatedObjectLookups.test.js:75:12)

[Natalia Bidart]

​PR ready for review.

[GitHub <noreply@…>]

In a245604:

Fixed #36284, Refs #31170 -- Ensured related lookup popups are closed properly.

In the admin, when selecting related objects via the helpers defined in
RelatedObjectLookups.js, the dismissRelatedLookupPopup function was
attempting to access window.relatedWindows, which does not exist in
real execution, causing related lookup popups to remain open.

This change ensures that this code correctly accesses the module-local
relatedWindows by explicitly assigning it to window.relatedWindows.

Regression in 91bebf1adb43561b54bac18e76224759dc70acb3.

Thanks Matthias Kestenholz for the report, the fix ideas, and testing.

Co-authored-by: Matthias Kestenholz <mk@…>

[Natalia <124304+nessita@…>]

In 614be949:

[5.2.x] Fixed #36284, Refs #31170 -- Ensured related lookup popups are closed properly.

In the admin, when selecting related objects via the helpers defined in
RelatedObjectLookups.js, the dismissRelatedLookupPopup function was
attempting to access window.relatedWindows, which does not exist in
real execution, causing related lookup popups to remain open.

This change ensures that this code correctly accesses the module-local
relatedWindows by explicitly assigning it to window.relatedWindows.

Regression in 91bebf1adb43561b54bac18e76224759dc70acb3.

Thanks Matthias Kestenholz for the report, the fix ideas, and testing.

Co-authored-by: Matthias Kestenholz <mk@…>

Backport of a245604277eb9edeba234dacf199890766462709 from main.