Updated: Comment #N

Problem/Motivation

Because modern local actions are keyed by route name and legacy are keyed by path, both are added.

Proposed resolution

When possible, key legacy local actions by route name, and ensure that modern ones take precedence

Remaining tasks

Does this need tests if we're about to remove all legacy local actions within the next several weeks?

User interface changes

Fix visual bug

API changes

N/A

N/A

Files: 
CommentFileSizeAuthor
#5 local-actions-2105167-5.patch2.05 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 58,414 pass(es).
[ View ]
#5 interdiff.txt900 bytestim.plunkett
#1 local-actions-2105167-1.patch1.17 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 58,929 pass(es), 4 fail(s), and 0 exception(s).
[ View ]
#1 Screen Shot 2013-10-04 at 2.09.56 PM.png17.94 KBtim.plunkett
#1 Screen Shot 2013-10-04 at 2.10.06 PM.png18 KBtim.plunkett

Comments

Status:Active» Needs review
StatusFileSize
new18 KB
new17.94 KB
new1.17 KB
FAILED: [[SimpleTest]]: [MySQL] 58,929 pass(es), 4 fail(s), and 0 exception(s).
[ View ]

Before:
Screen Shot 2013-10-04 at 2.09.56 PM.png

After:

Screen Shot 2013-10-04 at 2.10.06 PM.png

This looks reasonable to me. Initially we keyed the plug local actions by path which prevented this problem, but later we switched to route name.

By putting anything from hook_menu also by route it's the correct fix.

Status:Needs review» Reviewed & tested by the community

tested and fixed my problem. If pwolanin is ok with it, RTBC then :)

Status:Reviewed & tested by the community» Needs work

The last submitted patch, local-actions-2105167-1.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new900 bytes
new2.05 KB
PASSED: [[SimpleTest]]: [MySQL] 58,414 pass(es).
[ View ]

Two local actions are in a different order now:

menu_test.local_action4:
  path: '/menu-test-local-action/dynamic-title'
menu_test.local_action2:
  path: '/menu-test-local-action/hook_menu'

When two hook_menu-based local actions have the same weight, they were previously ordered by href, now they are ordered by route name.

That's why these switched (d < h, 2 < 4).

Yay for test coverage!

Status:Needs review» Reviewed & tested by the community

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.