Re: [Django] #36284: Related lookup popup doesn't close after selecting an existing item

[Django]

#36284: Related lookup popup doesn't close after selecting an existing item
--------------------------------------+------------------------------------
Reporter: Matthias Kestenholz | Owner: (none)
Type: Bug | Status: new
Component: contrib.admin | Version: 5.2
Severity: Release blocker | Resolution:
Keywords: RelatedObjectLookups | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by 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!
--
Ticket URL: <https://code.djangoproject.com/ticket/36284#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36284: Related lookup popup doesn't close after selecting an existing item
--------------------------------------+------------------------------------
Reporter: Matthias Kestenholz | Owner: (none)
Type: Bug | Status: new
Component: contrib.admin | Version: 5.2
Severity: Release blocker | Resolution:
Keywords: RelatedObjectLookups | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Comment (by 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?
{{{#!diff
diff --git
a/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js
b/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js
index 5395386087..d6ce297981 100644
--- a/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js
+++ b/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js
@@ -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 a/js_tests/admin/RelatedObjectLookups.test.js
b/js_tests/admin/RelatedObjectLookups.test.js
index 722aa7ae7b..0d71d88f2a 100644
--- a/js_tests/admin/RelatedObjectLookups.test.js
+++ b/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 || [];
}
});

}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36284#comment:4>

[Django]

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

-------------------------------------+-------------------------------------
Reporter: Matthias Kestenholz | Owner: Natalia
| Bidart
Type: Bug | Status: assigned

Component: contrib.admin | Version: 5.2
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Accepted
RelatedObjectLookups |

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Natalia Bidart):

* owner: (none) => Natalia Bidart
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36284#comment:5>

[Django]

#36284: Related lookup popup doesn't close after selecting an existing item
-------------------------------------+-------------------------------------
Reporter: Matthias Kestenholz | Owner: Natalia
| Bidart
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
RelatedObjectLookups |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/36284#comment:6>

[Django]

#36284: Related lookup popup doesn't close after selecting an existing item
-------------------------------------+-------------------------------------
Reporter: Matthias Kestenholz | Owner: Natalia
| Bidart
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
RelatedObjectLookups |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by 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)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36284#comment:7>

[Django]

#36284: Related lookup popup doesn't close after selecting an existing item
-------------------------------------+-------------------------------------
Reporter: Matthias Kestenholz | Owner: Natalia
| Bidart
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
RelatedObjectLookups |
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia Bidart):

* needs_better_patch: 1 => 0

Comment:

[https://github.com/django/django/pull/19328 PR] ready for review.
--
Ticket URL: <https://code.djangoproject.com/ticket/36284#comment:8>

[Django]

#36284: Related lookup popup doesn't close after selecting an existing item
-------------------------------------+-------------------------------------
Reporter: Matthias Kestenholz | Owner: Natalia
| Bidart

Type: Bug | Status: closed
Component: contrib.admin | Version: 5.2
Severity: Release blocker | Resolution: fixed

Keywords: | Triage Stage: Accepted
RelatedObjectLookups |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by GitHub <noreply@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"a245604277eb9edeba234dacf199890766462709" a245604]:
{{{#!CommitTicketReference repository=""
revision="a245604277eb9edeba234dacf199890766462709"
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 <m...@feinheit.ch>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36284#comment:9>

[Django]

#36284: Related lookup popup doesn't close after selecting an existing item
-------------------------------------+-------------------------------------
Reporter: Matthias Kestenholz | Owner: Natalia
| Bidart
Type: Bug | Status: closed
Component: contrib.admin | Version: 5.2
Severity: Release blocker | Resolution: fixed

Keywords: | Triage Stage: Ready for
RelatedObjectLookups | checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia Bidart):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/36284#comment:10>

[Django]

#36284: Related lookup popup doesn't close after selecting an existing item
-------------------------------------+-------------------------------------
Reporter: Matthias Kestenholz | Owner: Natalia
| Bidart
Type: Bug | Status: closed
Component: contrib.admin | Version: 5.2
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
RelatedObjectLookups | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Natalia <124304+nessita@…>):

In [changeset:"614be94957d6b17a8b3d051204e62559e1f089e6" 614be949]:
{{{#!CommitTicketReference repository=""
revision="614be94957d6b17a8b3d051204e62559e1f089e6"
[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 <m...@feinheit.ch>

Backport of a245604277eb9edeba234dacf199890766462709 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36284#comment:11>