Allow "'non-clickable" menu items

[stgoos]

In the new menu generator I'm missing the option to create "non-clickable" menu items.

What I'm after is that I want to create for example a main menu item with the title "Links" which is non-clickable (no url attached to it) and basically only acts as an umbrella item for the actual links I want to locate as subitems of the item "Links".

• Home
• Something else
• Links (this one should be non-clickable)
  • external link 1
  • external link 2
  • etc

I think that an optional tickbox in the add link section will do the trick. Basically, all it has to do is to "disable" the check whether or not a valid URL format has been submitted and, of course, it has to trigger some modified html output.

Hope you guys can add this in the 3.0 release because this would basically complete the menu generator :)

Keep up the good work and I'm really looking forward to the 3.0 release!

[ryan]

A workaround for now is to use "#" as the url.

[ryan]

This won't happen for 3.0.

[oellefsen]

Another workaround in 3.0 is to add a custom menu item with a bogus url, and then remove the url, keeping the label, once it's added to the menu structure.

Would be good to have it fixed in the next release, as having a top menu item (with subitems) as links is bad UI design, even for a webpage. I think users do not expext menus to behave like that.

I also really appreciate the new menu builder, it takes a lot of work out of making the site navigation. Thanks for the good work!

[Jesper800]

There are basically two approaches to this as far as the outputted HTML is concerned.

The first option is to output an anchor tag without the href-attribute. This is what already happens when you add a custom link and remove the URL, as oellefsen suggested. The second option is to remove/replace the anchor tag.

The second option will probably cause a lot of problems styling-wise, as a lot of styling is applied on the anchor tags in menus. The first one is the way to go, in my opinion, as it won't cause any of the problems and still have the desired effect.

[celloexpressions]

Replying to Jesper800:

The first option is to output an anchor tag without the href-attribute. This is what already happens when you add a custom link and remove the URL, as oellefsen suggested.

I'd be interested in seeing this approach. For the UI side, it would probably work well as a toggle in the add-custom-link area that hid the url field. It could potentially just create a custom menu item without a URL, utilizing the underlying functionality of the workaround.

Alternately, there could be an additional "placeholder" menu item type that exhibited this behavior.

[celloexpressions]

Introduce support for placeholder menu items, which function as custom items without urls.

[celloexpressions]

Improved front-end support for placeholder menu items in bundled themes.

[celloexpressions]

13273.diff​ implements core support and UI for "placeholder" nav menu items. Placeholders are similar to custom links, but without URLs. Accordingly, the UI is built-in to the existing "Links" section, as a checkbox that toggles the visibility of the URL field. The patch includes no-js support. Alternatively, a "Placeholders" section could be created, although that would be less elegant.

On the front-end, placeholders would primarily be used as parent menu items, as placeholders for sub-menus in top-level menus. Generally, the only enhancement that themes could (or would need to) make to support this feature would be to apply cursor: default to these menu items, in place of the default cursor: text for unlinked links. This enhancement certainly isn't required and some themes may take advantage of this feature further than just tweaking the cursor. 13273.diff​ implements the small CSS fix in all of the bundled themes, as the fix improves the UX somewhat for each of those themes.

[DrewAPicture]

I think we've all probably seen a use-case for this at one time or another so it's not without merit.

However, I have some concerns, primarily with introducing a core feature that requires theme styling changes to work as expected.

Additionally, I think if we move forward with this, we're going to need to take some time to consider the best workflow for creating and editing placeholder links. Should there be a checkbox only in the add pane? Or should we just allow adding custom links with no URL? Or what using about special URL placeholder (heh) like #. Or what about a menu item setting?

[DrewAPicture]

Replying to DrewAPicture:

However, I have some concerns, primarily with introducing a core feature that requires theme styling changes to work as expected.

More like "requires theme styling changes to make non-links appear as such". The non-links will "work", but the cursors will likely confuse people without the change.

[celloexpressions]

Replying to DrewAPicture:

Additionally, I think if we move forward with this, we're going to need to take some time to consider the best workflow for creating and editing placeholder links. Should there be a checkbox only in the add pane? Or should we just allow adding custom links with no URL? Or what using about special URL placeholder (heh) like #. Or what about a menu item setting?

The problem with # is that it implies click-ability like any other link (which may be desirable for some themes), as opposed to being merely an placeholder. # will also cause issues in themes where menus aren't at the top of the page (including fixed navbars like Twenty Fourteen has), because of the scroll-to-top functionality.

I'm definitely interested in more opinions on the best workflow for adding and managing placeholders. I like treating it as its own type once added, like the patch does, but there're certainly other options.

[celloexpressions]

This ticket remains a fairly-common need for users. Core (and the web) would benefit from a consistent and accessible mechanism to "headings" in menus, that break out of link elements entirely and allow users to semantically create submenus without top-level links.

Let's step back and approach this as a broader need for an accessible, core mechanism for this common use case in building menus. The associated markup changes (along with any other changes to menu markup that could improve semantics and accessibility) can be coordinated with theme back-compat via the use of a theme-supports flag (potentially under the existing html5 array of updated-markup features).

13273.diff​ may have reusable elements for the admin UI changes. These would also need to be implemented in the customizer menus panel.

[isabel_brison]

#52119 was marked as a duplicate.

[NekoJonez]

#53601 was marked as a duplicate.

[joedolson]

This behavior is now available in the navigation block, but that doesn't solve the problem for a lot of sites using classic themes. There are a few issues to solve here:

1) Core could add a setting for the menu to make top level items into buttons. Option text might need some careful thought.
2) In keeping with the historic practice of the navigation menu builder, core should not enqueue any scripts or CSS to handle this.
3) Therefore, this should be behind a theme support flag, so that a supporting theme can provide appropriate CSS and scripting to support the option.

I think this is doable, and would be of benefit to users. It offers an option where you can get the output like a block theme - with top level buttons - but get more control over the scripting that handles it.

[joedolson]

Going to give this a try.

[sonaliprajapati]

Menus:

Go to Appearance > Menus

In the left panel, click Custom Links

In the URL field, enter # (just a hash)

In the Link Text field, enter the menu title (e.g., Links)

Click Add to Menu

Expand the menu item and remove the # if desired, or leave it — but you may also:

Optional enhancement with jQuery:
method :1
=========
jQuery(function($){

$('a[href="#"]').on('click', function(e){

e.preventDefault();

});

});

Method :2

.menu-item a[href="#"] {

pointer-events: none;
cursor: default;

}

[nikunj8866]

Instead of immediately displaying the error message "Please provide a valid link." when a user submits an empty URL for a custom menu item, we could improve the UX by offering a confirmation prompt. The confirmation could inform users that a blank URL will result in a non-clickable menu item (useful for placeholder or parent menu purposes), and ask: The URL is empty. This will create a non-clickable menu item. Are you sure you want to proceed?

This change would provide more clarity and flexibility for users intentionally adding placeholder menu items.

[oglekler]

Group Titles

[oglekler]

I am suggesting adding Group Titles (check the screenshot above) that will not be links, it will be clear for users, and there is no need to add any dialogues. Right now we are working around by placing # instead of links, that is not the ideal solution. Instead of a tag can be span.

[joedolson]

I like the idea of adding a new object type that can be treated differently; it might make implementation easier.

However, I still think that there's a lot to figure out in this. What the object might become is going to depend on where it is in the menu - on the top level, it would need to be a button, within a menu it might need to be something else entirely, and that could depend on whether it's a parent within the submenu or not.

Thinking about implementations, in a submenu it could either need to be a heading or a trigger to expand another submenu.

While I'm intrigued by this, I'm wondering whether it's practical - it's a lot of work to implement, and in the meantime the block editor will be moving towards a more significant mega menu editing option.

This would provide something for classic themes, but there are already many different solutions for the problem out there. Before we can really start implementing, we'd have to have some clear ideas of what this is going to accomplish.

Are we aiming to enable mega menu structures within the classic menu editor, but without the scripting to control them, or are we going to be more conservative about what options the settings allow?

[joedolson]

Punting this to Future Release. There are too many open questions to resolve and have ready in four weeks.

Trac ticket: https://core.trac.wordpress.org/ticket/13273

[tusharaddweb]

The repeated requests and duplicate tickets suggest this is a common use case that would benefit from a core solution rather than relying on # links or other workarounds.

I support introducing a dedicated menu item type (e.g., "Group Title" or "Placeholder") instead of overloading Custom Links. This would make the intent clearer for users and provide a better foundation for handling accessibility and semantics appropriately in different menu contexts.

[shailesh7714]

We understand the need for “non-clickable” parent menu items that act purely as group headers for sub-items (like your “Links” example). This is a common navigation pattern, and your proposed solution (a simple checkbox to disable URL validation and adjust HTML output accordingly) is aligned with how we’d likely approach it as well.

We’ll add this to the feature considerations for the 3.0 release. Specifically, we’ll evaluate:

Support for menu items without URLs (null/empty link support)
A UI option to mark an item as a “parent / non-clickable”
Proper rendering so it still outputs as a menu label but not a hyperlink

Appreciate you taking the time to explain the requirement so clearly — it helps a lot in prioritizing and shaping features.

<ul>

<li><a href="/">Home</a></li>
<li><a href="/something-else">Something else</a></li>

<li class="menu-parent">

<span>Links</span>
<ul>

<li><a href="​https://example.com/1">External link 1</a></li>
<li><a href="​https://example.com/2">External link 2</a></li>

</ul>

</li>

</ul>

Merge after #13273, which accounts for three of the failing tests.

• [ ] self-closing non-HTML elements
• [ ] remove opening tag when required attributes are missing, and closing tag
• [ ] replace C0 controls with their escapes, rather than stripping them away

I'm supportive of this direction overall. I'll leave some feedback on different points.

I started this review at 0b1535e0f5974562a364fc6298a3fb87b5a9bb5e but it moved while I was reviewing, so some of this review may be stale by the time I post.

---

General things to debug:

echo wp_kses( '<template>foo</template>', [ 'template' => true ] );
// Prints: </template>

echo wp_kses( '<div style>x</div>', [ 'div' => ['style' => true] ] );
// Prints: <div style="1">x</div>

echo wp_kses( '<div style="col&#x01;or:red">x</div>', [ 'div' => [ 'style' => true ] ] );
// Before (trunk): <div>x</div>
// After         : <div style="color:red">x</div>

---

This is an interesting one, the block delimiter is preserved (improvement) but for some reason wp:image becomes wp:core/image:

echo wp_kses( '', [] );
// Before: <!-- wp:image {"alt":"x"} -->
// After:

---

_Incomplete tokens_ are interesting.

KSES would leave something like a trailing <b this is lost as HTML text (it would escape <). Browsers do not do that and this PR does not do that, the <b… is dropped.

That means that partial tokens like the unclosed <b above but also "atomic elements" are stripped entirely:

echo wp_kses( '<p>keep me</p><b this is lost', ['p'=>[],'b'=>[]] ) . "\n";
echo wp_kses( '<p>keep me</p><title>this is lost', ['p'=>[],'title'=>[]] ) . "\n";
echo wp_kses( '<p>keep me</p><style>*{content:"this is lost";}', ['p'=>[],'title'=>[]] ) . "\n";
echo wp_kses( '<p>keep me</p><style>*{content:"this is lost";}', ['p'=>[],'style'=>[]] ) . "\n";
echo wp_kses( '<p>keep me', ['p'=>[]] ) . "\n";

Before/after diff of output:

.txt

@@ -1 @@
-<p>keep me</p>&lt;b this is lost
-<p>keep me</p><title>this is lost
-<p>keep me</p>*{content:"this is lost";}
-<p>keep me</p><style>*{content:"this is lost";}
+<p>keep me</p>
+<p>keep me</p>
+<p>keep me</p>
+<p>keep me</p>
 <p>keep me

This is worth considering carefully. The HTML snippets KSES sees don't necessarily align with coherent boundaries. In particular echo wp_kses_post( 'a<b', [] ); is a<b on trunk (which seems reasonable) but on this branch its a.

We _could_ align with the previous behavior, assume that the incomplete tokens are HTML text, and escape them as such. There may be negative implications to that, it's worth some careful consideration.

---

Foreign content is difficult, is this a case of integration points bailing because they're risky?

echo wp_kses( '<math><mtext>x</mtext></math>', ['math'=>[],'mtext'=>[]] );
// (no output)

---

</br> produces warnings.

echo wp_kses( '</br>', [ 'br' => true ] );
// Warning: foreach() argument must be of type array|object, true given in /var/www/html/wp-includes/kses.php on line 1499
// Warning: foreach() argument must be of type array|object, null given in /var/www/html/wp-includes/kses.php on line 1536
// Prints: <br>

First warning is the allowed tags. I used true for the allowed tags value.

This change produces a warning where before it did not:
​https://github.com/WordPress/wordpress-develop/blob/0b1535e0f5974562a364fc6298a3fb87b5a9bb5e/src/wp-includes/kses.php#L1499

Maybe true is intentionally supported:

​https://github.com/WordPress/wordpress-develop/blob/049287aeda8855d6c29af747839377d7669a9fe6/src/wp-includes/kses.php#L1533-L1537

Either way, it's filterable and important to to follow whatever KSES did previously. I think if the value is true it should use an empty array (no attributes allowed).

Second warning is here:

​https://github.com/WordPress/wordpress-develop/blob/0b1535e0f5974562a364fc6298a3fb87b5a9bb5e/src/wp-includes/kses.php#L1536

As it is now, this needs a null guard. I had proposed a fix at the tag processor level in ​https://github.com/WordPress/wordpress-develop/pull/9657 that we could consider.

@sirreal this is great. thanks for the awesome review and findings.

• the TEMPLATE issue was silly; the code was decrementing the depth before processing the closing tags. I’ve resolved that by splitting the depth adjustment to before for openers and after for closers. TEMPLATE is still off by policy. we can open it up, but I haven’t done that yet, as I think it’s probably reasonable to not allow user-provided inputs to add them. for example, to supplant some TEMPLATE that might already be on the page otherwise.

• style="1" had to do with calling get_attribute() manually rather than relying on the normalized-to-string value. I’ve fixed that and also only run through style processing if the value is non-true. this also removes the C0 controls before processing.

• style="color;red" this seems like one of the many kinds of improvements occurring with this rewrite. what about this should be debugged?

• but for some reason wp:image becomes wp:core/image: that’s because it’s being re-serialized and the Block Processor returns a fully-qualified name. I wouldn’t call this a bug, but we could consider removing the core/, and potentially via a new method on the block processor like ->get_minimal_block_type(). this only happens on block delimiters whose JSON attributes are modified.

• for incomplete tokens I still consider this an improvement, and it goes back to our original rationale for pausing at incomplete tokens. in a browser, this stuff is even ignored. we can look at an example like a<b and think it makes sense to escape, but what about <!-- wp:para or <div class="wp-fullwidth attachment-id-15" data-wp-bi and such? those look far more like tags and comments that _should not_ be escaped and revealed on the page. in the case of the block delimiter, it would invite disclosure of something that was perhaps meant to be private on the server. thus, in the absence of any way to know what would have come next, truncating as a means of inevitable data loss feels preferable to showing syntax.

• the </br> case has been fixed. I’m not sure why it arose for that one in my testing and not for other tags. added if ( is_array() ) guards.

wp:image becomes wp:core/image

I was confused here because I expected it to happen across core blocks, but I didn't observe the same behavior with the paragraph block. I may not have been testing appropriately.

<div style="col&#x01;or:red">x</div>

I called this out just to make sure we know what's different and understand the implications. Was this passed through safecss_filter_attr() before and is it now? What explains the different handling?

• <div style="col&#x01;or:red">not red</div> is not red, but I guess the control character removing KSES does makes it <div style="color:red">not red</div> which _is_ red and makes sense to keep if we're removing control characters. (Note, this has : in the declaration, your reply had a typo ; between property and value).
• <div style="col&#x01;or;red">x</div> (with the ; instead of :) seems objectively better. Before it became or;red, now it's color;red which makes more sense. I suspect the lingering problem that there's no valid CSS declaration is a problem at another layer we can eventually address with improved CSS handling.

for incomplete tokens I still consider this an improvement

I _mostly_ agree that removing it is the right thing to do. I think it would be appropriate to escape the incomplete token as HTML in sometimes and drop it others, but that would require a defining where the incomplete token parsing and a set of rules for how to deal with each case. Defining those rules will require some arbitrary decisions that are likely to be surprising as often as not.

One important change that's worth worth documenting clearly is that _some_ elements must be properly closed, while _other_ elements do not need to be closed. This is especially important since we've seen examples where the KSES input starts with end tags. That implies that other KSES input may ends with unclosed tags.

Again, I think on the whole these are probably _good_ to remove, but may still be _surprising_. An example that lists the "atomic elements" defined by the tag processor and a few examples should help with this, e.g. <div>Everything after this is removed:<title>gone</div>

core/image vs. image

I’ve normalized by removing the core/ namespace prefix.

<template> is always removed, is that correct? The Interactivity API often relies on template elements. It may be unusual for those to come from untrusted sources, but I'm concerned about blocking it entirely.

I’ve re-gated this to only block when TEMPLATE isn’t configured. In the earlier drafts, it was qualitatively different, but now, we are tracking foreign content and template content separately from how we track skipping content.

A major concern is that MathML is extremely restricted, most MathML markup I've tested is stripped because it quickly encounters integration points. I think this needs a more permissive solution because as-is it disallows a lot of potentially valid and conforming content that would previously pass.

That’s something we can iterate on. Fundamentally it’s only about how much accounting we duplicate here. I’d rather start more-locked-down and relax the rules as we can.

HTML character references are normalized. Often this is fine, but it can produce shortcode syntax where HTML character references avoided it.

Seems reasonable enough to escape, but also, the appropriate way to “escape” shortcodes is with the double-bracket syntax, like [[gallery]].

wp_remove_unwanted_c0_controls() seems to run on text nodes but not the contents of atomic elements. Those are mostly the same category so it's worth considering whether it should be applied there or not.

I really don’t like this. It gets really confusing when we start applying it to special atomic elements, which have their own unique escaping rules. I still think this was probably the wrong layer at which to address the problem: if we can audit XML generation we should be free to remove C0-controls-removal as it was originally intended to prevent generating broken XML.

One thing I'd consider is to completely remove known void HTML elements that don't satisfy the required attributes. That seems to align with the intended behavior.

This is what it’s supposed to be doing already, and I think it does. Are you seeing it fail to do that?

The problem with the existing behaviors and tests is that OBJECT is _not_ a void element, and wp_kses() is looking to the errant self-closing flag as a proxy for that.

If you change your test/example code to a void or special atomic element, or a self-closing foreign element, then it should fully remove it.

This should inject newlines into some HTML elements

Do you think this should be an easy thing to resolve? Just ensure that the modifiable text always starts with a newline? Or is the problem that we’re pushing out the decoded text and what we should be doing is building a custom <pre>#text</pre> and then chopping off the <pre> and </pre>?

In both cases, I'd expect the block name to remain unchanged. This is likely limited to Core blocks.

This is technically true; the serialization changes, but the block name remains unchanged. I’ve added the aesthetic change, but they are semantically equivalent. I understand the desire for aesthetics, but I’m not scoping this to require byte-identical idempotence. wp_kses() is likely not even idempotent in many places this isn’t. This is a good goal, but I would not want us to stall the work simply because of neutral variations.

That's the situation right now on trunk

This is also the scope-level for merging this work. There are lots of things I don’t like about wp_kses() legacy behavior, but I don}t want to demand that we fix everything at first. After this change, these things _should_ be considerably easier to update than they have been, since we have a structural approach to take here.

---

pushing updates in bfedf65535 which address your points unless stated otherwise.