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.
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.
Comment | File | Size | Author |
---|---|---|---|
#24 | shortcut-front-934714-24.patch | 3.37 KB | dcam |
#19 | add-frontpage-934714-19.patch | 3.59 KB | caiovlp |
#16 | 934714-16.patch | 4.36 KB | swentel |
#13 | 934714-13.patch | 2.48 KB | swentel |
#11 | 934714-11.patch | 1.82 KB | swentel |
Comments
Comment #1
webchickThe 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.Comment #2
IbnDrupal CreditAttribution: IbnDrupal commentedIssue is here:
shortcut.module line 624
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
Comment #3
furamag CreditAttribution: furamag commentedPer 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:
And we should use
<front>
instead of empty string for Path field.Comment #4
swentel CreditAttribution: swentel commentedFirst patch - needs tests still, but let's see what the bot says.
Comment #5
David_Rothstein CreditAttribution: David_Rothstein commentedI 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 ashttp://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.Comment #6
aspilicious CreditAttribution: aspilicious commentedI'm with david on this one. http://localhost/d7/ doesn't make sense. Ans I like his proposal
Comment #7
Bojhan CreditAttribution: Bojhan commentedIsn't there a problem when you change your frontpage, with say views?
Comment #8
swentel CreditAttribution: swentel commented@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.
Comment #9
Bojhan CreditAttribution: Bojhan commentedOke :) Seems like a quick fix
Comment #10
swentel CreditAttribution: swentel commentedPatch which allows you to add a link to the frontpage with an empty path.
Comment #11
swentel CreditAttribution: swentel commentedAs Bojhan pointed out on IRC, <front> should be possible as well.
Comment #12
swentel CreditAttribution: swentel commentedNeeds work though - something is going wrong with adding the empty path - it doesn't show up yet.
Comment #13
swentel CreditAttribution: swentel commentedOk, this one should be better - add and edit shortcut links have 2 different submit handlers, didn't realize that first.
Comment #14
Bojhan CreditAttribution: Bojhan commentedTested both usecases, and it worked. Marked RTBC.
Comment #15
sunThere are no tests yet.
Comment #16
swentel CreditAttribution: swentel commentedReroll for new core structure. Added '' as a path in testShortcutLinkAdd + extra update test for it.
Comment #17
chx CreditAttribution: chx commented#16: 934714-16.patch queued for re-testing.
Comment #19
caiovlp CreditAttribution: caiovlp commentedI 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.
Comment #20
cweagansCode style is good. Logic changes seem sane. I say RTBC
Comment #21
cweagansThis too
Comment #22
webchickOk, 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.
Comment #23
cweagansCongrats on your first core patch caiovlp!
Comment #24
dcam CreditAttribution: dcam commentedBackported #19 to 7.x.
Comment #25
valthebaldPatch worked for 7.x-dev as well:
Comment #26
David_Rothstein CreditAttribution: David_Rothstein commentedWell, 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