Opened 18 minutes ago
#37088 new Bug
path based MediaAsset equality
| Reported by: | Johannes Maron | Owned by: | |
|---|---|---|---|
| Component: | Forms | Version: | 6.0 |
| Severity: | Normal | Keywords: | |
| Cc: | Johannes Maron | Triage Stage: | Unreviewed |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | yes | UI/UX: | no |
Description
This is a spin-off from here: https://github.com/django/django/pull/21239#issuecomment-4396168812
Currently MediaAsset.eq compares only the path.
This approach was deliberately chosen to benefit performance.
However, in the case of link-elements and even script this approach can break.
For a link, rel=stylesheet and rel=refretch can share the same path. Prefetching is also not too crazy a use case; someone will probably try to do it.
A compromise could be to define an explicit list of attributes to compare, instead of doing a full dictionary comparison via attributres.
Something like:
diff --git a/django/forms/widgets.py b/django/forms/widgets.py
index 6972155402..70561d2c8c 100644
--- a/django/forms/widgets.py
+++ b/django/forms/widgets.py
@@ -66,14 +66,20 @@ class MediaOrderConflictWarning(RuntimeWarning):
class MediaAsset:
element_template = "{path}"
+ # Only compare selected attributes to ensure performant comparison in Media.merge.
+ equality_attributes = ("path",)
+
def __init__(self, path, **attributes):
self._path = path
self.attributes = attributes
def __eq__(self, other):
- # Compare the path only, to ensure performant comparison in
- # Media.merge.
- return (self.__class__ is other.__class__ and self.path == other.path) or (
+ return (
+ self.__class__ is other.__class__ and self.path == other.path and all(
+ self.attributes.get(attr, None) == other.attributes.get(attr, None)
+ for attr in self.equality_attributes
+ )
+ ) or (
isinstance(other, str) and self._path == other
)