backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 40 forks source link

Views need to update and delete entries in menu_links. #1737

Open bd0bd opened 8 years ago

bd0bd commented 8 years ago

Hi!

I have created View page Article with Cyrillic path - Статьи (Статьи = Articles). Then I was going to rename first capital letter - C - to small one - с. But the actual URL address could not be renamed, it remains Статьи and not статьи.

Well, I have deleted this View page and created a new one but still URL address begins with capital С when I click menu item even if I use only small с everywhere in the View settings-configs.

BD 1.3.4

ttttttttttb

654 6

bd0bd commented 8 years ago

The only solution (for me) was to go to the database and manually change link_path. Not sure if I need to change router_path too.

bd0bd commented 8 years ago

Is it a bug?

ghost commented 8 years ago

Cannot confirm, all works fine. I can rename path from Статьи to статьи in Views.

ghost commented 8 years ago

@bd0bd : you are confused by the fact that Edit link shows the menu title under the label Path, if the path is "owned" by a module (views here), and thus not editable in this form (actually, it's the menu title themed as a link to the path - it's intended to be clickable…)

Feature request : [UX] The Path label should be re-worded as just Menu link ?

bd0bd commented 8 years ago

Cannot confirm, all works fine.

Not fine. Impossible to rename it. Only directly in the database it was possible for me.

Menu item Статьи should be linked to статьи, but it was linked to Статьи even after I have deleted the view page and created it again with статьи everywhere in views settings.

you are confused by the fact that Edit link shows the menu title under the label Path

No, not confused now. I talk only about the menu item Статьи - it links to Статьи (domain.com/Статьи) but should be linked to статьи (domain.com/статьи) .

:smile:

bd0bd commented 8 years ago

It is very funny! Right now I can not change it (menu URL link) from статьи to Статьи. I have deleted the view page (with URL статьи) and created a new one (with URL Статьи). But menu item Статьи still links to статьи. It should link to Статьи. Its a miracle!

55

default

bd0bd commented 8 years ago

I can rename path from Статьи to статьи in Views.

I can do it too. But the problem is with the menu item URL link - it does not link to correct address.

bd0bd commented 8 years ago

Feature request : [UX] The Path label should be re-worded as just Menu link ?

Of cause the current name is incorrect and should be re-worded I think. But the problem remains - impossible to rename URL link from capital to small letter or from small to capital.

bd0bd commented 8 years ago

This is 100% bug.

You can rename Article to article and then article to Article. But it is impossible to do the same with cyrillic статья to Статья and then Статья to статья.

I think the bug is with cyrillic letter с.

quicksketch commented 8 years ago

I think the bug is with cyrillic letter с.

Is the Cyrillic c or C different from the English one? I just checked this. Indeed typing a upper or lowercase C on an English keyboard is a different underlying character code, even though they appear identical. I've been copy/pasting the letters from this issue for testing.

Here is my attempt at reproducing this:

So it looks like everything is working the way it is supposed to. However, I found that it is still possible to access the same URL with or without the capitalization. I can access the page at both http://backdrop.dev/статья and http://backdrop.dev/Статья. The menu link itself always uses the proper capitalization.

So this is something a little unexpected. It looks like URLs in Backdrop are not case-sensitive. I can visit /node/1 or /NoDe/1 and they both work. I don't recall having made this change, so it either was unintentional or done pre-forking from Drupal. Never mind, turns out this is the case in D7 as well.

Aside from the URLs being case-insensitive, I couldn't reproduce the issue at all with the menu link being incorrect. Changing the menu link in Views immediately updated the menu link on the front-end to be the correct value each time, as shown in the screenshots.

quicksketch commented 8 years ago

This is 100% bug.

BTW, @bd0bd I invited you to the "Docs and Organization" team, so you can now add labels to issues.

bd0bd commented 8 years ago

So it looks like everything is working the way it is supposed to.

No. It does not work the way it is supposed to :smile: Where I can send you login and password from the site so you could see it yourself?

UPDATE: I have sent it to your quicksketch.org address. Now you can see this problem yourself.

BTW, @bd0bd I invited you to the "Docs and Organization" team, so you can now add labels to issues.

Thank you! But I am not ready to take this responsibility. It is better for me to continue in the way as it is now.

quicksketch commented 8 years ago

@bd0bd I couldn't reproduce the issue on your site either

There must be some difference between what I'm trying and what you're trying.

But I am not ready to take this responsibility.

No responsibility involved, it just allows you to label any issues you find as either bugs or features. :smile:

quicksketch commented 8 years ago

Oops, sorry it should have occurred to me you didn't want the URL shared. Removed.

bd0bd commented 8 years ago

No responsibility involved, it just allows you to label any issues you find as either bugs or features.

I understand but sometimes I could be wrong with my issues like it might be now when I write here This is 100% bug.

quicksketch commented 8 years ago

Hm, strange. So on your site @bd0bd, I can get the problem but only for the exact path статьи. If I make the path anything else, such as статьи-2, I can uppercase the C without any problem.

I think there's something weird with the menu links in your database table. Could you try the following database queries?

delete from menu_links where router_path = 'статьи';
delete from menu_router where router_path = 'статьи';

Then clear your caches from the admin bar. The menu link and router item will be automatically regenerated.

bd0bd commented 8 years ago

I couldn't reproduce the issue on your site either

I do not understand why but it does not work for me.

It is impossible to rename menu item URL from capital letter С to small с.

The only way to solve it - to edit it in the database.

bd0bd commented 8 years ago

such as статьи-2, I can uppercase the C without any problem.

Yes, you are right ! It is very strange!

bd0bd commented 8 years ago

I think there's something weird with the menu links in your database table. Could you try the following database queries?

I did it.

SOLVED! Everything is ok now with the menu item. Thank you very much!

bd0bd commented 8 years ago

OMG!

The problem is again here! Views path is Статьи but the menu item URL is статьи.

quicksketch commented 8 years ago

Could you try the following queries and post the results?

select path, page_callback, type from menu_router where path = 'статьи';
select link_path, router_path, link_title, module from menu_links where link_path = 'статьи';

My theory is that there is a menu link in the database that has a different capitalization for the path статьи. For some reason that link isn't showing up in the user interface. But when your view is configured to be at that path, the stray menu link is found in the database and starts getting used. Because MySQL is not case-sensitive, it's finding this lower-case path instead of the upper-case one.

bd0bd commented 8 years ago

I think it is better if I give you access to the DB. I have already sent it to you.

quicksketch commented 8 years ago

Okay, here's the full steps to reproduce. It's not a an issue with Cyrillic characters, it's a case-sensitivity issue that affects all characters:

Workarounds:

I'm not sure whether this is a bug in Views or in the Menu system, but now we know how to reproduce it! Yay, thanks @bd0bd for reporting the issue. I hope the above work-arounds can be used until we can fix the problem in core.

bd0bd commented 8 years ago

Thank you very much! I am happy you was able to reproduce it :smile:

bd0bd commented 8 years ago

Delete the menu link from within the view

How to delete it? I do not see this option in view /admin/structure/views/view/articles

ghost commented 8 years ago

Now I can reproduce it too.

If you create a page in Views and give it a path, Views creates an entry in menu_router. And if you give it a menu link, Views creates an entry in menu_links and update the entry in menu_router.

if you now edit this menu link in Menu UI, the entry in menu_links will be updated and marked as customized.

if you want to edit this menu link in Views now, you will see the old values of the menu link. It's the value from the entry in menu_router. Menu UI only updates entries in menu_links.

Any changes of these menu link will only update the entry in menu_router. But not in menu_links because its marked as customized. Then its "locked" for Views, and Views cannot update it. Its the same for the path if you only lower- or upper-case it.

The first one If you lower- or upper-case a path in Views e.g. from article to Article, the Menu API uses the path from the customized, "locked" entry in menu_links for link building e.g. article. It works because the path matching is case-insensitive. And that can confuse a lot.

If you create a new path for the page in Views, the old entry in menu_router will be deleted and a new one will be set. The same in menu_links if the entry is not marked as customized.

If its marked as customized, menu_links keeps the old entry and set a new one. The old entries are orphaned now, they don't have a matching route/system path in menu_router. But they still in the database.

Another one If you create another page in Views, and use a route of an orphaned entry as path, you get this entry as menu link of your page. And that can confuse a lot, too.

Last one If you want to edit a customized, "locked" menu link in Views, you have to reset it in the Menu UI. But you can do it only if the path is exactly the same because now the path matching is case-sensitive, otherwise you get a PDOException.

If you get an error, go back to Views and set the path to the value you can see in the Menu UI.

@bd0bd In your case, I would reset all menu links Views pages in the Menu UI. And only create, update and delete menu links in Views to prevent these troubles. And until there is a better solution. I would also delete orphaned entries in menu_links.

Solution For nodes you can create , update, and delete a menu link in the node form ui as well as in the menu ui without any problems. I would like to see it in Views too.

Views can implement hook_menu_link_insert, _update, _delete to update the menu link of a view. Views also must be able to update a customized entry and prevent orphaned entries in menu_links

I hope this helps to unravel the nasty trouble.

bd0bd commented 8 years ago

For nodes you can create , update, and delete a menu link in the node form ui as well as in the menu ui without any problems. I would like to see it in Views too.

Me 2.

In your case, I would reset all menu links Views pages in the Menu UI.

It is impossible. I have this error:

Notice: Undefined index: Статьи in menu_reset_item() (line 433 of /home/www/domain.com/core/modules/menu/menu.module). Notice: Undefined index: title in _menu_link_build() (line 2821 of /home/www/domain.com/core/includes/menu.inc). Notice: Undefined index: path in _menu_link_build() (line 2822 of /home/www/domain.com/core/includes/menu.inc). PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'link_path' cannot be null: UPDATE {menu_links} SET menu_name=:db_update_placeholder_0, plid=:db_update_placeholder_1, link_path=:db_update_placeholder_2, router_path=:db_update_placeholder_3, hidden=:db_update_placeholder_4, external=:db_update_placeholder_5, has_children=:db_update_placeholder_6, expanded=:db_update_placeholder_7, weight=:db_update_placeholder_8, depth=:db_update_placeholder_9, p1=:db_update_placeholder_10, p2=:db_update_placeholder_11, p3=:db_update_placeholder_12, p4=:db_update_placeholder_13, p5=:db_update_placeholder_14, p6=:db_update_placeholder_15, p7=:db_update_placeholder_16, p8=:db_update_placeholder_17, p9=:db_update_placeholder_18, module=:db_update_placeholder_19, link_title=:db_update_placeholder_20, options=:db_update_placeholder_21, customized=:db_update_placeholder_22 WHERE (mlid = :db_condition_placeholder_0) ; Array ( [:db_update_placeholder_0] => main-menu [:db_update_placeholder_1] => 0 [:db_update_placeholder_2] => [:db_update_placeholder_3] => [:db_update_placeholder_4] => -1 [:db_update_placeholder_5] => 0 [:db_update_placeholder_6] => 0 [:db_update_placeholder_7] => 0 [:db_update_placeholder_8] => 0 [:db_update_placeholder_9] => 1 [:db_update_placeholder_10] => 334 [:db_update_placeholder_11] => 0 [:db_update_placeholder_12] => 0 [:db_update_placeholder_13] => 0 [:db_update_placeholder_14] => 0 [:db_update_placeholder_15] => 0 [:db_update_placeholder_16] => 0 [:db_update_placeholder_17] => 0 [:db_update_placeholder_18] => 0 [:db_update_placeholder_19] => system [:db_update_placeholder_20] => [:db_update_placeholder_21] => a:0:{} [:db_update_placeholder_22] => 0 [:db_condition_placeholder_0] => 334 ) in menu_link_save() (line 3220 of /home/www/domain.com/core/includes/menu.inc).

ghost commented 8 years ago

If you want to edit a customized, "locked" menu link in Views, you have to reset it in the Menu UI. But you can do it only if the path is exactly the same because now the path matching is case-sensitive, otherwise you get a PDOException.

If you get an error, go back to Views and set the path to the value you can see in the Menu UI.

In Menu UI click on edit of the menu link. Copy the path and paste it in the View path. Save the view and now try to reset the menu link in the Menu UI

bd0bd commented 8 years ago

But you can do it only if the path is exactly the same because now the path matching is case-sensitive, otherwise you get a PDOException.

I am sorry for my inattention. Thank you!

klonos commented 8 years ago

I just tried creating a normal menu item from a view. When I remove the item from within the Views UI (click the menu and select "No menu entry " -> save view), the menu item is removed from the view but not from the menu. I would expect it to be removed too or at least to be asked what I want to do about it. Also, the only option in the menu edit UI is to edit this item (+ the disable checkbox of course) - no option to delete it though. So you are left with orphan menu items that cannot be deleted. On top of that, these menu items do not have editable paths, so you cannot edit that either.

Wouldn't it make sense to have some sort of link/relationship between the views-generated menu items and their parent view? This way, one could update their title, path, description text and weight from either end.

A "views-generated" note next to these items in the menu UI would help too (the same way we have the "module-provided" note for layouts for instance).

Last thing (as a note), the menu item on the menu UI has a "Parent link" drop-down option that allows it to be moved in various menus and in various depths, while in the views UI the respective drop-down is one with a "Menu" label that allows only top-level menus to be selected as parents.

klonos commented 8 years ago

...so what is the deeper issue here? Not handling capitalization correctly, or not having the menu items settings being "interconnected" between views and the menu system?

bd0bd commented 8 years ago

I think both issues should be fixed :smile: But personally for me the issue with capitalization problem is not very important now because I know how to solve it.. Thank you!

ghost commented 8 years ago

Maybe this can help abit https://github.com/rstamm/backdrop-analyzer

quicksketch commented 8 years ago

...so what is the deeper issue here? Not handling capitalization correctly, or not having the menu items settings being "interconnected" between views and the menu system?

@rstamm laid it all out on a technical level. In summary, Views needs to update the entries in the menu_links table when a menu setting for a view is changed.

When I remove the item from within the Views UI (click the menu and select "No menu entry " -> save view), the menu item is removed from the view but not from the menu.

Besides accommodating for changes like capitalization, if the menu item is deleted it would make sense to remove the menu link as well. Right now Views just isn't concerning itself with menu links at all.

klonos commented 8 years ago

Fair enough. Should we handle the capitalization issue here and file a separate issue for the menu/views interconnection or handle everything here?

klonos commented 8 years ago

...BTW, I think that if one has previously added a menu item through a view, then if they edit the view and select "No menu entry", there should be a question asking them what they want to do with the existing menu item. In the menu edit UI we give users two options: either delete or disable (using the checkbox). We ask for confirmation for deletion of individual menu items, but not for simply disabling them. Perhaps in views, it also makes sense to offer either removing completely or simply disabling the respective menu item.

ghost commented 8 years ago

I just tried creating a normal menu item from a view. When I remove the item from within the Views UI (click the menu and select "No menu entry " -> save view), the menu item is removed from the view but not from the menu. I would expect it to be removed too or at least to be asked what I want to do about it. Also, the only option in the menu edit UI is to edit this item (+ the disable checkbox of course) - no option to delete it though. So you are left with orphan menu items that cannot be deleted. On top of that, these menu items do not have editable paths, so you cannot edit that either.

Yes, right. But in this case I wouldn't say its an orphaned menu link coz the path of this link is still valid, the menu link still works fine. To remove the menu link completly, you have to set a new path in the view and save it, then it will be removed from the database. After that you can switch back to the old path.

...so what is the deeper issue here? Not handling capitalization correctly, or not having the menu items settings being "interconnected" between views and the menu system?

Its not only capitalization. Its upper- or lowercase of one or more characters of a path.

ghost commented 8 years ago

@rstamm laid it all out on a technical level. In summary, Views needs to update the entries in the menu_links table when a menu setting for a view is changed.

Yes, update and delete entries in menu_links.thats the main issue.

klonos commented 8 years ago

Yes, right. But in this case I wouldn't say its an orphaned menu link coz the path of this link is still valid, the menu link still works fine.

Agreed. but the menu link was not created individually from within the Menu UI. It was added by a view through the Views UI and although not technically an "orphaned" menu item, it is a "handicapped" one: you cannot remove it from the menu UI - you can only disable it and you cannot edit its path:

backdrop-edit_menu_link-added_trhough_the_views_ui

...as you can with menu items added through the Menu UI:

backdrop-edit_menu_link-added_trhough_the_menu_ui

klonos commented 8 years ago

...so you are left with an orphan menu item that "works" when it comes to the front-end (clicking it takes you to a URL that works), but what happens if you remove the view that created it (and thus the URL that used to work)?

klonos commented 8 years ago

...scratch that. If you delete the view, then the menu item is actually removed. Good!

The text when we remove a view is this:

Deleting the view [view name] will remove all pages, blocks, and other displays provided by this view.

We should update it to say "...as well as any menu items provided by this view".

klonos commented 8 years ago

...the fact remains though that if a menu item created by a view is removed from within the view UI, it simply gets "unlinked" and there is no way to delete it from the menu UI or edit its path.

When you edit the view path though, the menu item path is updated and it leads to the new path.

ghost commented 4 years ago

@klonos Sorry I haven't taken the time to read through this issue, but just wondering if it's still relevant...?