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

          Attachments

            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