It's required when using multilevel menus. The top level items should not be <a href="..."></a> links - just <div class="..."></div> (or span). They must not to lead to any path - just open some dynamic menu.

CommentFileSizeAuthor
#124 allow-menu-items-without-path-1543750-124.patch1.61 KBAndrej Galuf
#105 allow-menu-items-without-path-1543750-95.diff2.56 KBmlevasseur
#95 allow-menu-items-without-path-1543750-95.patch3.57 KBmgifford
#93 interdiff-1543750-88-93.txt3.46 KBInternetDevels
#93 allow-menu-items-without-path-1543750-93.patch3.69 KBInternetDevels
#88 drupal--1543750--allow-menu-items-without-path--88.patch3.74 KBmgifford
#84 drupal--1543750--allow-menu-items-without-path--84.patch3.68 KBmgifford
#79 drupal--1543750--allow-menu-items-without-path--79.patch9.23 KBamontero
#79 interdiff--75-79.txt2.63 KBamontero
#77 drupal--1543750--allow-menu-items-without-path--77.patch9.23 KBamontero
#77 interdiff--75-77.txt2.63 KBamontero
#75 drupal--1543750--allow-menu-items-without-path--75.patch7.29 KBamontero
#72 allow_empty_menu_path-1543750-72.patch6.6 KBidflood
#72 interdiff.txt748 bytesidflood
#71 allow_empty_menu_path-1543750-71.patch6.61 KBidflood
#71 interdiff.txt2.44 KBidflood
#68 allow_empty_menu_path-1543750-68.patch6.56 KBidflood
#68 interdiff.txt2.34 KBidflood
#66 allow_empty_menu_path-1543750-66-test-only.patch2.26 KBidflood
#66 allow_empty_menu_path-1543750-66.patch6.52 KBidflood
#63 allow_empty_menu_path-1543750-63.patch4.25 KBmgifford
#60 allow_empty_menu_path-1543750-60.patch4.26 KBmgifford
#49 allow_empty_menu_path-1543750-49.patch7.72 KBprogga
#46 allow_empty_menu_path-1543750-46.patch7.7 KBprogga
#45 Path-none.png28.85 KBmgifford
#42 allow_empty_menu_path-1543750-42-test-only.patch2.26 KBidflood
#42 allow_empty_menu_path-1543750-42.patch7.7 KBidflood
#37 allow_empty_menu_path-1543750-37.patch5.44 KBprogga
#35 allow_empty_menu_path-1543750-35.patch4.54 KBprogga
#30 allow_empty_menu_path-1543750-30.patch2.88 KBmgifford
#24 allow_empty_menu_path-1543750-24.patch2.89 KBdagmar
#19 allow_empty_menu_path-1543750-18.patch2.9 KBidflood
#16 allow_empty_menu_path-1543750-16.patch2.85 KBidflood
#13 allow_empty_menu_path-1543750-13.patch2.25 KBidflood
#9 allow-empty-menu-path-9.patch586 bytesmgifford
#1 allow-empty-menu-path.patch.txt887 bytesamontero
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

amontero’s picture

+1.
Searching for how to allow empty paths reveals that this is a popular requested feature, especially to accomplish multi level menu parent items that should lead to nowhere, just contain more items.
I tried modifying drupal_valid_path to accept "#" as a valid path, but didn't worked. Menu item is not displayed in the menu item list admin page (however, it is saved in the DB). I can't figure it out with my current core knowledge. Anyway, the # path may not be the best solution.
Also tagging to make it more visible (added "DrupalWTF" tag because seems such an obvious feature to me).
It's a must have to allow multilevel menus, and ML menus are needed in lots of scenarios, IMO.

Related modules:
https://drupal.org/project/special_menu_items
https://drupal.org/project/void_menu

shp’s picture

In my opinion "<none>" is better than "#".

P. S. The extension of the patch must be ".patch" and the status of the issue must be "needs review" to be patch automatically tested.

amontero’s picture

Searching around later I found also <none> as a possible path, and I like it more, too.
I attached the patch in txt form to avoid the testbot go through it, since I actually know it is failing and it was just a "trying luck" quick attempt :)

amontero’s picture

mgifford’s picture

Curios why this is tagged with accessibility? The accessibility issue hasn't been defined.

gagarine’s picture

I'm the maintainer of special menu items. I think it would be better to create a more extensible system using token. The basic idea is module can register a function than return the link content for token like , , ...

See #1287610: Create a Menu Item Type API

webchick’s picture

Status: Active » Needs work
Issue tags: +Needs tests

Looks like there is a patch here, although it's needs work. Marking appropriately. We would also need tests for this.

mgifford’s picture

Status: Needs work » Needs review

bumping the bot as this should be tested. It applies nicely in .git, but not sure if it needs the .patch extension.

mgifford’s picture

FileSize
586 bytes

basically it's #1.

Status: Needs review » Needs work
Issue tags: -Needs tests, -DrupalWTF, -Accessibility, -menu system

The last submitted patch, allow-empty-menu-path-9.patch, failed testing.

mgifford’s picture

Status: Needs work » Needs review

#9: allow-empty-menu-path-9.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Needs tests, +DrupalWTF, +Accessibility, +menu system

The last submitted patch, allow-empty-menu-path-9.patch, failed testing.

idflood’s picture

With the following patch the menu get displayed but it behave like the <front>. I tried to modify the theme_menu_link function but it doesn't have any effect, certainly a little piece I'm missing.

shp’s picture

Status: Needs work » Needs review

+1 for "<none>" instead of "#"!

JordanMagnuson’s picture

Also agree that the path should be "", rather than "#"

idflood’s picture

With the following patch if a menu item as a path <none> it will not be outputted as a link but simply as text. I believe some work is still needed but it should be a good starting point.

edit: Here are some things missing:

  • Document the <none> special path just like the <front>
  • Write test(s)
  • Maybe if a menu as a <none> path we should automatically expend it
  • Maybe Fix styling in bartik, seven, ... since without the link the menu doesn't look right

I certainly forgot some other remaining tasks so if you have other ideas feel free to add them.

AaronMcHale’s picture

Hi, just saw this from another module.

This would be a great solution to this problem. I havn't tried out this patch yet, however I was wondering if it still creates an <a></a> tag, because when applying styles if there is no <a> tag in e.g. a main menu, when it comes to creating interactive menus, not having the <a> tag can conflict with some menu styling techniques.

idflood’s picture

@AaronMcH: The patch in #16 simply remove the < a > tag if a the link is < none >.

Here is a new patch which adds a < span > around the element title. This should resolve the possible styling issue I hope. The only possibility that I see with the < a > tag would be to make an href="#", but that would break javascripts relying on the url to define a state for instance.

idflood’s picture

AaronMcHale’s picture

The problem that I am having is that "#" isn't recognised as a path when trying to create a new menu item. I wonder if you could make it configurable under the settings for that menu, as I could think of a number of possible HTML tags that a person might want to use.

That actually give me another idea, what if "<none>" just removed the "<a>" tag from the output, However the user could use say "<tag>p class="note"" or "<tag>a href="#"", where anything after the "&lttag>" would be wrapped around "&lt&gt&quote; in the output, and at the end the closing tag would be inserted.

What do you think?
Thanks

mgifford’s picture

mgifford’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests, +DrupalWTF, +Accessibility, +menu system

The last submitted patch, allow_empty_menu_path-1543750-18.patch, failed testing.

dagmar’s picture

Status: Needs work » Needs review
FileSize
2.89 KB

Re-rolled

Status: Needs review » Needs work

The last submitted patch, allow_empty_menu_path-1543750-24.patch, failed testing.

crutch’s picture

needed to fire menu minipanels if wanting to use click trigger instead of hover

andypost’s picture

mgifford’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests, -DrupalWTF, -Accessibility, -menu system

Status: Needs review » Needs work
Issue tags: +Needs tests, +DrupalWTF, +Accessibility, +menu system

The last submitted patch, allow_empty_menu_path-1543750-24.patch, failed testing.

mgifford’s picture

Status: Needs work » Needs review
FileSize
2.88 KB

Just a re-roll, but I'm getting an error with PHP Fatal error: Cannot redeclare menu_link_load() that might be related to another patch. Not sure.

mgifford’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests, +DrupalWTF, +Accessibility, +menu system

The last submitted patch, allow_empty_menu_path-1543750-30.patch, failed testing.

andypost’s picture

I see no code around that redeclares menu_item_load()

tstoeckler’s picture

+++ b/core/includes/common.inc
@@ -2094,7 +2094,7 @@ function url($path = NULL, array $options = array()) {
   // The special path '<front>' links to the default front page.
...
+  if ($path == '<front>' || $path == '<none>') {

The comment should be updated.

progga’s picture

Status: Needs work » Needs review
FileSize
4.54 KB

Rerolling patch #30 with the help from folks at the Drupal Drop In Sprint - London February 2013

andypost’s picture

Status: Needs review » Needs work
+++ b/core/includes/common.incundefined
@@ -2118,7 +2118,7 @@ function url($path = NULL, array $options = array()) {
   // The special path '<front>' links to the default front page.
-  if ($path == '<front>') {
+  if ($path == '<front>' || $path == '<none>') {

#34 not addressed

+++ b/core/includes/theme.incundefined
@@ -1801,7 +1801,13 @@ function theme_links($variables) {
-          $item = l($link['title'], $link['href'], $link);
+         if ($link['href'] != '<none>') {
+           $item = l($link['title'], $link['href'], $link);
+         }
+         else {
+           $item = '<span>' . $link['title'] . '</span>';
+         }
+

wrong code indent

+++ b/core/modules/menu_link/lib/Drupal/menu_link/MenuLinkStorageController.phpundefined
@@ -143,7 +143,7 @@ public function save(EntityInterface $entity) {
     // This is the easiest way to handle the unique internal path '<front>',
     // since a path marked as external does not need to match a router path.
-    $entity->external = (url_is_external($entity->link_path) || $entity->link_path == '<front>') ? 1 : 0;
+    $entity->external = (url_is_external($entity->link_path) || $entity->link_path == '<front>' || $entity->link_path == '<none>') ? 1 : 0;

this comment should be changed as #34 suggests too

progga’s picture

Status: Needs work » Needs review
FileSize
5.44 KB

Rerolled patch #35 based on @andypost's feedback.

andypost’s picture

Status: Needs review » Needs work

Great! Now it needs test coverage for RTBC

smira’s picture

+1 for RTBC
i tested on latest d8 build and works like a charm

smira’s picture

Status: Needs work » Reviewed & tested by the community
pjcdawkins’s picture

Status: Reviewed & tested by the community » Needs work

See #38 - it needs automated tests.

idflood’s picture

Status: Needs work » Needs review
FileSize
7.7 KB
2.26 KB

I tried to add some tests but I'm not sure if I made anything right. It works but the test may belong to another file, and the tests are probably too specific.

Status: Needs review » Needs work
Issue tags: -Needs tests, -DrupalWTF, -Accessibility, -menu system

The last submitted patch, allow_empty_menu_path-1543750-42.patch, failed testing.

idflood’s picture

Status: Needs work » Needs review
Issue tags: +Needs tests, +DrupalWTF, +Accessibility, +menu system
mgifford’s picture

FileSize
28.85 KB

that works for path <none> when I added a new menu item to the footer - admin/structure/menu/manage/footer

results" />

I think someone should look over the tests and mark it RTBC.

progga’s picture

Combined the 2 patches from #42.

dawehner’s picture

+++ b/core/includes/common.incundefined
@@ -1811,6 +1812,10 @@ function url($path = NULL, array $options = array()) {
+  else if ($path == '<none>') {

The drupal code style is a bit odd here: it should be "elseif"

+++ b/core/includes/path.incundefined
@@ -199,7 +199,7 @@ function drupal_valid_path($path, $dynamic_allowed = FALSE) {
+  if ($path == '<front>' || $path == '<none>' || url_is_external($path)) {

Maybe it's easier to read when it's using an in_array($path, array('

+++ b/core/modules/system/lib/Drupal/system/Tests/Menu/EmptyLinkPathTest.phpundefined
@@ -0,0 +1,70 @@
+ * Definition of Drupal\system\Tests\Menu\EmptyLinkPathTest.

Should be \Contains ...

+++ b/core/modules/system/lib/Drupal/system/Tests/Menu/EmptyLinkPathTest.phpundefined
@@ -0,0 +1,70 @@
+class EmptyLinkPathTest extends WebTestBase {

Have you tried making this a DrupalUnitTest? I don't see any browsing on the actual site, so this seems possible, even if it's harder to do.

+++ b/core/modules/system/lib/Drupal/system/Tests/Menu/EmptyLinkPathTest.phpundefined
@@ -0,0 +1,70 @@
+  public static function getInfo() {

According to http://drupal.org/node/1354 there should be an empty lines after the class.

+++ b/core/modules/system/lib/Drupal/system/Tests/Menu/EmptyLinkPathTest.phpundefined
@@ -0,0 +1,70 @@
+  function testEmptyLinkPath($module = 'menu_test') {

it's odd to have a parameter. I don't think that something calls the test method with a parameter :) .. Additional this method should be marked as public.

aspilicious’s picture

It doesn't happen a lot but I had a usecase where I had to create a "#something" path (yes, an internal page link) on a menu item. But drupal doesn't allow that.

Probably another issue but I would see this solved too.

progga’s picture

Updated patch #46 based on @dawehner's 1st and 3rd suggestions.

@aspilicious, this patch will not allow you to enter "#something" as a menu path. Sorry to disappoint you :-(

klonos’s picture

I'd be interested in that too. Separate issue then?

amontero’s picture

@aspillicious @klonos: comment #4 has links to issues that may be related.

dawehner’s picture

+++ b/core/modules/system/lib/Drupal/system/Tests/Menu/EmptyLinkPathTest.phpundefined
@@ -0,0 +1,69 @@
+class EmptyLinkPathTest extends WebTestBase {

Is there a reason why we can't use DrupalUnitTestBase here?

progga’s picture

> Is there a reason why we can't use DrupalUnitTestBase here?

Because url(), drupal_valid_path(), etc. hits the database. The relevant tables will be absent during unit test runs.

mparker17’s picture

@mgifford asked in #5 why this was tagged with accessibility, but I don't think that question ever got answered. Anybody know?

kbell’s picture

Does anyone know the backport-to-D7 status of this patch?
Thanks,
Kelly

mgifford’s picture

@mparker17 - I suspect it's because people would like to add skip-links to the navigation. Having a menu that allows you to jump to specific anchors would be quite useful.

@kbell - Until it's committed to 8, the backport to 7 likely isn't going to be started.

andypost’s picture

+1 to anchors

mgifford’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests, +DrupalWTF, +Accessibility, +menu system, +LONDON_2013_APRIL

The last submitted patch, allow_empty_menu_path-1543750-49.patch, failed testing.

mgifford’s picture

Status: Needs work » Needs review
FileSize
4.26 KB

functions moved to core/modules/menu_link/lib/Drupal/menu_link/Plugin/Core/Entity/MenuLink.php

core/includes/common.inc seems to no longer be relevant.

Status: Needs review » Needs work
Issue tags: -Needs tests, -DrupalWTF, -menu system, -LONDON_2013_APRIL

The last submitted patch, allow_empty_menu_path-1543750-60.patch, failed testing.

andypost’s picture

+++ b/core/modules/menu_link/lib/Drupal/menu_link/Plugin/Core/Entity/MenuLink.phpundefined
@@ -426,9 +426,9 @@ public static function postDelete(EntityStorageControllerInterface $storage_cont
-    $this->external = (url_is_external($this->link_path) || $this->link_path == '<front>') ? 1 : 0;
+    $entity->external = (url_is_external($entity->link_path) || $entity->link_path == '<front>' || $entity->link_path == '<none>') ? 1 : 0;

There's no $entity use $this->external

mgifford’s picture

Status: Needs work » Needs review
FileSize
4.25 KB

This makes sense looking at the code. Thanks @andypost!

amateescu’s picture

Component: menu system » menu_link.module

The patch looks good to me, but we still need test coverage..

dawehner’s picture

Issue tags: +Needs tests

add tag

idflood’s picture

Here is a simple reroll of the test I created in #42. The second file contains the test and the patch from #63.

I'm not sure why the url function is not modified in #63 like it was in #42 but there may be a good reason for it. So we either can remove the test on the url function or fix it, both solutions should work.

amateescu’s picture

Status: Needs review » Needs work
--- /dev/null
+++ b/core/modules/system/lib/Drupal/system/Tests/Menu/EmptyLinkPathTest.php

This test should be moved to the menu_link module.

+++ b/core/modules/system/lib/Drupal/system/Tests/Menu/EmptyLinkPathTest.php
@@ -0,0 +1,70 @@
+ * Definition of Drupal\system\Tests\Menu\EmptyLinkPathTest.

Contains \Drupal...

+++ b/core/modules/system/lib/Drupal/system/Tests/Menu/EmptyLinkPathTest.php
@@ -0,0 +1,70 @@
+use Drupal\simpletest\WebTestBase;

This should be a DrupalUnit test.

+++ b/core/modules/system/lib/Drupal/system/Tests/Menu/EmptyLinkPathTest.php
@@ -0,0 +1,70 @@
+  public static function getInfo() {

Missing {@inheritdoc}.

+++ b/core/modules/system/lib/Drupal/system/Tests/Menu/EmptyLinkPathTest.php
@@ -0,0 +1,70 @@
+   * Test automatic reparenting of menu links.
+   */
+  function testEmptyLinkPath($module = 'menu_test') {

That doesn't sound right :)

+++ b/core/modules/system/lib/Drupal/system/Tests/Menu/EmptyLinkPathTest.php
@@ -0,0 +1,70 @@
+    $this->assertTrue($path_is_valid, '&lt;none> is a valid path.');

This is weird, why do we escape the first one but not the second? Same for all occurences in this test.

+++ b/core/modules/system/lib/Drupal/system/Tests/Menu/EmptyLinkPathTest.php
@@ -0,0 +1,70 @@
+    $this->assertTrue($contains_link, 'Menu link without path return a span.');

Maybe "Menu items without a path are displayed as a element."?

idflood’s picture

Status: Needs work » Needs review
FileSize
2.34 KB
6.56 KB

Thanks for the review. Here is a reroll with some corrections but not all.

Contains \Drupal...

Checked on other tests and this line looks right. Could you explain what is wrong?

This should be a DrupalUnit test.

#53 explain why this can't be a DrupalUnit test.

Missing {@inheritdoc}.

This is not present on all other tests I checked but I still added it : )

edit: I wanted to make the url method pass the test again but I've found that it changed a lot... We certainly need to make something like PathProcessorFront.

Status: Needs review » Needs work

The last submitted patch, allow_empty_menu_path-1543750-68.patch, failed testing.

amateescu’s picture

Contains \Drupal...

Checked on other tests and this line looks right. Could you explain what is wrong?

Not all files have been upgraded to the new standard, which is "Contains \ClassName". See https://drupal.org/node/1354#file.

#53 explain why this can't be a DrupalUnit test.

This is exactly why I said DrupalUnit test and not just Unit test :) More specifically, I'm talking about DrupalUnitTestBase, which has the ability to create database tables.

About {@inheritdoc}, yes, I know not all files have them, but that shouldn't stop us to make the new ones right ;)


And now for some more nitpicks:


+++ b/core/modules/menu_link/lib/Drupal/menu_link/Tests/EmptyLinkPathTest.php
@@ -0,0 +1,73 @@
+class EmptyLinkPathTest extends WebTestBase {
+  /**
+   * {@inheritdoc}

Missing empty line before the docblock of this method.

+++ b/core/modules/menu_link/lib/Drupal/menu_link/Tests/EmptyLinkPathTest.php
@@ -0,0 +1,73 @@
+   * Test creation of empty menu links.

Tests creation..

+++ b/core/modules/menu_link/lib/Drupal/menu_link/Tests/EmptyLinkPathTest.php
@@ -0,0 +1,73 @@
+  function testEmptyLinkPath($module = 'menu_test') {

All methods should specify their visibility, in this case it's 'public'.

+++ b/core/modules/menu_link/lib/Drupal/menu_link/Tests/EmptyLinkPathTest.php
@@ -0,0 +1,73 @@
+    $this->assertTrue($contains_link, 'Menu items without a path are displayed as a element.');

My mistake in the previous post, the <span> tag was cut off. So this should be: 'Menu items without a path are displayed as a <span> element.'

+++ b/core/modules/menu_link/lib/Drupal/menu_link/Tests/EmptyLinkPathTest.php
@@ -0,0 +1,73 @@
+    $this->assertTrue($contains_link, 'Function theme_links() accept &lt;none> path.');

Same problem as before, the right > is not escaped.

And now you made me curious about url().. what exactly doesn't work about it?

idflood’s picture

Status: Needs work » Needs review
FileSize
2.44 KB
6.61 KB

Thanks for the review and the explanations : )

The url() function return something like this if you pass < none >: drupalroot/%3Cnone%3E
The test check for an empty string which should be better I think.

idflood’s picture

Oops, I forgot to remove a thing I added to check what returned the url method.

Status: Needs review » Needs work
Issue tags: -Needs tests, -Accessibility

The last submitted patch, allow_empty_menu_path-1543750-72.patch, failed testing.

kovacevo’s picture

Status: Needs work » Needs review
Issue tags: +Needs tests, +Accessibility
amontero’s picture

Status: Needs review » Needs work

The last submitted patch, drupal--1543750--allow-menu-items-without-path--75.patch, failed testing.

amontero’s picture

Status: Needs work » Needs review
FileSize
2.63 KB
9.23 KB

Reroll keeps failing on the same single test: "Url return an empty string." on EmptyLinkPathTest.php, line 40 - Drupal\menu_link\Tests\EmptyLinkPathTest->testEmptyLinkPath()

First attempt to fix the failing url() return value test. Instead of expecting url() returning '', now it expects '#'.
Also, the url() function still needed to be modified for dealing with ''. Now it should do with adding a path processor for it (PathProcessorNone.php).

Status: Needs review » Needs work

The last submitted patch, drupal--1543750--allow-menu-items-without-path--77.patch, failed testing.

amontero’s picture

Status: Needs work » Needs review
FileSize
2.63 KB
9.23 KB

Erroneous git format-patch against wrong branch. This is the right patch for #77.

Status: Needs review » Needs work
Issue tags: -Needs tests, -Accessibility

The last submitted patch, drupal--1543750--allow-menu-items-without-path--79.patch, failed testing.

mgifford’s picture

Status: Needs work » Needs review
Issue tags: +Needs tests, +Accessibility
mgifford’s picture

There was just this one error in the test last time:

Url return a '#' string. Other EmptyLinkPathTest.php 40 Drupal\menu_link\Tests\EmptyLinkPathTest->testEmptyLinkPath()

Status: Needs review » Needs work

The last submitted patch, drupal--1543750--allow-menu-items-without-path--79.patch, failed testing.

mgifford’s picture

Status: Needs work » Needs review
FileSize
3.68 KB

Reroll

The last submitted patch, drupal--1543750--allow-menu-items-without-path--84.patch, failed testing.

mgifford’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, drupal--1543750--allow-menu-items-without-path--84.patch, failed testing.

Anonymous’s picture

Issue summary: View changes

Some illustrations...

mgifford’s picture

mgifford’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 88: drupal--1543750--allow-menu-items-without-path--88.patch, failed testing.

The last submitted patch, 88: drupal--1543750--allow-menu-items-without-path--88.patch, failed testing.

mgifford’s picture

Hmm, this is failing to install with this on simplyTest.me:

Database configuration
Class Drupal\Core\PathProcessor\PathProcessorNone does not exist

Seems ok though in my local install.

InternetDevels’s picture

Status: Needs work » Needs review
FileSize
3.69 KB
3.46 KB

Added new patch.

Status: Needs review » Needs work

The last submitted patch, 93: allow-menu-items-without-path-1543750-93.patch, failed testing.

mgifford’s picture

Status: Needs work » Needs review
FileSize
3.57 KB

reroll...

Status: Needs review » Needs work

The last submitted patch, 95: allow-menu-items-without-path-1543750-95.patch, failed testing.

mgifford’s picture

Ok, not sure what changed here, but...

WD php: ReflectionException: Class [error]
Drupal\Core\PathProcessor\PathProcessorNone does not exist in
ReflectionClass->__construct() (line 959 of
/var/lib/drupaltestbot/sites/default/files/checkout/core/vendor/symfony/dependency-injection/Symfony/Component/DependencyInjection/ContainerBuilder.php).
ReflectionException: Class Drupal\Core\PathProcessor\PathProcessorNone does not exist in ReflectionClass->__construct() (line 959 of /var/lib/drupaltestbot/sites/default/files/checkout/core/vendor/symfony/dependency-injection/Symfony/Component/DependencyInjection/ContainerBuilder.php).
Drush command terminated abnormally due to an unrecoverable error. [error]

slotid’s picture

In a file js.js which is marked in the *.info file in your theme directory put a code like that one below

jQuery(document).ready(function($){	
	
	$('#menu-main-id > a').removeAttr('href').css('cursor', 'default');
	
});
mgifford’s picture

Issue tags: +Needs reroll

So, this change (adding %none):
core/modules/menu_link/lib/Drupal/menu_link/MenuLinkFormController.php

needs to get moved to:
core/modules/menu_link_content/src/Form/MenuLinkContentForm.php

core/includes/menu.inc core/includes/path.inc & core/core.services.yml are all changed too.

paulmckibben’s picture

This needs pretty significant re-architecture since the last patch. I started to try, but quickly got lost. :)
A key change is that the Link field is now used to implement the menu link destination field. I don't think it's a good idea for the Link field to support <none> by default.
Also, https://www.drupal.org/node/2421941 has a discussion about removing the use of <front>.

Where does all this leave this issue? For example, would it make sense to NOT require a value in the link destination field, allowing it to be empty?

AaronMcHale’s picture

I think these should still exist, the difference between these and is front represents an actual path, which really should be just a /, whereas these are like technical replacement tokens that perform special operations.

rishikant05’s picture

Assigned: Unassigned » rishikant05
rishikant05’s picture

Assigned: rishikant05 » Unassigned
mikey_p’s picture

So I just dug into this a little bit and i think there are going to be three main components to supporting this (either in core or contrib):

1) Menu link content entity needs to have altered definition to make link field not required. This pretty much works just by altering the baseFieldDefinitions().
2) Menu link plugin type needs to support having an empty link or empty URL related parameters.
3) The menu.html.twig template will need to check the menu_link isn't empty before printing the link, and alternately just print the title.

I tried this and the main problem I ran into is that of course portions of the MenuContentLink entity assume a link is required. The entity technically saved to the DB just fine, but the postSave() method calls some bits to update the menu link plugin definition, which tries to load the URL from the link, which is NULL and produces a fatal error. If this is supported in contrib, then this will probably need a new entity class to extend/override core/modules/menu_link_content/src/Entity/MenuLinkContent.php.

mlevasseur’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
2.56 KB

Rerolled...

Conflicts: core/core.services.yml, core/includes/menu.inc
Removed: core/includes/path.inc, core/modules/menu_link/lib/Drupal/menu_link/MenuLinkFormController.php

Status: Needs review » Needs work

The last submitted patch, 105: allow-menu-items-without-path-1543750-95.diff, failed testing.

mikey_p’s picture

I don't think the patch above is moving the right direction. Instead of allowing a new path, we should be working towards not requiring a path at all. Further, theme_menu_link() shouldn't be added as that is not part of menu.html.twig. Likewise, the changes to template_preprocess_links() don't make any sense either as you can see it has support for handling text links (items without a URL) and the changes there will break links.html.twig anyway.

danielbeeke’s picture

I created a proof of concept to fix this with a contrib module

https://www.drupal.org/sandbox/danielbeeke/2515610

Internet’s picture

D8 - As above tried above in #1.
We used 'url path setting': /# (# on its own would not be accepted) and the placeholder appears to work fine with bootstrap light.
Result: a class="dropdown-toggle" aria-expanded="false" aria-haspopup="true" role="button" data-toggle="dropdown" href="#"

dawehner’s picture

Version: 8.0.x-dev » 8.1.x-dev
Component: menu_link.module » menu_link_content.module

This sounds like a perfect feature request for 8.1. Its also great that we indeed already have a contrib module, thank you @danielbeeke

danielbeeke’s picture

I have updated menu_link_placeholder.

The module makes it possible to skip the field 'path' like mikey_p said.
To use the module you need to change your menu.tpl. As mikey_p said, the rendering of a link is where it goes wrong.

In preprocess_menu_link the module adds 'no_link' => TRUE, my template picks that up and renders a span.

Maybe we can do something like that too in core?

dawehner’s picture

Well, let's discuss how to implement it properly, that for sure is a good idea.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Manuel Garcia’s picture

So now that #2693725: Add <nolink> to allow for non-link links is in, do we still need this?

mgifford’s picture

Status: Needs work » Closed (duplicate)

I can't see why. We can re-open if needed.

generalredneck’s picture

Looks like the functionality that was commited to 8.2.x doesn't allow you to do this via the UI so I don't know if this matches yalls usecase or not... I'm using the latest 8.2.x-dev as of today.

http://screencast.com/t/QYk74K0xspk
http://screencast.com/t/TsblTBTuHT

pixelsweatshop’s picture

@generalredneck, I see you used <none> when it looks like it should be <nolink>

Edit: Removed. Your second image wasn't working when I looked at it. I see you tried nolink as well.

Andrej Galuf’s picture

GUI fails because the form validates the path and <nolink> is not a valid path (unlike <front>, which gets converted into one). This originates in MenuLinkContent.php's baseFieldDefinitions, which defines the link field as... well... link.

ckaotik’s picture

Status: Closed (duplicate) » Needs work

Reopened as the GUI does not yet allow for this yet.

I've worked around this using this patch, however I don't feel that it's the best solution to work with this. Patching \Drupal\Core\Url certainly doesn't feel right, there must be a better solution :/

diff --git a/core/lib/Drupal/Core/Url.php b/core/lib/Drupal/Core/Url.php
index a37627e..188c7f5 100644
--- a/core/lib/Drupal/Core/Url.php
+++ b/core/lib/Drupal/Core/Url.php
@@ -398,7 +398,7 @@ protected static function fromInternalUri(array $uri_parts, array $options) {
     // only accept/contain paths without a leading slash, unlike 'internal:'
     // URIs, for which the leading slash means "relative to Drupal root" and
     // "relative to Symfony app root" (just like in Symfony/Drupal 8 routes).
-    if (empty($uri_parts['path'])) {
+    if (empty($uri_parts['path']) || $uri_parts['path'] === '<nolink>') {
       $uri_parts['path'] = '<none>';
     }
     elseif ($uri_parts['path'] === '/') {
diff --git a/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php b/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php
index fd670fd..5e168dc 100644
--- a/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php
+++ b/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php
@@ -137,7 +137,7 @@ public static function validateUriElement($element, FormStateInterface $form_sta
     // URI , ensure the raw value begins with '/', '?' or '#'.
     // @todo '<front>' is valid input for BC reasons, may be removed by
     //   https://www.drupal.org/node/2421941
-    if (parse_url($uri, PHP_URL_SCHEME) === 'internal' && !in_array($element['#value'][0], ['/', '?', '#'], TRUE) && substr($element['#value'], 0, 7) !== '<front>') {
+    if (parse_url($uri, PHP_URL_SCHEME) === 'internal' && !in_array($element['#value'][0], ['/', '?', '#'], TRUE) && substr($element['#value'], 0, 7) !== '<front>' && substr($element['#value'], 0, 8) !== '<nolink>') {
       $form_state->setError($element, t('Manually entered paths should start with /, ? or #.'));
       return;
     }

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Stephen Ollman’s picture

How to get this progresses into the next Core release?

It's nice functionality to have and the requirement has been around since the early days of Drupal, so I'm surprised that it still hasn't been included in D8.

rcodina’s picture

@Stephen Ollman I'm surprised too. I was expecting this to be included on 8.2 version! In drupal 8 we have no Special Menu Items module! Anyone knows a workaround?

danielbeeke’s picture

Andrej Galuf’s picture

I've thrown myself at this this morning, starting with previous solutions above. Three things would need to be fixed in order to get this working (GUI inclusive):

  1. \Drupal\link\Plugin\Field\FieldWidget\LinkWidget::validateUriElement must accept the value of <none> in addition to <front>. This allows the GUI to validate the <none> path as valid.
  2. \Drupal\Core\Url:fromInternalUri must be passed an empty value that gets converted into <none>
  3. A theme fix to render an empty <a> as a <span> or something similar

Note that the comment for Url::fromInternalUri is false - the method does not accept <front> and <none>, it throws InvalidArgumentException if they are passed. However, <front> is converted to an internal path before that happens and <none> is not (we'll get to that). Ironically, the same method creates <front> and <none> afterwards, which are then validated and converted into urls. So we pass <none>, which gets converted into 'internal:', only to get reconverted into <none> again. Go figure.

Based on #2421941: Determine whether not only "/" but also "<front>" should be valid input for the Link widget I say the right solution is to stop the whole conversions with <front> and <none>, link should be allowed to be empty (i.e. not required) and done. This would return an empty href, which the template can render as <span> instead of <a> and we'd be done. Unfortunately, the value null also gets rejected, which means this is a dead-end without further modifications.

As it turns out, the key hides not in Url or in MenuLinkContent, but in LinkWidget, specifically in LinkWidget::getUserEnteredStringAsUri. This method is responsible for converting the string <front> into 'internal:/' (see line 118). If we add another condition right after it that converts <none> into an empty string and keep the validation from 1st point above, the GUI passes, validates and saves the value 'internal:', which returns a link with an empty href. Now it's just a matter of a link template handling empty href as a span. And to finalize it, fix LinkWidget::getUriAsDisplayableString to convert an empty (null) path into <none>

Required adjustments to LinkWidget are attached.

alexfdg’s picture

I´m not a code expert and I look for easy ways to solve issues without extra modules,
so I found this solution, hope somebody helps: http://www.thecomputergroup.com/blog/how-to-make-a-drupal-menu-item-link...

pwolanin’s picture

Status: Needs work » Fixed

You can already use a path of route:<none> or route:<nolink> so I think we can call this fixed.

davidhernandez’s picture

Status: Fixed » Needs work

I believe the intention is to not have it render an tag. Doesn't route: still do that?

dawehner’s picture

Status: Needs work » Fixed

The link generator certainly has dedicated code for exactly this usecase:

    elseif ($url->isRouted() && $url->getRouteName() === '<nolink>') {
      $generated_link = new GeneratedNoLink();
      unset($attributes['href']);
    }

given that, this seems to be solved already.

jibran’s picture

Status: Fixed » Closed (duplicate)
davidhernandez’s picture

Yes, I see. Didn't realize it before, but it indeed now produces a <span> tag instead of <a>. That's workable.

ckaotik’s picture

Yes, the functionality of a <nolink> route is supported by core, as per #2693725: Add <nolink> to allow for non-link links . But no, the ui does not allow its use yet. A link field (e.g. on a menu link) with input of <nolink> is still rejected in Drupal\link\Plugin\Field\FieldWidget\LinkWidget::validateUriElement().

This was exactly my point when I reopened this issue months ago. Andrej (see #124) also nicely described what's happening and came to the same conclusion. Without a patch, non-developers have no way of using this option.

davidhernandez’s picture

You have to input route:<nolink>. Not the most user friendly.

AaronMcHale’s picture

I'm with @davidhernandez, user shouldn't be required to input "route:", but where to proceed, does ths get re-opened, new issue, re-open issue linked in #129, does it even matter that the user needs to provide "route:"?

tim.plunkett’s picture

New issue please. The original premise of this (and the other) issue is addressed. It is no possibly, no matter how unwieldy.

GiorgosK’s picture

Why is this no in the description underneath
"route:< nolink >" so people don't have to look in the forums for something so simple ?
should we put a feature request ?

Leon Kessler’s picture

There's an issue using route:<nolink> whereby the LinkGenerator considers this a link to the front page. This mean the active class gets set incorrectly.
I've opened a separate issue with patch: #2838351: Links with no path get active class on front page