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

Using setURLSuffix on URLSegment field is not respected in SiteTreeURLSegmentField.js correctly #2723

Open
RVXD opened this issue Mar 4, 2022 · 7 comments

Comments

@RVXD
Copy link

RVXD commented Mar 4, 2022

I'd like to be able to remove the ?stage=Stage link from the URLSegment field in the CMS. Some clients keep copying this link into their newsletters/socialmedia messages. If people click on it these links they need to login. For most non-technical users this is confusing.

Removing the urlSuffix in Page.php by setting it to false works in source, but javascript adds it again. Resulting in link with undefined at the end.

Setting URLSuffix to false, if page is published:

public function getCMSFields()
{
      $fields = parent::getCMSFields();
        if( $this->isPublished() ) {
            $fields->FieldByName('Root.Main.URLSegment')->setURLSuffix(false);
        }
}

Workaround setting URLSuffix to '?', if page is published:
Now all the links will end with a '?'

public function getCMSFields()
{
      $fields = parent::getCMSFields();
        if( $this->isPublished() ) {
            $fields->FieldByName('Root.Main.URLSegment')->setURLSuffix('?');
        }
}

Current SiteTreeURLSegmentField.js
Always adds suffix. If it is empty or false causing 'undefined' in the URL.

// Transfer current value to holder
this.find('.URL-link').attr('href', encodeURI(url + field.data('suffix'))).text(previewUrl);

Proposed change to SiteTreeURLSegmentField.js:
Only add suffix if it not false.

// Transfer current value to holder
this.find(".URL-link").attr("href",encodeURI(t+(e.data("suffix")?e.data("suffix"):''))).text(n)}
RVXD added a commit to RVXD/silverstripe-cms that referenced this issue Mar 4, 2022
Removing the urlSuffix in Page.php by setting it to false works in source, but javascript adds it again. Resulting in link with undefined at the end.

**Setting URLSuffix to false, if page is published:**
```
public function getCMSFields()
{
      $fields = parent::getCMSFields();
        if( $this->isPublished() ) {
            $fields->FieldByName('Root.Main.URLSegment')->setURLSuffix(false);
        }
}
```
See issue: silverstripe#2723
silverstripe#2723
@michalkleiner
Copy link
Contributor

Do you want to open a pull request, @RVXD?

@emteknetnz
Copy link
Member

Sorry, we can't really do this. The ?stage=Stage query string is used to ensure that HTTP caching is disabled. Also at once point ?stage=Stage wasn't always used and instead sessions were used to say which stage you were on. However this was changed to always using the ?stage=Stage to resolve a class of bug of which I can't exactly remember

@RVXD
Copy link
Author

RVXD commented Mar 9, 2022

I still think the change to SiteTreeURLSegmentField.js could and should be made without having a negative impact on the functionality. A lot of users think this is the URL they can communicate to the world. The ?stage=Stage part does not have to be removed at all from the preview pane or anywhere else. Just from the segment field, so that if someone copies it, it is not in there.
The functionality of $fields->FieldByName('Root.Main.URLSegment')->setURLSuffix(false) is broken in the javascript part of the CMS. If set to false it should not give back undefined in the url.
Hope you can give it another thought.

@michalkleiner
Copy link
Contributor

Yeah, I think if @emteknetnz takes a second look and reads through the issue again, it is really only about fixing the JS to correctly respect the already existing settings, nothing else. It's not intended to remove the query param.
I'll reopen the issue and adjust the title.

@michalkleiner michalkleiner reopened this Mar 9, 2022
@michalkleiner michalkleiner changed the title Remove urlSuffix stage=Stage from URLSegment in CMS Using setURLSuffix on URLSegment field is not respected in SiteTreeURLSegmentField.js correctly Mar 9, 2022
@michalkleiner
Copy link
Contributor

Alternatively, if we want to ensure false can't be passed to setURLSuffix we need to throw an application exception.

@GuySartorelli
Copy link
Member

GuySartorelli commented Mar 31, 2022

This change won't affect the default behaviour, but makes extending or changing the behaviour to suit the needs of a given project easier. Makes sense to me. I'd even go so far as to say that technically not respecting an empty suffix could be considered a bug.

@purplespider
Copy link
Contributor

purplespider commented Feb 23, 2024

I came here looking to do as @RVXD wants to, but this bug stops it from being possible to remove ?stage=Stage for those who wish to.

Another workaround for the problem @RVXD is trying to stop could be to override SiteTreeURLSegmentField.ss to add a "Copy" button, .e.g. adding:

<button onclick="navigator.clipboard.writeText('{$URLPrefix}{$Value}')" role="button" type="button" class="btn btn-outline-secondary btn-sm font-icon-clipboard-pencil">Copy</button>

Which then looks like this:
Screenshot 2024-02-23 at 16 51 44@2x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants