Moodle
  1. Moodle
  2. MDL-22390

Auto-linking does not like brackets.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor 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:
    • Rank:
      6009

      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.

        Issue Links

          Activity

          Hide
          Tim Hunt added a comment -

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

          Show
          Tim Hunt added a comment - I see that Jira's auto-linking get this right
          Hide
          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
          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
          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
          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
          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
          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
          Tim Hunt added a comment -

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

          Show
          Tim Hunt added a comment - Test: How does Jira handle http://en.wikipedia.org/wiki/Slash_(punctuation ) I wonder?
          Hide
          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
          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
          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
          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
          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
          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
          Tim Hunt added a comment -
          Show
          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 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 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
          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
          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
          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
          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
          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
          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 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 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
          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
          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
          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
          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
          Petr Škoda added a comment -

          works as described, thanks

          Show
          Petr Škoda added a comment - works as described, thanks
          Hide
          Marina Glancy added a comment -

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

          Closing as fixed!

          Show
          Marina Glancy added a comment - Thanks for your awesome work! This has now become a part of Moodle. Closing as fixed!
          Hide
          Dan Marsden added a comment -
          Show
          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: