neos / neos-development-collection

The unified repository containing the Neos core packages, used for Neos development.
https://www.neos.io/
GNU General Public License v3.0
264 stars 222 forks source link

BUG: Shortcut Nodetypes do not support simple anchor targets #3829

Open mtelgkamp opened 2 years ago

mtelgkamp commented 2 years ago

Is there an existing issue for this?

Current Behavior

If I enter an anchor link like #top to the target of a shortcut, the result is an a-tag with href="/#top" attribute. This does not work as expected.

Expected Behavior

An anchor-only link should not be prepended with a leading /

Steps To Reproduce

Environment

- Flow: 7.3
- Neos: 7.3
- PHP: 7.4

Anything else?

I created a workaround similar to the one in #3803 where instead of anchor links javascript: links in the shortcut target where changed.

namespace Add\Namespace\For\Aspect;

use Neos\ContentRepository\Domain\Model\Node;
use Neos\Flow\Aop\JoinPointInterface;
use Neos\Flow\Annotations as Flow;

/**
 * @Flow\Scope("singleton")
 * @Flow\Aspect
 */
class AnchorShortcutTargetAspect
{
    /**
     * @Flow\Around("method(Neos\Flow\Mvc\Routing\UriBuilder->uriFor())")
     * @param \Neos\Flow\Aop\JoinPointInterface $joinPoint
     * @return string
     */
    public function rewritePluginViewUris(JoinPointInterface $joinPoint)
    {
        $arguments = $joinPoint->getMethodArguments();

        $node = $arguments['controllerArguments']['node'] ?? null;
        if ($node && $node instanceof Node && $node->getNodeType()->isOfType('Neos.Neos:Shortcut')) {
            $target = $node->getProperty('target');
            if (substr($target, 0, 1) === '#') {
                return $target;
            }
        }
        return $joinPoint->getAdviceChain()->proceed($joinPoint);
    }
}
mhsdesign commented 2 years ago

leading slash rings a bell ...

is this maybe related to:

Since guzzle/psr7 Version 2.0.0, the Uri-Path has to start with a slash: https://github.com/guzzle/psr7/commit/d09f8f0dc98f0ccff482ed36ed7b6b927dc8b287#diff-c39fba9d7bfeaa6ed2cdf1cd5a05de305dcf28bd5ef11e7e44c06f3d4d473fc5R730

remembered that thing from https://github.com/neos/redirecthandler/pull/58#issue-1279606821

mtelgkamp commented 2 years ago

In our case we use

             "name": "guzzlehttp/psr7",
            "version": "1.7.0",

so it does not seem to be related to guzzlehttp/psr7 v2.0.0

mhsdesign commented 2 years ago

And i couldnt find any recent changes to Neos\Flow\Mvc\Routing\UriBuilder->uriFor() in the blame ...

mtelgkamp commented 2 years ago

It does not need to be a recent change, as the problem was most likely introduced with an upgrade from an old Neos version (I think 4,x) to 7.1 / 7.3 so it might be a problem for a longer time already. If there is an alternative / better way to create anchor links I am happy to use it, but I do not have any opportunity in mind.

mhsdesign commented 2 years ago

Ahh i see, i now understand what you wanted to archive:

You are using dummy shortcuts with simple # links so that you have enforced/"typed"/neos-aware links when you use them.

Thats clever ^^

kdambekalns commented 2 years ago

An anchor-only link should not be prepended with a leading /

Unless you build an absolute URI (https://www.acme.com#anchor would be bad) or you pass the "path" to "PSR-7-things" (requires a path to start with a slash). Bottom line a "naked" anchor is not a proper URI!?

🤔

mtelgkamp commented 2 years ago

As far as I understand a valid URI does not need to have a / between the authority and the fragment, so https://www.acme.com#anchor is a valid URI.

RFC7230, Section 5.3.1 states that an empty path needs to be replaced by a "/" for the origin-form of an URI, but in the case we use it as a link destination the goal is not to have the origin form, but a valid link destination.

According to RFC3986, Section 3.3 path is allowed to be empty.

And according to the doc-comment on Psr\Http\Message\UriInterface getPath()

It's the task of the user to handle both "" and "/".

Which in my opinion says that both options are possible.

kdambekalns commented 1 year ago

Now I saw this for the first time (after an update of a site). Reminded me to come back here… Indeed this worked earlier.