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
260 stars 221 forks source link

No URL created for root node. Should be "/". #2759

Closed ronykhoury closed 4 years ago

ronykhoury commented 4 years ago

Description

The URL to the root node should be a slash /. Instead the href attribute of the a-tag is completely empty.

Steps to Reproduce

  1. For example, consider a node tree structure like this:
    + Root page       This is a normal page with content on it
    |-- Home          This is a shortcut to the Root page
    |-- About us      This is a normal page with content on it
  2. Here's an example Fusion template to generate a link list (navigation, etc.):

    prototype(MySite.Website:Document.Fragment.Menu.Main) < prototype(Neos.Fusion:Component) {
    root = ${site}
    menuItems = Neos.Neos:MenuItems
    
    renderer = afx`
    <ul>
      <Neos.Fusion:Loop items={props.menuItems}>
        <li>
          <Neos.Neos:NodeLink node={item.node} attributes.class="nav-item">
            {item.label}
          </Neos.Neos:NodeLink>
        </li>
      </Neos.Fusion:Loop>
    </ul>
    `
    }
  3. This is how the generated HTML for the link list looks like:
    <u>
    <li>
    <a href class="nav-item">Home</a>
    </li>
    <li>
    <a href="/about-us.html" class="nav-item">About us</a>
    </li>
    ...
    </ul>

Expected behavior

The link pointing to the root page should look like this:

<a href="/" class="nav-item">Home</a>

Actual behavior

The URL to the root page is not being generated. The href attribute is empty.

Affected Versions

Neos/Flow: Tested with the latest Neos packages (list created via composer show | grep "neos/"):

neos/behat                           dev-master 33c186a
neos/buildessentials                 dev-master a7ee073
neos/cache                           6.0.3             
neos/composer-plugin                 2.0.1             
neos/content-repository              5.0.3             
neos/diff                            5.0.3             
neos/eel                             6.0.3             
neos/error-messages                  6.0.3             
neos/flow                            6.0.3             
neos/flow-log                        6.0.3             
neos/fluid-adaptor                   6.0.3             
neos/form                            5.0.1             
neos/fusion                          5.0.3             
neos/fusion-afx                      v1.3.2            
neos/http-factories                  6.0.3             
neos/imagine                         3.1.2              
neos/kickstarter                     6.0.3             
neos/media                           5.0.3             
neos/media-browser                   5.0.3             
neos/neos                            5.0.3             
neos/neos-setup                      dev-master 56c8742
neos/neos-ui                         5.0.0             
neos/neos-ui-compiled                5.0.0              
neos/nodetypes-assetlist             5.0.3             
neos/nodetypes-basemixins            5.0.3             
neos/nodetypes-contentreferences     5.0.3             
neos/nodetypes-form                  5.0.3             
neos/nodetypes-html                  5.0.3             
neos/nodetypes-navigation            5.0.3             
neos/party                           5.0.2             
neos/redirecthandler                 4.0.2             
neos/redirecthandler-databasestorage 4.0.1             
neos/redirecthandler-neosadapter     4.0.3             
neos/redirecthandler-ui              2.0.3             
neos/seo                             3.0.5             
neos/setup                           dev-master 8611798
neos/site-kickstarter                5.0.3             
neos/twitter-bootstrap               3.0.5             
neos/utility-arrays                  6.0.3             
neos/utility-files                   6.0.3             
neos/utility-mediatypes              6.0.3             
neos/utility-objecthandling          6.0.3             
neos/utility-opcodecache             6.0.3             
neos/utility-pdo                     6.0.3             
neos/utility-schema                  6.0.3             
neos/utility-unicode                 6.0.3             

PHP: PHP Version => 7.3.9-1+0~20190902.44+debian9~1.gbpf8534c

ronykhoury commented 4 years ago

@kitsunet As you requested in slack, I'm tagging you to this issue.

mficzel commented 4 years ago

A very simple way to reproduce the issue is to look at the header in a default neos-base-distribution or neos-dev-distribution. The neos-logo should linke the root of the site but actually has an empty href <a href>

mficzel commented 4 years ago

The uriBuilder already returns an empty string "" as url for the site node while in 4.3 it correctly returned "/".

mficzel commented 4 years ago

I managed to verify that the frontendNodeRoutePartHandler also did not change between 4.3 an 5.0 it return "" as path for the site node. My current suspicion is that the Router in Flow changed the handling of empty pathes unintentionally.

janpawellek commented 4 years ago

This bug is pretty critical since it breaks the "Home" link (e.g. in the menu when clicking on the logo) for many production sites. An empty href attribute will cause the browser to stay on the page instead of navigating to the home page.

I found the bug beeing introduced recently in commit https://github.com/neos/flow-development-collection/commit/9cb62156fd8f452447d743729039fbe95d66ee5d For me https://github.com/janpawellek/flow-development-collection/commit/41b9a38200a0cd332d0f3915804da556a9d0bdfb fixes the issue, however I don't understand the implications (at least tests won't work anymore). So this might not be the proper way to fix it. Maybe @skurfuerst can help?

mficzel commented 4 years ago

@janpawellek can you open a pr for that since you already have a solution

skurfuerst commented 4 years ago

Hey, I'll also check the stuff. Sorry for the breakage!

All the best, Sebastian

skurfuerst commented 4 years ago

@janpawallek @mficzel @kitsunet IMHO the fix is great the way you prepared it; I fixed the testcases in https://github.com/neos/flow-development-collection/pull/1839.

IMHO this case happens now because https://github.com/neos/flow-development-collection/pull/1804/files#diff-fcf7039aeecafa562dd6ad82a86bb3b4R361 has changed.

All the best, Sebastian

janpawellek commented 4 years ago

Great, thank you for the fast fix @skurfuerst !