Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] slug always rewritten on any change of page if it contains the path segments #98

Open
dkd-kaehm opened this issue Oct 30, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@dkd-kaehm
Copy link

dkd-kaehm commented Oct 30, 2023

Each editors change on page record leads to slug update, if it contains some path segments from admin.

How to reproduce:

  1. Apply provided EXT:sluggi settings
  2. As admin: prepend slug with some path like /virtual-parent-slug/... so the slug is /virtual-parent-slug/my-test-page
  3. As editor: Do any change on that page

Current behavior:

  • The slug is updated to /my-test-page, if not changed.
  • NOT ONLY slugs "last segment only" is updated, if changed. But the whole slug is overriden.

Expected behavior:

  • The slug IS NOT updated at all, if not changed.
  • ONLY slugs "last segment only" is updated, if changed.

Additional infos:

EXT:sluggi version: 11.1.2 @ TYPO3 11.5.32

It does not matter if the slug is locked or not, but it is really critical on locked slugs, because it breaks in our case the SEO score and back links and so on.

The source of trouble:

The value is only last path segment of real slug:

<div class="input-group">
  <span class="input-group-addon">https://example.com/virtual-parent-slug</span>
  <input class="form-control t3js-form-field-slug-readonly" data-bs-toggle="tooltip" value="/my-test-page" readonly="" aria-label="/wir-ueber-uns">
  <input type="text" ... data-tx-sluggi-sync="0" data-tx-sluggi-lock="1" placeholder="/my-test-page" ... ...>
  ...
  <input type="hidden" class="t3js-form-field-slug-hidden" name="data[pages][100][slug]" value="/my-test-page">
</div>

EXT:sluggi settings:

'sluggi' => [
    'disable_slug_update_information' => '0',
    'exclude_page_types' => '199,255,254',
    'last_segment_only' => '1',
    'pages_fields' => '[["title"]]',
    'slash_replacement' => '1',
    'synchronize' => '0',
    'whitelist' => '',
],

Proposal:

It is really difficult for a set of possibilities, but something like this:

  • Enrich the BE rendered markup and or TCA with "banned slug part"(secured with salted) and disallow to edit it on JS stack.
  • Do not override the "value" with $segment in InputSlugElement::replaceValues(), but only on JS stack and only on Non-"banned slug part"
  • Check on server:
    • if "banned slug part"(secured with salted) is not manipulated,
    • if slug was changed reasonable and aware on "banned slug part"
    • etc.
    • if all requirements are met, leave the "slug" field in TCA fields on command
    • if one of requirements is not met, react...
  • etc.

Current solution in our project:

Following patch at least helps against losing the locked slug:

Subject: [PATCH] [BUGFIX] Avoid unacceptable slug changes on locked slugs

EXT:sluggi always(on each pages change) rewrites the slug with path segments to the last path segment, 
even the slug is locked and not changed by editor.
Also from `/virtual-parent-slug/my-test-page` to `/my-test-page`.
This change prevents overrides on slugs at least on "locked" slugs.

**Note: This is a partial fix only and not locked slugs will be still overridden.**

Relates: #98
---
Index: Classes/DataHandler/HandlePageUpdate.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/Classes/DataHandler/HandlePageUpdate.php b/Classes/DataHandler/HandlePageUpdate.php
--- a/Classes/DataHandler/HandlePageUpdate.php	(revision b2c353eefafd944be268bef3420cc594872d9004)
+++ b/Classes/DataHandler/HandlePageUpdate.php	(date 1698709057070)
@@ -66,7 +66,7 @@

         if ($this->shouldSynchronize($pageRecord, $fields)) {
             $fields = $this->synchronize($pageRecord, $fields);
-        } elseif ($this->isManualUpdateWithOnlyLastSegmentAllowed($fields)) {
+        } elseif ($this->isManualUpdateWithOnlyLastSegmentAllowed($fields) && !($pageRecord['slug_locked'])) {
             $fields = $this->updateLastSegment($pageRecord, $fields);
         }
     }
Index: Classes/Backend/Form/InputSlugElement.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/Classes/Backend/Form/InputSlugElement.php b/Classes/Backend/Form/InputSlugElement.php
--- a/Classes/Backend/Form/InputSlugElement.php	(revision b2c353eefafd944be268bef3420cc594872d9004)
+++ b/Classes/Backend/Form/InputSlugElement.php	(date 1698708196083)
@@ -73,7 +73,7 @@
         if (!empty($inaccessibleSlugSegments) && 0 === strncmp($editableSlugSegments, $inaccessibleSlugSegments, strlen($inaccessibleSlugSegments))) {
             $editableSlugSegments = substr($editableSlugSegments, strlen($inaccessibleSlugSegments));
         }
-        if ($allowOnlyLastSegment && !empty($editableSlugSegments)) {
+        if ($allowOnlyLastSegment && !($this->data['databaseRow']['slug_locked']) && !empty($editableSlugSegments)) {
             $segments = explode('/', $editableSlugSegments);
             $editableSlugSegments = '/' . array_pop($segments);
             $prefix .= implode('/', $segments);
dkd-kaehm added a commit to dkd-kaehm/sluggi that referenced this issue Oct 30, 2023
EXT:sluggi always(on each pages change) rewrites the slug with path segments to the last path segment,
even the slug is locked and not changed by editor.
Also from `/virtual-parent-slug/my-test-page` to `/my-test-page`.
This change prevents overrides on slugs at least on "locked" slugs.

**Note: This is a partial fix only and not locked slugs will be still overridden.**

Relates: wazum#98
dkd-kaehm added a commit to dkd-kaehm/sluggi that referenced this issue Oct 30, 2023
EXT:sluggi always(on each pages change) rewrites the slug with path segments to the last path segment,
even the slug is locked and not changed by editor.
Also from `/virtual-parent-slug/my-test-page` to `/my-test-page`.
This change prevents overrides on slugs at least on "locked" slugs.

**Note: This is a partial fix only and not locked slugs will be still overridden.**

Relates: wazum#98
dkd-kaehm added a commit to dkd-kaehm/sluggi that referenced this issue Oct 30, 2023
EXT:sluggi always(on each pages change) rewrites the slug with path segments to the last path segment,
even the slug is locked and not changed by editor.
Also from `/virtual-parent-slug/my-test-page` to `/my-test-page`.
This change prevents overrides on slugs at least on "locked" slugs.

**Note: This is a partial fix only and not locked slugs will be still overridden.**

Relates: wazum#98
Ports: wazum#99
@wazum wazum added the bug Something isn't working label Nov 9, 2023
@ahlswedem
Copy link

We can confirm this problem for EXT:sluggi version: 10.1.0 @ TYPO3 10.4.40.

@wazum
Copy link
Owner

wazum commented Dec 13, 2023

Thanks for the explanation, I'll work on this in the coming days.

@wazum
Copy link
Owner

wazum commented Dec 14, 2023

@dkd-kaehm @ahlswedem can you please test with the latest versions of the matching branch (no tags yet)? I hope I fixed all problems with this.
"wazum/sluggi": "dev-main"
"wazum/sluggi": "v11.x-dev"
"wazum/sluggi": "v10.x-dev"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants