Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-22390

Auto-linking does not like brackets.

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.8, 2.2.1, 2.3.7, 2.4.4, 2.5
    • Fix Version/s: 2.3.8, 2.4.5, 2.5.1
    • Component/s: Filters
    • Labels:
    • Testing Instructions:
      Hide

      1. Ensure you have the URL auto-linking filter enabled.

      2. Try posting some text with some tricky URLs in, such as in https://moodle.org/mod/forum/discuss.php?d=227432.

      Verify that all reasonable URLs are linked, and that stray punctuation immediately after the link is not made into part of the link.

      Show
      1. Ensure you have the URL auto-linking filter enabled. 2. Try posting some text with some tricky URLs in, such as in https://moodle.org/mod/forum/discuss.php?d=227432 . Verify that all reasonable URLs are linked, and that stray punctuation immediately after the link is not made into part of the link.
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      This came up on the Moodle developers course.

      There were some questions like

      According to Patrick Lauke (http://www.ariadne.ac.uk/issue44/lauke/) which ...

      (There were created without using the HTML editor, so using FORMAT_MOODLE, not FORMAT_HTML.)

      What was happening was that the bit that automatically converts URLs to links was making a link out of

      http://www.ariadne.ac.uk/issue44/lauke/)

      which does not work. I think we need to adjust this bit of code so that it does not include the ) character in this case.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            timhunt Tim Hunt added a comment -

            I see that Jira's auto-linking get this right

            Show
            timhunt Tim Hunt added a comment - I see that Jira's auto-linking get this right
            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for reporting this issue.

            We have detected that this issue has been inactive for over a year has been recorded as affecting versions that are no longer supported.

            If you believe that this issue is still relevant to current versions (2.1 and beyond), please comment on the issue. Issues left inactive for a further month will be closed.

            Michael d;

            lqjjLKA0p6

            Show
            salvetore Michael de Raadt added a comment - Thanks for reporting this issue. We have detected that this issue has been inactive for over a year has been recorded as affecting versions that are no longer supported. If you believe that this issue is still relevant to current versions (2.1 and beyond), please comment on the issue. Issues left inactive for a further month will be closed. Michael d; lqjjLKA0p6
            Hide
            salvetore Michael de Raadt added a comment -

            I'm closing this issue as it appears to have become inactive and is probably not relevant to a current supported version. If you are encountering this problem or one similar, please launch a new issue.

            Show
            salvetore Michael de Raadt added a comment - I'm closing this issue as it appears to have become inactive and is probably not relevant to a current supported version. If you are encountering this problem or one similar, please launch a new issue.
            Hide
            timhunt Tim Hunt added a comment -

            I just tested, and this is still an issue, and very annoying it is too.

            For example, ensure the "Convert URLs into links and images" filter is enabled, then make a forum post with a body like:

            This is a test: (http://example.com/index.html)

            Show
            timhunt Tim Hunt added a comment - I just tested, and this is still an issue, and very annoying it is too. For example, ensure the "Convert URLs into links and images" filter is enabled, then make a forum post with a body like: This is a test: ( http://example.com/index.html )
            Hide
            timhunt Tim Hunt added a comment -

            Test: How does Jira handle http://en.wikipedia.org/wiki/Slash_(punctuation) I wonder?

            Show
            timhunt Tim Hunt added a comment - Test: How does Jira handle http://en.wikipedia.org/wiki/Slash_(punctuation ) I wonder?
            Hide
            timhunt Tim Hunt added a comment -

            https://moodle.org/mod/forum/discuss.php?d=227432

            So, there is no solution that magically does the right thing in all cases. Here is my work in progress which fixes this, but breaks the other cases from the unit tests.

            https://github.com/timhunt/moodle/compare/master...MDL-22390

            Show
            timhunt Tim Hunt added a comment - https://moodle.org/mod/forum/discuss.php?d=227432 So, there is no solution that magically does the right thing in all cases. Here is my work in progress which fixes this, but breaks the other cases from the unit tests. https://github.com/timhunt/moodle/compare/master...MDL-22390
            Hide
            timhunt Tim Hunt added a comment -

            Oh, a new test-case to add. For input www.bbc.co.uk only the www.bbc.co bit gets made into a link!. The forum thread shows several problems.

            Show
            timhunt Tim Hunt added a comment - Oh, a new test-case to add. For input www.bbc.co.uk only the www.bbc.co bit gets made into a link!. The forum thread shows several problems.
            Hide
            timhunt Tim Hunt added a comment -

            I think this is a good improvement, so I am submitting it for peer review.

            As well are reviewing the code, peer reviewers are invited to comment on whether this should be back-ported to stable branches.

            Show
            timhunt Tim Hunt added a comment - I think this is a good improvement, so I am submitting it for peer review. As well are reviewing the code, peer reviewers are invited to comment on whether this should be back-ported to stable branches.
            Hide
            timhunt Tim Hunt added a comment -
            Show
            timhunt Tim Hunt added a comment - Another broken case here: https://moodle.org/mod/forum/discuss.php?d=228773#p993402 And here: https://moodle.org/mod/forum/discuss.php?d=229559 (docs link in my post.)
            Hide
            matteo Matteo Scaramuccia added a comment -

            Hi Tim,
            I hope your work will be soon reviewed: IMHO it is actually helpful to fix/enh auto-linking as much as possible.

            Show
            matteo Matteo Scaramuccia added a comment - Hi Tim, I hope your work will be soon reviewed: IMHO it is actually helpful to fix/enh auto-linking as much as possible.
            Hide
            poltawski Dan Poltawski added a comment -

            Hi Tim,

            Well its tricky to really get your head aroudn the regexps when reviewing, but luckily you've created sensible tests for it and they all pass for me. +1

            Show
            poltawski Dan Poltawski added a comment - Hi Tim, Well its tricky to really get your head aroudn the regexps when reviewing, but luckily you've created sensible tests for it and they all pass for me. +1
            Hide
            poltawski Dan Poltawski added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            poltawski Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            timhunt Tim Hunt added a comment -

            Acutally, I am reopening this, ebcause I want to add more tests for the two other cases in my last comment.

            Show
            timhunt Tim Hunt added a comment - Acutally, I am reopening this, ebcause I want to add more tests for the two other cases in my last comment.
            Hide
            matteo Matteo Scaramuccia added a comment -

            Hi Tim,
            would you mind, maybe in a separate issue, to look at the glossary filter too to exclude URLs from being a target for that filter?
            See an example here: https://moodle.org/mod/forum/discuss.php?d=230250#p999906.

            TIA,
            Matteo

            Show
            matteo Matteo Scaramuccia added a comment - Hi Tim, would you mind, maybe in a separate issue, to look at the glossary filter too to exclude URLs from being a target for that filter? See an example here: https://moodle.org/mod/forum/discuss.php?d=230250#p999906 . TIA, Matteo
            Hide
            timhunt Tim Hunt added a comment -

            Right. Extra unit test added, and code fixed as a result, on the train to Scotland. This fix is being pushed to you from a place with a beautiful view across Upper Loch Torridon.

            Show
            timhunt Tim Hunt added a comment - Right. Extra unit test added, and code fixed as a result, on the train to Scotland. This fix is being pushed to you from a place with a beautiful view across Upper Loch Torridon.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Regex are awful to review. Unit tests are the light in the dark.
            Many thanks Tim, this has been integrated now.

            Show
            samhemelryk Sam Hemelryk added a comment - Regex are awful to review. Unit tests are the light in the dark. Many thanks Tim, this has been integrated now.
            Hide
            skodak Petr Skoda added a comment -

            works as described, thanks

            Show
            skodak Petr Skoda added a comment - works as described, thanks
            Hide
            marina Marina Glancy added a comment -

            Thanks for your awesome work! This has now become a part of Moodle.

            Closing as fixed!

            Show
            marina Marina Glancy added a comment - Thanks for your awesome work! This has now become a part of Moodle. Closing as fixed!
            Hide
            danmarsden Dan Marsden added a comment -
            Show
            danmarsden Dan Marsden added a comment - this may have screwed up some existing stuff: https://moodle.org/mod/forum/discuss.php?d=231543 https://moodle.org/mod/forum/discuss.php?d=232357

              People

              • Votes:
                1 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  8/Jul/13