Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `cms/admin/pageadmin.py:274` </location>
<code_context>
+            form._request = request
+        else:
+            # In case it's not the advanced view, return an empty form
+            class Meta:
+                model = Page
+                fields = []
</code_context>

<issue_to_address>
**issue (complexity):** Consider refactoring by moving the EmptyPageForm class definition outside the get_form method and consolidating repeated permission checks into a helper function.

```python
# Move this out of get_form and only define it once per module
class EmptyPageForm(ModelForm):
    class Meta:
        model = Page
        fields = []

# -----------------------------
def get_form(self, request, obj=None, **kwargs):
    """Get PageForm or a no-op form if not in advanced settings."""
    if not getattr(request, '_advanced_settings', False):
        return EmptyPageForm
    FormClass = super().get_form(request, obj, **kwargs)
    FormClass._site = get_site(request)
    FormClass._request = request
    return FormClass

# -----------------------------
# Helper to collapse repeated permission checks
def _ensure_permission(self, request, obj, check_fn, message):
    if not check_fn(request, obj):
        raise PermissionDenied(message)

def change_permissions(self, request, page_id):
    page = self.get_object(request, object_id=page_id)
    if page is None:
        raise self._get_404_exception(page_id)

    # combine your two permission checks into one call
    self._ensure_permission(
        request,
        page,
        self.has_change_advanced_settings_permission,
        "No permission for editing advanced settings"
    )

    request._change_page_permissions = True
    show_permissions = (
        get_cms_setting('PERMISSION') and
        self.has_change_permissions_permission(request, obj=page)
    )
    context = {
        'title': _("Change Page Permissions"),
        'show_permissions': show_permissions,
    }
    return self.change_view(request, page_id, extra_context=context)
```
</issue_to_address>

Please address the comments from this code review:

## Overall Comments
- The logic that combines advanced‐settings and permissions checks is duplicated across the admin view, toolbar, and tree rendering—consider extracting it into a single helper to keep behaviors consistent.
- The dynamic EmptyForm creation in get_form is clever but a bit opaque—extract it into a well-named private method or inner class to make the fallback intent clearer.

## Individual Comments

### Comment 1
<location> `cms/admin/pageadmin.py:307-315` </location>
<code_context>
             show_permissions = False
-        context = {"title": _("Advanced Settings"), "show_permissions": show_permissions}
+
+        show_advanced_settings = self.has_change_advanced_settings_permission(request, obj=page)
+
+        if not show_permissions and not show_advanced_settings:
+            raise PermissionDenied("No permission for editing advanced settings")
+
</code_context>

<issue_to_address>
**🚨 suggestion (security):** Permission check order may leak information about permissions.

Check if the page exists before performing permission checks to avoid revealing permission information for non-existent pages.

```suggestion
        if not page:
            from django.http import Http404
            raise Http404("Page does not exist")

        if get_cms_setting("PERMISSION"):
            show_permissions = self.has_change_permissions_permission(request, obj=page)
        else:
            show_permissions = False

        show_advanced_settings = self.has_change_advanced_settings_permission(request, obj=page)

        if not show_permissions and not show_advanced_settings:
            raise PermissionDenied("No permission for editing advanced settings")
```
</issue_to_address>

### Comment 2
<location> `cms/admin/pageadmin.py:1389-1390` </location>
<code_context>
                 "has_add_page_permission": user_can_add(user, target=page),
                 "has_change_permission": user_can_change(request.user, page, site),
-                "has_change_advanced_settings_permission": user_can_change_advanced(request.user, page, site),
+                "has_change_advanced_settings_permission": (
+                    user_can_change_advanced(request.user, page, site) or user_can_change_permissions(request.user, page, site)
+                ),
                 "has_move_page_permission": has_move_page_permission,
</code_context>

<issue_to_address>
**🚨 question (security):** Combining advanced settings and permissions checks may blur permission boundaries.

Please verify that expanding access via either advanced settings or permissions checks does not unintentionally allow users elevated privileges beyond your intended model.
</issue_to_address>

### Comment 3
<location> `cms/static/cms/js/modules/cms.modal.js:909-910` </location>
<code_context>
             // If the response contains the close-frame (and potentially the data bridge),
             // the form was saved successfully
-            that.saved = that.saved || body.hasClass('cms-close-frame');
+            that.saved = that.saved || body.hasClass('cms-close-frame')
+                || body.find('script#django-admin-popup-response-constants').length > 0;

             // tabindex is required for keyboard navigation
</code_context>

<issue_to_address>
**suggestion:** Checking for script tag may be fragile if admin markup changes.

Using a specific script tag as a success indicator may break if Django admin markup changes. Please evaluate if a more stable method exists.

Suggested implementation:

```javascript
            that.saved = that.saved || body.hasClass('cms-close-frame')
                || body.find('.messagelist :not(.error)').length > 0;

```

```javascript
            var saveSuccess = Boolean(contents.find('.messagelist :not(.error)').length) ||
                body.find('.messagelist :not(.error)').length > 0;

```
</issue_to_address>

### Comment 4
<location> `cms/static/cms/js/modules/cms.modal.js:944-945` </location>
<code_context>
             }

-            var saveSuccess = Boolean(contents.find('.messagelist :not(".error")').length);
+            var saveSuccess = Boolean(contents.find('.messagelist :not(".error")').length) ||
+                body.find('script#django-admin-popup-response-constants').length > 0;

             // in case message didn't appear, assume that admin page is actually a success
</code_context>

<issue_to_address>
**suggestion:** Duplicated logic for save success could be refactored.

Refactor the save success logic into a shared helper to avoid duplication and improve maintainability.

Suggested implementation:

```javascript
            function isSaveSuccess(contents, body) {
                return Boolean(contents.find('.messagelist :not(".error")').length) ||
                    body.find('script#django-admin-popup-response-constants').length > 0;
            }

            that.saved = that.saved || body.hasClass('cms-close-frame')
                || isSaveSuccess(contents, body);

```

```javascript
            var saveSuccess = isSaveSuccess(contents, body);

```
</issue_to_address>

### Comment 5
<location> `cms/tests/test_admin.py:873-882` </location>
<code_context>
+    @override_settings(CMS_PERMISSION=True)
</code_context>

<issue_to_address>
**suggestion (testing):** Consider adding tests for users with only permission to change permissions (not advanced settings).

Please add a test for users with only change permissions to verify they can access the advanced settings endpoint as intended.
</issue_to_address>

### Comment 6
<location> `cms/static/cms/js/modules/cms.modal.js:944-945` </location>
<code_context>

</code_context>

<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))

<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.

From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>