I know it's not logical, since you have the little house icon, but:

  • Click edit shortcut
  • Add shortcut
  • Name it, and submit as empty path (frontpage)

Drupal says The link must correspond to a valid path on the site., but frontpage is a valid path.

Files: 
CommentFileSizeAuthor
#24 shortcut-front-934714-24.patch3.37 KBdcam
PASSED: [[SimpleTest]]: [MySQL] 39,390 pass(es).
[ View ]
#19 add-frontpage-934714-19.patch3.59 KBcaiovlp
PASSED: [[SimpleTest]]: [MySQL] 39,903 pass(es).
[ View ]
#16 934714-16.patch4.36 KBswentel
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 934714-16.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#13 934714-13.patch2.48 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 32,898 pass(es).
[ View ]
#11 934714-11.patch1.82 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 32,891 pass(es).
[ View ]
#10 934714-10.patch1.8 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 32,894 pass(es).
[ View ]
#4 934714-4.patch506 bytesswentel
PASSED: [[SimpleTest]]: [MySQL] 32,866 pass(es).
[ View ]
shortcut_bug.png28.7 KBCoornail

Comments

webchick’s picture

The way to handle this would be to make Shortcut module recognize <front> like Block, Menu and other modules do. Erroring out on an empty path is actually fine and expected.

IbnDrupal’s picture

Issue is here:

shortcut.module line 624

<?php
 
return !url_is_external($path) && menu_get_item($path);
?>

It calls menu_get_item to obtain information on the link. Problem is that there is no menu item so it returns a false.

If you comment out && menu_get_item($path) it accepts , but not suggested as I'm sure it will break things

furamag’s picture

Per my understanding this is problem in menu_get_item function. That function check table "menu_router" field "path" and return FALSE because there is no path for ''. But we can fix this in shortcut_valid_link() function.
shortcut.module line 622 should be:

return !url_is_external($path) && (menu_get_item($path) || $path == '<front>');

And we should use <front> instead of empty string for Path field.

swentel’s picture

Version:7.0-beta1» 8.x-dev
Status:Active» Needs review
Issue tags:+Needs tests
StatusFileSize
new506 bytes
PASSED: [[SimpleTest]]: [MySQL] 32,866 pass(es).
[ View ]

First patch - needs tests still, but let's see what the bot says.

David_Rothstein’s picture

Issue tags:+needs backport to D7

I don't think allowing <front> really solves this bug. Given the user interface here, it is nonintuitive to type <front>, because the form element will then wind up reading as http://localhost/d7/<front> which is nonsensical. So, people will still leave this blank expecting it to point to the home page, as per the original bug report.

Why not make an empty path correspond to the home page, as was requested? It really seems to me like the menu API itself should be changed to support that, but in case that gets complicated (especially for D7 backport), it would be pretty easy to have the shortcut module save <front> behind the scenes but display an empty string in the form instead.

aspilicious’s picture

I'm with david on this one. http://localhost/d7/ doesn't make sense. Ans I like his proposal

Bojhan’s picture

Isn't there a problem when you change your frontpage, with say views?

swentel’s picture

@bojhan, not really, behind the scenes will be used as the menu path and that will trigger the right frontpage on the front regardless to what that is set.
@David I'll come up with a new patch somewhere this week. We'll probably need an extra validation on the shortcut set to make sure there is only one link to the frontpage in that set.

Bojhan’s picture

Status:Needs review» Needs work

Oke :) Seems like a quick fix

swentel’s picture

Status:Needs work» Needs review
StatusFileSize
new1.8 KB
PASSED: [[SimpleTest]]: [MySQL] 32,894 pass(es).
[ View ]

Patch which allows you to add a link to the frontpage with an empty path.

swentel’s picture

StatusFileSize
new1.82 KB
PASSED: [[SimpleTest]]: [MySQL] 32,891 pass(es).
[ View ]

As Bojhan pointed out on IRC, <front> should be possible as well.

swentel’s picture

Status:Needs review» Needs work

Needs work though - something is going wrong with adding the empty path - it doesn't show up yet.

swentel’s picture

Status:Needs work» Needs review
StatusFileSize
new2.48 KB
PASSED: [[SimpleTest]]: [MySQL] 32,898 pass(es).
[ View ]

Ok, this one should be better - add and edit shortcut links have 2 different submit handlers, didn't realize that first.

Bojhan’s picture

Status:Needs review» Reviewed & tested by the community

Tested both usecases, and it worked. Marked RTBC.

sun’s picture

Status:Reviewed & tested by the community» Needs work

There are no tests yet.

swentel’s picture

Status:Needs work» Needs review
StatusFileSize
new4.36 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 934714-16.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Reroll for new core structure. Added '' as a path in testShortcutLinkAdd + extra update test for it.

chx’s picture

#16: 934714-16.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+Needs tests, +needs backport to D7

The last submitted patch, 934714-16.patch, failed testing.

caiovlp’s picture

Status:Needs work» Needs review
StatusFileSize
new3.59 KB
PASSED: [[SimpleTest]]: [MySQL] 39,903 pass(es).
[ View ]

I rewrote swentel's tests and updated it to reflect new file structure - I believe files have been added and removed since the patch was submitted. All credit for the fix itself goes to swentel.

cweagans’s picture

Code style is good. Logic changes seem sane. I say RTBC

cweagans’s picture

Status:Needs review» Reviewed & tested by the community

This too

webchick’s picture

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)

Ok, looks like this approach has buy-in from folks, and it comes with tests, too!

Committed and pushed to 8.x. Moving to 7.x for backport.

cweagans’s picture

Congrats on your first core patch caiovlp!

dcam’s picture

Status:Patch (to be ported)» Needs review
StatusFileSize
new3.37 KB
PASSED: [[SimpleTest]]: [MySQL] 39,390 pass(es).
[ View ]

Backported #19 to 7.x.

valthebald’s picture

Status:Needs review» Reviewed & tested by the community

Patch worked for 7.x-dev as well:

  1. Tried to add new shortcut with <front> path
  2. Got an error
  3. Applied the patch
  4. Tried to add new shortcut with <front> path
  5. Shortcut was added
  6. Shortcut points to the right page (home page)
David_Rothstein’s picture

Status:Reviewed & tested by the community» Fixed

Well, it's interesting that <front> works directly also, since the goal of the patch was primarily to make an empty string work here :)

But I checked that that works too (and the tests prove it as well), plus this patch looks good to me for Drupal 7, so... I went ahead and committed it to 7.x. Thanks! http://drupalcode.org/project/drupal.git/commit/160e454

Status:Fixed» Closed (fixed)
Issue tags:-Needs tests, -needs backport to D7

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