Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
After updating to a new version of HEAD, dropping my test database (and recreating another one) and installing a fresh copy of head, I went to the Modules page and enabled every core module. Then I went immediately to the /admin/build/menu-customize/navigation page, and moved to the second page from the pager.
The item "Sources" (from Aggregator) appears linked to the disabled "Search", as in the screenshot. However, it you edit "Sources" it shows its parent as "Feed aggregator" (screenshot attached).
Comment | File | Size | Author |
---|---|---|---|
#21 | no-pager-208498-21.patch | 1.57 KB | pwolanin |
#16 | menu-pager-208498-16.patch | 4.87 KB | pwolanin |
#17 | no-pager-208498-17.patch | 1.41 KB | pwolanin |
#13 | menu-pager-208498-13.patch | 4.09 KB | pwolanin |
#7 | menu_links_rebuild.patch | 1.13 KB | theborg |
Comments
Comment #1
keith.smith CreditAttribution: keith.smith commentedHuh. I can confirm this every time I try it:
- download and install a clean copy of HEAD with new database
- enable all core modules
- go to menus, navigation menu (note that menu items are displayed on two pages)
- go to second page of menu items
- note that "Sources" (from aggregator) appears under "Search"
Comment #2
theborg CreditAttribution: theborg commentedI can reproduce with a clean install and all core optional modules enabled.
Comment #3
webernet CreditAttribution: webernet commentedI can reproduce this as well - the menu_links table appears to be OK.
Probably related to fact that it's two pages, and the child item appears on a different page from the parent.
Also -- Note that 'Recent posts' is on the second page, when it should be near the top of the menu tree.
Comment #4
theborg CreditAttribution: theborg commentedMore info:
It displays more pages than are needed. To reproduce only activate 'aggregator'.
menu_overview_form
is passing an incorrect count of total items displayed topager_query
.
The pager cuts the rows based on the query and 'sources' is affected.
Comment #5
theborg CreditAttribution: theborg commentedComment #6
pwolanin CreditAttribution: pwolanin commentedif the data is OK, but it's displayed wrong it may not warrant marking this critical, though it may totally ruin Drag-n-drop functionality
Comment #7
theborg CreditAttribution: theborg commentedThis is a *very experimental patch* (waiting chx to give an opinion) that resets all menu_links that are not customized or active.
Comment #8
catchWell, it's an experimental patch that works, on first impressions at least. On a 5.x-6.x upgrade, reproduced the extra menu items, ran update.php (with no updates, but all the way through anyway), and the extra items were gone from the admin panel. I'd been messing with menu drag and drop for the other js issue, so some of the old links were 'customised' and hence not deleted, but I'll try to do some more detailed testing later.
Comment #9
chx CreditAttribution: chx commentedhttp://drupal.org/node/211359#comment-696623
Comment #10
webernet CreditAttribution: webernet commentedThe isn't a duplicate. And the patch isn't right -- that simply rebuilds the menu tree, reordering the records in the table making this appear to be fixed, when in fact the source of the problem is the pager.
I believe that pager most likely takes the first n items from the menu table for page 1, whether or not they belong on page 1.
Comment #11
pwolanin CreditAttribution: pwolanin commentedI think webernet is correct about the pager (which I always had some concerns about). Prior to the drag-n-drop it was not as much of a problem since you had to go to a separate screen to edit the parent/weight.
To do this right, it really needs to be a custom version of the pager that makes sure that all the children of a parent appear on the same page with it.
Comment #12
theborg CreditAttribution: theborg commentedI think that there are two problems here:
1) The count query passed is counting all the items and not the items that end being displayed (hidden, not active).
2) webernet is right on that the childs can be separate from parents because of the pager not knowning anything about families.
To solve 1) I would like to pass the total number of rows to pager_query with a number and we only need a menu function that counts active items.
To solve 2) yes a custom pager.
Comment #13
pwolanin CreditAttribution: pwolanin commentedMarking "needs review" so you all look at it, though this is obviously not totally polished. Seems to work and to render all the top level links in the right order. Gobor or someone want to test with localized titles?
Comment #14
chx CreditAttribution: chx commentedIt is bad enough that we can't use the built in parent choosers because they display full menus but to display all children if a parent is visible is :( let's decide whether we want core to support big menus or not.
Comment #15
keith.smith CreditAttribution: keith.smith commentedThe patch in #13 certainly solves my initial problem, and I couldn't break it with casual testing.
Minor, but if you have to re-roll for some other reason, I did notice an "admintrator" typo in a code comment.
Comment #16
pwolanin CreditAttribution: pwolanin commentedupdated patch with one slight change to the code and lots of doxygen comments.
Comment #17
pwolanin CreditAttribution: pwolanin commentedhere's the alternative suggested by chx - just lose the pager.
Comment #18
webernet CreditAttribution: webernet commentedRemoving the pager completely sounds like the simplest and safest solution. It also fixes the issue of not being able to drag and drop over multiple pages. Hopefully the page won't get too big.
Comment #19
chx CreditAttribution: chx commentedYou can always menu_alter this page to your contrib module which does something better or just access FALSE this page and provide a totally alternative interface. With the drilldown parent choosers moved to D7 we already decided that while the backend provides support for insane big trees, the core UI does not. Case closed.
Comment #20
webernet CreditAttribution: webernet commentedTested OK.
Comment #21
pwolanin CreditAttribution: pwolanin commentedThis patch is the same as above, but I also noticed that this form builder doesn't store in any accessible place which menu it is working with - thus added an additional line.
Comment #22
Dries CreditAttribution: Dries commented+1 to no pager -- I found it to be annoying when moving one menu item to another page. Works better with DnD. :)
Comment #23
Gábor HojtsyWell, since even Dries says we should remove the pager, I committed #21. Better solutions for big menus will hopefully be available in contrib.
Comment #24
Anonymous (not verified) CreditAttribution: Anonymous commentedAutomatically closed -- issue fixed for two weeks with no activity.