Make WordPress Core

Opened 12 years ago

Last modified 6 weeks ago

#24146 accepted defect (bug)

Menu items with blank labels are removed on saving

Reported by: rodrigowebjumpcombr's profile rodrigo@… Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: normal Version: 3.5.1
Component: Menus Keywords: dev-feedback has-patch needs-testing
Focuses: Cc:

Description (last modified by SergeyBiryukov)

Hello,

When edit an item of menu leaving the label in blank, the item is auto deleted.
There are two problems in that:

  1. I could need an item with blank title to add some class with an background image
  2. When this item has subitems with two or more depth, all subitems are moved to first depth loosing submenus hierarchy. Moreover if i try to drag the subitems to make the hierarchy again, after save, all subitems come back to first depth. While i not change the depth of first item this issue occurs again.

Best regards

Attachments (5)

24146.patch (2.5 KB) - added by vinod dalvi 11 years ago.
Patch Added.
24146-2.patch (1.8 KB) - added by LeoPeo 10 years ago.
revision of patch 2
24146.diff (1.8 KB) - added by MikeHansenMe 10 years ago.
Refresh plus code standard formatting updates
24146.2.diff (1.9 KB) - added by SergeyBiryukov 10 years ago.
24146.3.diff (2.0 KB) - added by vinod dalvi 8 years ago.

Download all attachments as: .zip

Change History (29)

#1 @rodrigo@…
12 years ago

  • Cc rodrigo@… added

#2 @johnbillion
12 years ago

  • Component changed from General to Menus

#4 @SergeyBiryukov
11 years ago

Related: #23051, #25214.

I had the impression that it's a duplicate of #23051, but it's not. Menu items with blank titles are still deleted on saving.

#5 @SergeyBiryukov
11 years ago

  • Description modified (diff)
  • Summary changed from Auto delete of blank labels menu itens to Menu items with blank labels are removed on saving

#6 @SergeyBiryukov
11 years ago

#25218 was marked as a duplicate.

#7 @vinod dalvi
11 years ago

  • Cc mozillavvd@… added
  • Keywords has-patch needs-testing 2nd-opinion added

Patch Added.

@vinod dalvi
11 years ago

Patch Added.

@LeoPeo
10 years ago

revision of patch 2

#8 @LeoPeo
10 years ago

Hello,
I've tested the patch 24146 and it works!

Right now, this is what I see:

https://www.dropbox.com/s/bzd5j6i5g0yprml/24146.png?dl=0

WP: 4.0
Language:Italian

As you can see elements has no label and also element heigh is affected.

I've patched in this way:

1) If $title is not set, $title: (no label)
2) If $title is not set, I add to .menu-item-title the class "no-title" so it's styled like you delete the title in preview.

Last edited 10 years ago by LeoPeo (previous) (diff)

#9 @SergeyBiryukov
10 years ago

#29729 was marked as a duplicate.

#10 @SergeyBiryukov
10 years ago

  • Milestone changed from Awaiting Review to 4.1

#11 @SergeyBiryukov
10 years ago

  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

#12 @johnbillion
10 years ago

  • Keywords 4.2-early added; needs-testing 2nd-opinion removed
  • Milestone changed from 4.1 to Future Release

#13 @iseulde
10 years ago

  • Milestone changed from Future Release to 4.2

has-patch 4.2-early so moving to 4.2.

@MikeHansenMe
10 years ago

Refresh plus code standard formatting updates

This ticket was mentioned in Slack in #core by drew. View the logs.


10 years ago

#15 @DrewAPicture
10 years ago

  • Keywords needs-patch added; has-patch 4.2-early removed

In testing 24146.diff, I noticed a couple of standout issues:

  • With the patch applied, the menus screen is tossing a notice:
    PHP Notice:  Undefined variable: no_title_class in /srv/www/core/mutrunk/src/wp-admin/includes/nav-menu.php on line 109
    
  • Creating and saving ordinary menu links with no label works as expected, though it looks like the same is not true for other types of menu items. Removing labels on category items, for instance, the label is simply restored on save.
Last edited 10 years ago by DrewAPicture (previous) (diff)

#16 @DrewAPicture
10 years ago

The latest patch gets us part-way toward a fix here, just need to new patch addressing the feedback in comment:15. We'll need the new patch pretty soon to remain in consideration for 4.2 inclusion.

#17 @SergeyBiryukov
10 years ago

24146.2.diff fixes the notice and uses an existing string, but still doesn't look like a complete solution.

As noted in comment:15, if you remove the label for a post type or category item, the change is not saved and the label is restored, making the first part of the patch irrelevant.

Looks like 24146.patch attempted solve this, but it no longer applies cleanly and seems to break the labels of existing menu items when those changes are recreated manually. Also need to take [24560] into account.

Version 0, edited 10 years ago by SergeyBiryukov (next)

#18 @alexgarces
10 years ago

  • Keywords dev-feedback added

You can make a blank label adding the string " " at the label field. Doing it this way, the item is not removed from the menu.

#19 @SergeyBiryukov
10 years ago

  • Milestone changed from 4.2 to Future Release

#20 @SergeyBiryukov
10 years ago

#22070 was marked as a duplicate.

#21 @ocean90
9 years ago

In 32895:

Customizer: Fix live preview for menu item titles.

Show also a default label for menu items without a label which are assigned to a menu. This is currently only supported in the Customizer, see #24146 for nav menus screen.

see #32576.

@vinod dalvi
8 years ago

#22 @vinod dalvi
8 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

Updated patch to make it work with latest code changes.

The patch doesn't handle customizer menu as it is handled in another ticket.

#23 @ElectricFeet
5 years ago

Reminder to self: check Trac exhaustiveley before writing out a bug report.

I'll add my text here anyway, as it elaborates on some of the reasoning behind this issue -- specifically giving a scenario where this happens. Reading through the various comments above and noting that it's taking so long to fix, you might want to consider at least taking approach B (below) to the problem. That is, at least warn the user you're about to delete their work before deleting it.

Original aim:

To create menu items that shows icons with no text.

Problem:

Menu items can be deleted with no warning.

Steps to reproduce problem:

  1. Create new menu
  2. Create two new custom links (for example to https://example.com), with Link Text test1 and test2, adding each to the menu
  3. Save the menu.
  4. On the Manage Locations tab, add this menu to a theme location you will be able to see and save your changes.
  5. View site and check that the two menu items are there.
  6. Go back to Edit Menus. If the CSS Classes box is not available, enable it from the Screen Options pull down (top right of screen)
  7. Open the test1 menu item and add fa fa-envelope to the CSS classes.
  8. Delete the navigation label and save the menu.

The menu item test1 disappears and there is no way to get it back. The site does not display it. There was no warning or other indication to the user that the item was to be deleted. If this has been done for a lot of menu items, a lot of time/work is lost.

I should point out that I know that I can add <span class="fa fa-envelope"></span> to the navigation label to get the result I need; my issue is with no warning of impending deletion if a user tries to do this in the way described above.

Recommendation:

  1. I recommend that the menu item should remain, with no navigation label (similar to what happens if you add &nbsp; to the menu). This allows the user to continue to modify it -- including to remove it, if that was the original intention. (It's very unlikely that users will want to delete menu items by removing the navigation label, when they have a red "Remove" link at the bottom.)
  1. If it isn't possible to implement A, then -- in the case that there are menu items without Navigation Labels when the user clicks Save Menu -- another approach would be to warn the user that menu items without Navigation Labels will be deleted, and allow them to cancel the save. That way, they will be able to re-instate the navigation labels and not lose all their menu items.

Personally, I much prefer A. It gives the user more flexibility, behaviour is not unexpected, and allows more creative use of pseudo elements / classes etc.

This ticket was mentioned in PR #7699 on WordPress/wordpress-develop by @nirajgirixd.


6 weeks ago
#24

## Description

This PR addresses the issue where menu items without labels were being removed.

## Changes Made

  • Remove check for empty labels for menu item
  • Add no-title class for empty label menu item

## Why?

  • Menu items were being deleted silently, without prompting the user to confirm whether the item should be removed.
  • Users couldn't create a menu item with an empty label, which they might want to use for custom purposes—for example, to display an icon using the fa fa-envelope class without any label text.

## Steps to Reproduce

  • Create a new menu.
  • Add two custom links (e.g., https://example.com/) with link text labeled "test1" and "test2," then add each to the menu.
  • Save the menu.
  • Assign this menu to a theme location that is visible on the site, then save your changes.
  • Visit the site to confirm that both menu items are displayed.
  • Return to the Edit Menus section, edit the "test2" menu item and remove the label and hit the save button.
  • The item will be removed.

## Trac ticket: https://core.trac.wordpress.org/ticket/24146

Note: See TracTickets for help on using tickets.