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:

  1. 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.
  2. 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.

CommentFileSizeAuthor
linktarget.patch1.1 KBdouggreen

Comments

syscrusher’s picture

syscrusher commented
Assigned: Unassigned » syscrusher
Status: Needs review » Needs work

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

douggreen’s picture

douggreen commented

It sounds like this has been fixed, can this issue be closed?