Breadcrumb, help and tree are now fixed on 40x pages. Most (but not all) bugs were due to the shortcut for the access checking which in itself was a bit broken: only visible parents are access checked and we happily inherited from nonvisible
elements. In a weird tree configuration this could have lead to skipping access checking -- core does not have such weirdness.

CommentFileSizeAuthor
#9 menu_103.patch23.61 KBchx
#8 menu_102.patch22.28 KBchx
#7 menu_101.patch22.29 KBchx
#6 menu_100.patch22.28 KBchx
#4 menu_96.patch3.62 KBchx
menu_85.patch2.81 KBchx
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chx’s picture

chx CreditAttribution: chx commented

To test this patch, pick a 404 and a 403 page in HEAD -- the 403 page should be a tab -- observe what you see -- apply patch -- observe again and report.

dmitrig01’s picture

dmitrig01 CreditAttribution: dmitrig01 commented

404 - menu system errors gone, but garland page.tpl.php errors remain.

403:
before:
'access denied' but there are tabs (by task, by module), and in the sidebar (nav menu) there is a logs menu item.
All testing done with a fresh install, and I haven't even created a user. When I click on logs, the menu expands to show the correct items.
after:
works as expected

webernet’s picture

webernet CreditAttribution: webernet commented
Status: Needs review » Reviewed & tested by the community

Patch fixes the issue with tabs and menu items appearing when they shouldn't.

chx’s picture

chx CreditAttribution: chx commented
FileSize
menu_96.patch3.62 KB

I fixed the trail as well. This greatly simplifies matters.

webernet’s picture

webernet CreditAttribution: webernet commented
Status: Reviewed & tested by the community » Needs work
chx’s picture

chx CreditAttribution: chx commented
Title: Menu behaviour is faulty on 403 and 404 pages » Menu bugfixes: 40x pages, external items, parenting
Status: Needs work » Needs review
FileSize
menu_100.patch22.28 KB

Aside from bugfixes, I renamed menu_edit to menu_custom because it holds new items as well (we discussed with Steven) and changed the menu definition in system -- I consider the menu table structures now complete. Let's hope nothing arises. I wanted to include an upgrade path, In some sense, it's trivial, because menu_custom and the old menu table holds almost the same fields. The lack of the 'parent' field however, is a big, big problem. I will consult the powers that be how shall we proceed in this regard.

chx’s picture

chx CreditAttribution: chx commented
FileSize
menu_101.patch22.29 KB

Rerolled to HEAD

chx’s picture

chx CreditAttribution: chx commented
FileSize
menu_102.patch22.28 KB

Now...

chx’s picture

chx CreditAttribution: chx commented
FileSize
menu_103.patch23.61 KB

Better..

dmitrig01’s picture

dmitrig01 CreditAttribution: dmitrig01 commented

Everything worked. I played around with parenting, that worked. I set a link to drupal.org, that worked. The only thing is that on a menu item edit page, the admin menu is not expanded (see my screenshot

dmitrig01’s picture

dmitrig01 CreditAttribution: dmitrig01 commented
Status: Needs review » Reviewed & tested by the community

:-)

chx’s picture

chx CreditAttribution: chx commented

Thanks. That problem is a totally different bug which will need attention down the line, but it's so minor that even if I can't get it fixed before the freeze I won't lose sleep.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks! :)

Anonymous’s picture

(not verified) CreditAttribution: commented
Status: Fixed » Closed (fixed)