Merge fbb4882bb3 into bf3f87b759

The bulk rich text link rewriter logic introduced in PR 5875 doesn't
work properly if the rich text contains multiple link types where one
of those types doesn't need to be rewritten (for example external
links like <a href="https://wagtail.org/">).

This commit fixes that bug by handling these cases, and adds additional
unit tests to cover the failure cases.

Fixes issue 11957.

CHANGELOG.txt

@@ -11,11 +11,22 @@ Changelog
  * Fix: Enable `richtext` template tag to convert lazy translation values (Benjamin Bach)
  * Fix: Ensure permission labels on group permissions page are translated where available (Matt Westcott)
  * Fix: Preserve whitespace in comment replies (Elhussein Almasri)
+ * Fix: Address layout issues in the title cell of universal listings (Sage Abdullah)
  * Docs: Remove duplicate section on frontend caching proxies from performance page (Jake Howard)
  * Docs: Document `restriction_type` field on PageViewRestriction (Shlomo Markowitz)
  * Docs: Document Wagtail's bug bounty policy (Jake Howard)
  * Maintenance: Use `DjangoJSONEncoder` instead of custom `LazyStringEncoder` to serialize Draftail config (Sage Abdullah)
  * Maintenance: Refactor image chooser pagination to check `WAGTAILIMAGES_CHOOSER_PAGE_SIZE` at runtime (Matt Westcott)
+ * Maintenance: Exclude the `client/scss` directory in Tailwind content config to speed up CSS compilation (Sage Abdullah)
+
+
+6.1.1 (xx.xx.xxxx) - IN DEVELOPMENT
+~~~~~~~~~~~~~~~~~~
+
+ * Fix: Fix form action URL in user edit and delete views for custom user models (Sage Abdullah)
+ * Fix: Fix snippet copy view not prefilling form data (Sage Abdullah)
+ * Fix: Address layout issues in the title cell of universal listings (Sage Abdullah)
+ * Fix: Fix incorrect rich text to HTML conversion when multiple link / embed types are present (Andy Chosak, Matt Westcott)
 
 
 6.1 (01.05.2024)
@@ -123,6 +134,12 @@ Changelog
  * Maintenance: Refactor the Django port of `urlify` to use TypeScript, officially deprecate `window.URLify` global util (LB (Ben) Johnston)
 
 
+6.0.4 (xx.xx.xxxx) - IN DEVELOPMENT
+~~~~~~~~~~~~~~~~~~
+
+ * Fix: Fix snippet copy view not prefilling form data (Sage Abdullah)
+
+
 6.0.3 (01.05.2024)
 ~~~~~~~~~~~~~~~~~~
 

client/scss/components/_listing.scss

@@ -223,19 +223,13 @@ ul.listing {
 
   .title {
     word-break: break-word;
-    display: flex;
-    align-items: center;
-    justify-content: space-between;
-    gap: theme('spacing.2');
 
     .title-wrapper,
     h2 {
       @apply w-label-1;
-      display: inline-flex;
-      gap: theme('spacing.2');
+      display: inline;
       margin: 0;
       vertical-align: middle;
-      align-items: center;
 
       a {
         color: inherit;
@@ -247,6 +241,11 @@ ul.listing {
         }
       }
     }
+
+    .icon-folder {
+      margin: 3px 0.3em 0 0;
+      vertical-align: top;
+    }
   }
 
   .actions {

docs/releases/6.0.4.md

@@ -0,0 +1,16 @@
+# Wagtail 6.0.4 release notes - IN DEVELOPMENT
+
+_Unreleased_
+
+```{contents}
+---
+local:
+depth: 1
+---
+```
+
+## What's new
+
+### Bug fixes
+
+ * Fix snippet copy view not prefilling form data (Sage Abdullah)

docs/releases/6.1.1.md

@@ -0,0 +1,19 @@
+# Wagtail 6.1.1 release notes - IN DEVELOPMENT
+
+_Unreleased_
+
+```{contents}
+---
+local:
+depth: 1
+---
+```
+
+## What's new
+
+### Bug fixes
+
+ * Fix form action URL in user edit and delete views for custom user models (Sage Abdullah)
+ * Fix snippet copy view not prefilling form data (Sage Abdullah)
+ * Address layout issues in the title cell of universal listings (Sage Abdullah)
+ * Fix incorrect rich text to HTML conversion when multiple link / embed types are present (Andy Chosak, Matt Westcott)

docs/releases/6.2.md

@@ -24,6 +24,7 @@ depth: 1
  * Enable `richtext` template tag to convert lazy translation values (Benjamin Bach)
  * Ensure permission labels on group permissions page are translated where available (Matt Westcott)
  * Preserve whitespace in comment replies (Elhussein Almasri)
+ * Address layout issues in the title cell of universal listings (Sage Abdullah)
 
 
 ### Documentation
@@ -37,6 +38,7 @@ depth: 1
 
  * Use `DjangoJSONEncoder` instead of custom `LazyStringEncoder` to serialize Draftail config (Sage Abdullah)
  * Refactor image chooser pagination to check `WAGTAILIMAGES_CHOOSER_PAGE_SIZE` at runtime (Matt Westcott)
+ * Exclude the `client/scss` directory in Tailwind content config to speed up CSS compilation (Sage Abdullah)
 
 
 ## Upgrade considerations - changes affecting all projects

docs/releases/index.rst

@@ -6,7 +6,9 @@ Release notes
 
    upgrading
    6.2
+   6.1.1
    6.1
+   6.0.4
    6.0.3
    6.0.2
    6.0.1

tailwind.config.js

@@ -8,7 +8,12 @@ module.exports = {
   content: [
     './wagtail/**/*.{py,html,ts,tsx}',
     './wagtail/**/static_src/**/*.js',
-    './client/**/*.{js,ts,tsx,mdx}',
+    // Make sure NOT to include the `client/scss` directory,
+    // even if we don't specify `*.scss` files here.
+    // The directory would still be scanned for files, which would cause
+    // the styles to rebuild in a loop.
+    // https://tailwindcss.com/docs/content-configuration#styles-rebuild-in-an-infinite-loop
+    './client/src/**/*.{js,ts,tsx,mdx}',
     './docs/**/*.{md,rst}',
   ],
   corePlugins: {

wagtail/admin/templates/wagtailadmin/pages/listing/_page_title_explore.html

@@ -5,7 +5,7 @@
 <div class="title-wrapper">
     {% if page.is_site_root %}
         {% if perms.wagtailcore.add_site or perms.wagtailcore.change_site or perms.wagtailcore.delete_site %}
-            <a href="{% url 'wagtailsites:index' %}" title="{% trans 'Sites menu' %}" class="w-flex w-items-center">{% icon name="site" classname="initial" %}</a>
+            <a href="{% url 'wagtailsites:index' %}" title="{% trans 'Sites menu' %}">{% icon name="site" classname="initial" %}</a>
         {% endif %}
     {% endif %}
 
@@ -17,7 +17,7 @@
         without also reading out the buttons and indicators.
     {% endcomment %}
     {% fragment as page_title %}
-        <span id="page_{{ page.pk|unlocalize|admin_urlquote }}_title" class="w-flex w-items-center w-gap-2">
+        <span id="page_{{ page.pk|unlocalize|admin_urlquote }}_title">
             {% if not page.is_site_root and not page.is_leaf %}{% icon name="folder" classname="initial" %}{% endif %}
             {{ page.get_admin_display_title }}
         </span>

wagtail/admin/views/generic/__init__.py

@@ -15,6 +15,7 @@ from .mixins import (  # noqa: F401
 )
 from .models import (  # noqa: F401
     CopyView,
+    CopyViewMixin,
     CreateView,
     DeleteView,
     EditView,

wagtail/admin/views/generic/models.py

@@ -688,12 +688,18 @@ class CreateView(
         return super().form_invalid(form)
 
 
-class CopyView(CreateView):
+class CopyViewMixin:
     def get_object(self, queryset=None):
-        return get_object_or_404(self.model, pk=unquote(str(self.kwargs["pk"])))
+        return get_object_or_404(
+            self.model, pk=unquote(str(self.kwargs[self.pk_url_kwarg]))
+        )
 
-    def get_form_kwargs(self):
-        return {**super().get_form_kwargs(), "instance": self.get_object()}
+    def get_initial_form_instance(self):
+        return self.get_object()
+
+
+class CopyView(CopyViewMixin, CreateView):
+    pass
 
 
 class EditView(

wagtail/models/__init__.py

@@ -3226,7 +3226,7 @@ class PagePermissionTester:
     def can_reorder_children(self):
         """
         Reorder permission checking is similar to publishing a subpage, since it immediately
-        affects published pages. However, the it shouldn't care about the 'creatability' of
+        affects published pages. However, it shouldn't care about the 'creatability' of
         page types, because the action only ever updates pages.
         """
         if not self.user.is_active:

wagtail/rich_text/rewriters.py

@@ -4,8 +4,9 @@ Utility classes for rewriting elements of HTML-like strings
 
 import re
 from collections import defaultdict
-from itertools import chain
-from typing import Callable, Tuple
+from typing import Callable, Dict, List
+
+from django.utils.functional import cached_property
 
 FIND_A_TAG = re.compile(r"<a(\b[^>]*)>")
 FIND_EMBED_TAG = re.compile(r"<embed(\b[^>]*)/>")
@@ -28,6 +29,26 @@ def extract_attrs(attr_string: str) -> dict:
     return attributes
 
 
+class TagMatch:
+    """Represents a single matched tag in a rich text string"""
+
+    def __init__(self, match):
+        self.match = match  # a regexp match object
+        self.replacement = None  # to be filled in by the rewriter
+
+    @cached_property
+    def attrs(self):
+        return extract_attrs(self.match.group(1))
+
+    @property
+    def start(self):
+        return self.match.start()
+
+    @property
+    def end(self):
+        return self.match.end()
+
+
 class TagRewriter:
     def __init__(self, rules=None, bulk_rules=None, reference_extractors=None):
         self.rules = rules or {}
@@ -38,54 +59,65 @@ class TagRewriter:
         raise NotImplementedError
 
     def get_tag_type_from_attrs(self, attrs):
+        """Given a dict of attributes from a tag, return the tag type."""
         raise NotImplementedError
 
     def get_tag_replacements(self, tag_type, attrs_list):
-        # Note: return an empty list for cases when you don't want any replacements made
+        """Given a list of attribute dicts, all taken from tags of the same type, return a
+        corresponding list of replacement strings to replace the tags with.
+
+        Return an empty list for cases when you don't want any replacements made.
+        """
         raise NotImplementedError
 
     def __call__(self, html: str) -> str:
-        matches_by_tag_type, attrs_by_tag_type = self.extract_tags(html)
+        matches_by_tag_type = self.extract_tags(html)
+        matches_to_replace = []
 
-        replacements = [
-            self.get_tag_replacements(tag_type, attrs_list)
-            for tag_type, attrs_list in attrs_by_tag_type.items()
-        ]
+        # For each tag type, get the list of replacement strings for all tags of that type
+        for tag_type, tag_matches in matches_by_tag_type.items():
+            attr_dicts = [match.attrs for match in tag_matches]
+            replacements = self.get_tag_replacements(tag_type, attr_dicts)
+
+            if not replacements:
+                continue
+
+            for match, replacement in zip(tag_matches, replacements):
+                match.replacement = replacement
+                matches_to_replace.append(match)
+
+        # Replace the tags in order of appearance in the string, so that offsets remain valid
+        matches_to_replace.sort(key=lambda match: match.start)
 
         offset = 0
-        for match, replacement in zip(
-            chain(*matches_by_tag_type.values()), chain(*replacements)
-        ):
+        for match in matches_to_replace:
             html = (
-                html[: match.start() + offset]
-                + replacement
-                + html[match.end() + offset :]
+                html[: match.start + offset]
+                + match.replacement
+                + html[match.end + offset :]
             )
 
-            offset += len(replacement) - match.end() + match.start()
+            offset += len(match.replacement) - match.end + match.start
 
         return html
 
-    def extract_tags(self, html: str) -> Tuple[dict, dict]:
+    def extract_tags(self, html: str) -> Dict[str, List[TagMatch]]:
         """Helper method to extract and group HTML tags and their attributes.
 
-        Returns the full list of regex matches grouped by tag type as well as
-        the tag attribute dictionaries grouped by tag type.
+        Returns a dict of TagMatch objects, mapping tag types to a list of all TagMatch objects of that tag type.
         """
         matches_by_tag_type = defaultdict(list)
-        attrs_by_tag_type = defaultdict(list)
 
         # Regex used to match <tag ...> tags in the HTML.
         re_pattern = self.get_opening_tag_regex()
 
-        for match in re_pattern.finditer(html):
-            attrs = extract_attrs(match.group(1))
-            tag_type = self.get_tag_type_from_attrs(attrs)
+        for re_match in re_pattern.finditer(html):
+            tag_match = TagMatch(re_match)
+            tag_type = self.get_tag_type_from_attrs(tag_match.attrs)
 
-            matches_by_tag_type[tag_type].append(match)
-            attrs_by_tag_type[tag_type].append(attrs)
+            matches_by_tag_type[tag_type].append(tag_match)
 
-        return matches_by_tag_type, attrs_by_tag_type
+        return matches_by_tag_type
 
     def convert_rule_to_bulk_rule(self, rule: Callable) -> Callable:
         def bulk_rule(args):

wagtail/snippets/tests/test_snippets.py

@@ -35,7 +35,6 @@ from wagtail.snippets.action_menu import (
 )
 from wagtail.snippets.blocks import SnippetChooserBlock
 from wagtail.snippets.models import SNIPPET_MODELS, register_snippet
-from wagtail.snippets.views.snippets import CopyView
 from wagtail.snippets.widgets import (
     AdminSnippetChooser,
     SnippetChooserAdapter,
@@ -974,20 +973,29 @@ class TestSnippetCopyView(WagtailTestUtils, TestCase):
             StandardSnippet.snippet_viewset.get_url_name("copy"),
             args=(self.snippet.pk,),
         )
-        self.login()
+        self.user = self.login()
 
-    def test_simple(self):
+    def test_without_permission(self):
+        self.user.is_superuser = False
+        self.user.save()
+        admin_permission = Permission.objects.get(
+            content_type__app_label="wagtailadmin", codename="access_admin"
+        )
+        self.user.user_permissions.add(admin_permission)
+
+        response = self.client.get(self.url)
+        self.assertEqual(response.status_code, 302)
+        self.assertRedirects(response, reverse("wagtailadmin_home"))
+
+    def test_form_is_prefilled(self):
         response = self.client.get(self.url)
         self.assertEqual(response.status_code, 200)
         self.assertTemplateUsed(response, "wagtailsnippets/snippets/create.html")
 
-    def test_form_prefilled(self):
-        request = RequestFactory().get(self.url)
-        view = CopyView()
-        view.model = StandardSnippet
-        view.setup(request, pk=self.snippet.pk)
-
-        self.assertEqual(view._get_initial_form_instance(), self.snippet)
+        # Ensure form is prefilled
+        soup = self.get_soup(response.content)
+        text_input = soup.select_one('input[name="text"]')
+        self.assertEqual(text_input.attrs.get("value"), "Test snippet")
 
 
 @override_settings(WAGTAIL_I18N_ENABLED=True)

wagtail/snippets/views/snippets.py

@@ -5,7 +5,7 @@ from django.contrib.admin.utils import quote
 from django.core import checks
 from django.core.exceptions import ImproperlyConfigured, PermissionDenied
 from django.http import Http404
-from django.shortcuts import get_object_or_404, redirect
+from django.shortcuts import redirect
 from django.urls import path, re_path, reverse, reverse_lazy
 from django.utils.functional import cached_property
 from django.utils.text import capfirst
@@ -262,16 +262,8 @@ class CreateView(generic.CreateEditViewOptionalFeaturesMixin, generic.CreateView
         return context
 
 
-class CopyView(CreateView):
-    def get_object(self):
-        return get_object_or_404(self.model, pk=self.kwargs["pk"])
-
-    def _get_initial_form_instance(self):
-        instance = self.get_object()
-        # Set locale of the new instance
-        if self.locale:
-            instance.locale = self.locale
-        return instance
+class CopyView(generic.CopyViewMixin, CreateView):
+    pass
 
 
 class EditView(generic.CreateEditViewOptionalFeaturesMixin, generic.EditView):

wagtail/tests/test_rich_text.py

@@ -191,6 +191,28 @@ This is another image: <embed embedtype="image" id="2" format="left" />
         """
             )
 
+    def test_expand_db_html_mixed_link_types(self):
+        self.assertEqual(
+            expand_db_html(
+                '<a href="https://wagtail.org/">foo</a>'
+                '<a linktype="page" id="3">bar</a>'
+            ),
+            '<a href="https://wagtail.org/">foo</a><a href="/events/">bar</a>',
+        )
+
+        self.assertEqual(
+            expand_db_html(
+                '<a linktype="page" id="3">page</a>'
+                '<a linktype="document" id="1">document</a>'
+                '<a linktype="page" id="3">page</a>'
+            ),
+            (
+                '<a href="/events/">page</a>'
+                '<a href="/documents/1/test.pdf">document</a>'
+                '<a href="/events/">page</a>'
+            ),
+        )
+
 
 class TestRichTextValue(TestCase):
     fixtures = ["test.json"]
@@ -274,6 +296,12 @@ class TestLinkRewriterTagReplacing(TestCase):
         self.assertNotEqual(link_with_custom_linktype, '<a href="https://wagtail.org">')
         self.assertEqual(link_with_custom_linktype, "<a>")
 
+        # And should properly handle mixed linktypes.
+        self.assertEqual(
+            rewriter('<a href="https://wagtail.org/"><a linktype="page" id="3">'),
+            '<a href="https://wagtail.org/"><a href="/article/3">',
+        )
+
     def test_supported_type_should_follow_given_rules(self):
         # we always have `page` rules by default
         rules = {

wagtail/users/templates/wagtailusers/users/user_cell.html

@@ -2,7 +2,7 @@
 {% load wagtailadmin_tags %}
 
 {% block title %}
-    <div class="w-flex w-items-center">
+    <div class="w-inline-flex w-items-center">
         {% avatar user=instance size="small" classname="w-shrink-0" %}
         {{ block.super }}
     </div>

wagtail/users/tests/test_admin_views.py

@@ -782,6 +782,12 @@ class TestUserDeleteView(AdminTemplateTestUtils, WagtailTestUtils, TestCase):
         self.assertTemplateUsed(response, "wagtailusers/users/confirm_delete.html")
         self.assertBreadcrumbsNotRendered(response.content)
 
+        # Should render the form with the correct action URL
+        soup = self.get_soup(response.content)
+        delete_url = reverse("wagtailusers_users:delete", args=(self.test_user.pk,))
+        form_action = soup.select_one("form").attrs["action"]
+        self.assertEqual(form_action, delete_url)
+
     def test_delete(self):
         response = self.post(follow=True)
 
@@ -990,9 +996,13 @@ class TestUserEditView(AdminTemplateTestUtils, WagtailTestUtils, TestCase):
         history_link = header.find("a", attrs={"href": history_url})
         self.assertIsNotNone(history_link)
 
+        # Should render the form with the correct action URL
+        edit_url = reverse("wagtailusers_users:edit", args=(self.test_user.pk,))
+        form_action = soup.select_one("form").attrs["action"]
+        self.assertEqual(form_action, edit_url)
+
         url_finder = AdminURLFinder(self.current_user)
-        expected_url = f"/admin/users/edit/{self.test_user.pk}/"
-        self.assertEqual(url_finder.get_edit_url(self.test_user), expected_url)
+        self.assertEqual(url_finder.get_edit_url(self.test_user), edit_url)
 
     def test_legacy_url_redirect(self):
         with self.assertWarnsMessage(

wagtail/users/views/users.py

@@ -286,6 +286,7 @@ class Edit(EditView):
 
     success_message = gettext_lazy("User '%(object)s' updated.")
     error_message = gettext_lazy("The user could not be saved due to errors.")
+    context_object_name = "user"
 
     def setup(self, request, *args, **kwargs):
         super().setup(request, *args, **kwargs)
@@ -339,6 +340,7 @@ class Delete(DeleteView):
 
     page_title = gettext_lazy("Delete user")
     success_message = gettext_lazy("User '%(object)s' deleted.")
+    context_object_name = "user"
 
     def dispatch(self, request, *args, **kwargs):
         self.object = self.get_object()