Implement LocalTask plugins for login/password/register tabs [#2044539]

Comment	File	Size	Author
#15	user_local_task-2044539-15.patch	2.83 KB	dawehner

#15	interdiff.txt	1.35 KB	dawehner
#13	user_tasks-2044539-13.patch	1.48 KB	dawehner

#10	user-2044539-10.patch	4.11 KB	dawehner

#4	2044539-4.patch	7.23 KB	pwolanin

#4	2044539-3-4.increment.txt	3.12 KB	pwolanin
#3	2044539-3.patch	4.11 KB	pwolanin

#3	2044539-1-3.increment.txt	1.19 KB	pwolanin
#1	2044539-1.patch	3.51 KB	pwolanin

Comment #1

pwolanin CreditAttribution: pwolanin commented 18 July 2013 at 01:35

Status:

Active

» Needs review

File	Size
2044539-1.patch	3.51 KB

simple conversion

Log in or register to post comments

Comment #2

18 July 2013 at 02:45

Status:

Needs review

» Needs work

The last submitted patch, 2044539-1.patch, failed testing.

Log in or register to post comments

Comment #3

pwolanin CreditAttribution: pwolanin commented 19 July 2013 at 02:56

Status:

Needs work

» Needs review

File	Size
2044539-1-3.increment.txt	1.19 KB
2044539-3.patch	4.11 KB

Partial fix - also looks like this requires #2031353: URLgenerator broken for Drupal installed in a subdirectory - doesn't have a way to get a Drupal path So I expect a couple fails.

Log in or register to post comments

Comment #4

pwolanin CreditAttribution: pwolanin commented 19 July 2013 at 03:35

File	Size
2044539-3-4.increment.txt	3.12 KB
2044539-4.patch	7.23 KB

This fixes the base plugin - we should have just have been pulling the path from the route patten, as we did earlier in the issue where that class was created, instead of trying to use the generator since the base plugin doesn't handle dynamic paths.

Log in or register to post comments

Comment #5

pwolanin CreditAttribution: pwolanin commented 19 July 2013 at 15:33

Issue tags:

+MenuSystemRevamp, +WSCCI

tagging

Log in or register to post comments

Comment #6

janNelx adsasda CreditAttribution: janNelx adsasda commented 19 July 2013 at 16:08

I had the problem but it works now

Log in or register to post comments

Comment #7

dawehner

German

CreditAttribution: dawehner commented 20 July 2013 at 14:58

+++ b/core/lib/Drupal/Core/Menu/LocalTaskBase.phpundefined
@@ -93,15 +93,9 @@ public function getTitle() {
+    // to generate the system path.
+    $route = $this->provider->getRouteByName($this->getRouteName());
+    return trim($route->getPath(), '/');

As written in IRC this is a step backwards, as we would have to use the generate method in the future again.

+++ b/core/modules/user/user.moduleundefined
@@ -814,25 +814,21 @@ function user_menu() {
-  // Other authentication methods may add pages below user/login/.
-  $items['user/login/default'] = array(

I guess we should not remove that feature?

Log in or register to post comments

Comment #8

pwolanin CreditAttribution: pwolanin commented 20 July 2013 at 15:46

@dawehner - it would still be possible to add sub-tabs below login, it just doesn't require this hack of an extra default local task.

Log in or register to post comments

Comment #9

pwolanin CreditAttribution: pwolanin commented 20 July 2013 at 15:49

@dawehner - if we want to use the generator rather than the simple static pattern, then this conversion is blocked these 2 issue I think:
#2031353: URLgenerator broken for Drupal installed in a subdirectory - doesn't have a way to get a Drupal path
#2031487: When replacing the upcasted values in the request attributes array, retain the original raw value too

Log in or register to post comments

Comment #10

dawehner

German

CreditAttribution: dawehner commented 12 August 2013 at 22:35

File	Size
user-2044539-10.patch	4.11 KB

Rerolled the patch without the changes in the local action manager.

Log in or register to post comments

Comment #11

13 August 2013 at 00:00

Status:

Needs review

» Needs work

The last submitted patch, user-2044539-10.patch, failed testing.

Log in or register to post comments

Comment #12

dawehner

German

CreditAttribution: dawehner commented 13 August 2013 at 19:30

This currently fails because the path with enabled language like /en/user/password. Given this path to menu_item_route_access() fails.
The proper solution for this problem is #2046737: Add a method to the AccessManager that only needs a route name and parameters