Posted by douggreen on March 29, 2007 at 5:37pm
2 followers
Jump to:
| Project: | Links Package |
| Version: | 5.x-1.4 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | syscrusher |
| Status: | needs work |
Issue Summary
Thanks for the module! One minor problem... When trying to "open links in new window" with the setting of "external links only", I found two problems:
- the wrong variable is checked in links_get_goto_target. I don't know if you want to change the value here, or on the settings form, but I changed the value here because people are likely to have already set this value.
- the base_url doesn't need to be checked in links_is_external (because it is not yet prepended to the url), so rather than use url() to create the links/goto regex, just use "links/goto"
I've attached a patch for review.
| Attachment | Size |
|---|---|
| linktarget.patch | 1.1 KB |
Comments
#1
Greetings, and thanks for the patch!
This patch is a good start, but didn't entirely fix the problem. The second chunk in the patch was a straight-out fix for one of several issues. The first chunk wasn't quite the fix for the regexp problem, but I did some further digging into it and have it working now.
The feature now works for the actual "Related Links" list, but still not for the embedded links. I know how to fix that, but there's a large patch committed by another developer, which adds significant functionality, that will break if I go too far on this error before applying it.
So what I'm going to do is commit what I have so far as a partial fix, then work on that other patch, then revisit this issue for the "Links from Article Text" section.
Thanks again for the bug report and patch.
Syscrusher
#2
It sounds like this has been fixed, can this issue be closed?