It seems that this module affects the url() function in Drupal:
Using http://www.twitter.com?test=foo as my test URL, and using the code below, the url() function returns this:
http://www.twitter.com?test=foo
Using http://www.twitter.com/test/?test=faoo as my test URL (note the addition of /test/, the url() function returns this - when Any Menu Path is enabled:
/test/?test=foo
The Expectation:
That the external URL be returned in full.
The Code:
$url = "http://www.twitter.com?test=foo";
$url_parts = _link_parse_url($url);
if (!empty($url_parts['url'])) {
$item['url'] = url($url_parts['url'],
array(
'query' => isset($url_parts['query']) ? $url_parts['query'] : NULL,
'fragment' => isset($url_parts['fragment']) ? $url_parts['fragment'] : NULL,
'absolute' => TRUE,
'html' => TRUE,
)
);
}
print $item['url'];I had posted this issue in Link ( https://drupal.org/node/2236957 ) thinking it was a problem there, but after testing on a vanilla Drupal install I discovered it was this module that causes the problem. Any insight into how I could create a fix?
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | 2237615-9-better-handling-of-menu-items.patch | 3.79 KB | hanoii |
Comments
Comment #1
wOOge commentedComment #2
wOOge commentedComment #3
wizonesolutionswOOge, you mention that this is a conflict with the Link module. Do you mean that this doesn't happen if Link is disabled?
Anyway, I'm seeing the same behavior. In my case,
www.example.com/pathis being turned into/path(the site I am working on is atsubdomain.example.com).It doesn't happen for URLs without paths, though. I am looking into this.
Comment #4
wOOge commented@wizonesolutions good point — I think this module might be adversely affecting the Drupal url() function, from which Link module gets its links.
Comment #5
wizonesolutionsYeah, it is. It's the path change in
any_menu_path_url_outbound_alter(). I think we do that to work around a saving bug where the query string gets appended to the menu path and thus duplicated, although it doesn't actually fix that bug anyway.I'll be looking into a fix for this soon, but patches are welcome.
Comment #6
hanoiiDo you need that function at all? This module should not alter the url, is just too prone to breaking, what is the reason for that function to exist?
I just removed it and it works fine, if you have problems saving query strings I suggest giving a more limiting approach, so you keep this module simpler. This module is great, it is meant to do one thing (as far as I understood its functionality) which is to allow non existent path to be created. If drupal has issues saving such paths it shouldn't be a task for the module to sort that out, you could easily work it out with the redirect module.
Am I missing something?
For now I am running the module without that function at all.
Comment #7
hanoiiAnd I don't think this is a conflict with link module but rather a conflict with a lot of places, I for one thing have an external menu item to my profile in drupal.org which works just fine without the module and when I enabled this module it also renders it as an internal drupal path:
Comment #8
wizonesolutionshanoii: I don't think we need it either, but the issues it's supposed to address are:
https://drupal.org/node/1961996
https://drupal.org/node/1937018
So whatever solution we use needs to not break those. I am thinking the fix is in the submit function though; the URLs get saved wrong. Need to dig deeper. Patch welcome if you solve it sooner.
Comment #9
hanoiiI had a crack at it, I did a bit of a major rework of the module, removed a bit of unnecessary functions IMHO and changed the way some things are handled.
Most is the same, seems to work with all combinations, the only thing I did, is that in case of valid drupal path, I actually let the menu module does its thing so that menu_router works, so now aliases are properly used as well as some issues with PHP error notices.
Have a look at it.
Also, as I now have a pretty good understanding of the module, I'd like to request maintainer access if you are interested, as I can help maintain the code a bit.
I did notice the git repository is a bit messy. A commit was made on a tag, I think 1.3 that then was made 1.4, which you are not supposed to do, or at least it looks like it by browsing the repo.
Patch was made against tag 1.4 of git repository, coudln't find a proper branch to do it against it :s
Comment #10
wizonesolutionsThanks for volunteering to help maintain the module! I'm a co-maintainer, so I can't give you administrator access, but I'll ask my colleague about it for you. The bad practices are his fault, of course :P
I need this patch for a project I'm currently on, so I will be able to test it and commit it if it works. Probably this week.
Comment #11
caseyc commentedHanoii, we'd love to have you help out! Granting privs now.
Comment #12
hanoii@caseyc thanks!!! I have already fixed the git repo. I merged the query-params branch into 7.x-1.x (only commits were made on query-params and release were from that branch) and also merged 7.x-1.4 tag into 7.x-1.x, so now 7.x-1.x is the only branch with the proper history of all commits. Finally removed the query-params branch as it confuses more than what it adds.
About this patch, it will be great if you can test it out a bit. I am happy to commit it as I think it's pretty good, but worth hearing if you had some time to test.
Comment #13
wizonesolutions@hanoii, yep, should be able to test that out today or tomorrow, or at worst early next week. But I'll definitely be needing it.
Comment #14
wizonesolutionsRight...today, tomorrow, 3 weeks :)
This patch solved the problem. I would say to commit it. In the release notes (or maybe an update.php message), it'd be good to inform people that they may need to edit their Any Menu Path menu items because some of them might have had duplicated query strings or fragments without realizing it. This fix will reveal the problem.
Comment #15
nicolasg commentedI too was having many problems with the any_menu_path_url_outbound_alter() function.
Wasted 2 days trying to find out why some forms would not post data, and update.php would also cycle and not proceed.
This patch fixed it for me. Thanks.
Comment #16
Anonymous (not verified) commentedPatch #9 also worked for me. The module was manipulating several paths set in view fields, Display Suite custom fields and Link fields.
Thanks for your contribution, hanoii
Comment #17
rtrubshaw commentedCan I suggest that - since this is still a problem in 1.4 and since it's more than 5 months since the fix was agreed and committed - it is time for version 1.5 to be released as those of us that have clients who are averse to running -dev code have been reduced to downgrading this module to 1.2 to avoid the issues created by the rather inclusive nature of the hook_url_outbound_alter() implementation.
Edit:
@hanoii Sorry, it seemed from the comments that the developer was happy and that the change was committed. (As it should have been; it was good work.)
Comment #18
hanoiiThis patch was never committed. I have developer access to the repo, but gained it and somehow this major patch was due to be reviewed by the original devs. If I hear a thumbs up from dev, I can commit this and release a new version.
Comment #19
wizonesolutions@hanoii: +1, there's enough user feedback.
Comment #20
CatherineOmega commentedI can confirm that this patch allows mailto: and ?cc= email links to work once more. A new release would be great!
Comment #21
hanoiiOk, I will release a new version this week.
Comment #22
hanoiiCommitted