Moodle
  1. Moodle
  2. MDL-31885

The activity name auto-linking filter should not create links to webpages it was created on.

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.16, 2.0.2, 2.1, 2.2.1
    • Fix Version/s: 2.5
    • Component/s: Filters
    • Labels:
    • Database:
      Any
    • Testing Instructions:
      Hide
      1. Create a page with the name 'Introduction'
      2. Fill the content of the page with something like 'This introduction is ...', check "display description on the course page"
      3. Enable the activity name auto-linking (enabled by default)
      4. Make sure that the word introduction is not changed into a link both on course page and on module page
      Show
      Create a page with the name 'Introduction' Fill the content of the page with something like 'This introduction is ...', check "display description on the course page" Enable the activity name auto-linking (enabled by default) Make sure that the word introduction is not changed into a link both on course page and on module page
    • Workaround:
      Hide
      filter/activitynames/filter.php
      - if ($activity->mod != "label" and $activity->visible and empty($activity->groupmembersonly)) {
      
      + if ($activity->mod != "label" and 
      +    $activity->cm != $this->context->instanceid and
      +    $activity->visible and 
      +    empty($activity->groupmembersonly)) {
      
      Show
      filter/activitynames/filter.php - if ($activity->mod != "label" and $activity->visible and empty($activity->groupmembersonly)) { + if ($activity->mod != "label" and + $activity->cm != $ this ->context->instanceid and + $activity->visible and + empty($activity->groupmembersonly)) {
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull Master Branch:
      wip-MDL-31885-smartcache
    • Rank:
      38530

      Description

      A link on a webpage that refers to the webpage itself, provide no extra information at all. Links should refer to anchors on the same page, or other pages on the site or internet.

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for suggesting that and providing a fix.

          Show
          Michael de Raadt added a comment - Thanks for suggesting that and providing a fix.
          Hide
          Marina Glancy added a comment -

          taking this issue, I was working on the same line in MDL-33774

          Show
          Marina Glancy added a comment - taking this issue, I was working on the same line in MDL-33774
          Hide
          Marina Glancy added a comment -

          I added a check for contextlevel to submitted patch.
          Also I added second commit removing the cache inside filter_activitynames because caching prevented different logic for different modules

          Note that master branch is based on MDL-33774

          Show
          Marina Glancy added a comment - I added a check for contextlevel to submitted patch. Also I added second commit removing the cache inside filter_activitynames because caching prevented different logic for different modules Note that master branch is based on MDL-33774
          Hide
          Damyon Wiese 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.

          Thanks!

          Show
          Damyon Wiese 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. Thanks!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Side note: be careful when chaining branches, for some reason MDL-33774 final commits have changed (rebase or whatever) since MDL-31885 was created and right now they are different and the compare does not take rid of them. No big problem, just fyi.

          Show
          Eloy Lafuente (stronk7) added a comment - Side note: be careful when chaining branches, for some reason MDL-33774 final commits have changed (rebase or whatever) since MDL-31885 was created and right now they are different and the compare does not take rid of them. No big problem, just fyi.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Sorry, I'm reopening this:

          1) While I understand the reason to stop using current static cache in the filter (courseid-based), not usable anymore coz we are introducing cm-based variations, I think the filter, run without any cache can have a negative impact. Just imagine one big discussion, for example with, say, 1000 texts to process (more if filtering of headings is enabled). Without any caching, the system will have to perform 1000 fullclones, sortings and iterations, when all them are going to run for the same cm (a forum one).

          So I'd suggest to, at least, implement a cm-based caching (static or MUC, whatever) for $activitylist. And, ideally I'd also implement a course-based caching (for $sortedactivities), but surely it's harder to get 1000 texts to filter there, although not impossible.

          2) Given that you're moving the whole code, surely it would be a good moment to apply the coding-style rules to it (inline comments, no-space on function calls...)

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Sorry, I'm reopening this: 1) While I understand the reason to stop using current static cache in the filter (courseid-based), not usable anymore coz we are introducing cm-based variations, I think the filter, run without any cache can have a negative impact. Just imagine one big discussion, for example with, say, 1000 texts to process (more if filtering of headings is enabled). Without any caching, the system will have to perform 1000 fullclones, sortings and iterations, when all them are going to run for the same cm (a forum one). So I'd suggest to, at least, implement a cm-based caching (static or MUC, whatever) for $activitylist. And, ideally I'd also implement a course-based caching (for $sortedactivities), but surely it's harder to get 1000 texts to filter there, although not impossible. 2) Given that you're moving the whole code, surely it would be a good moment to apply the coding-style rules to it (inline comments, no-space on function calls...) Ciao
          Hide
          Marina Glancy added a comment -

          Hi Eloy,

          0) sorry, did not do rebasing right. Corrected now

          1) I removed the caching also because it is excessive.
          First, filter_activitynames should not be applied to strings because strings do not have tags (and links) anyway.
          Second, there is caching in the filterlib already, the filtered text is cached for every string.
          Third, there are no database requests in this function, so we don't really save much by caching. modinfo is cached already and all we were caching was the re-arranged activities list from modinfo.
          Forth, the only situation where the cache that I removed was used is: course view page that has a lot of texts (such as labels and activities with shown description). Hehe, saying that - before my labels issue MDL-33774 labels were not filtered at all...

          2) sure

          Anyway, when I was writing all that I had an idea of per-course caching. Will post soon

          Show
          Marina Glancy added a comment - Hi Eloy, 0) sorry, did not do rebasing right. Corrected now 1) I removed the caching also because it is excessive. First, filter_activitynames should not be applied to strings because strings do not have tags (and links) anyway. Second, there is caching in the filterlib already, the filtered text is cached for every string. Third, there are no database requests in this function, so we don't really save much by caching. modinfo is cached already and all we were caching was the re-arranged activities list from modinfo. Forth, the only situation where the cache that I removed was used is: course view page that has a lot of texts (such as labels and activities with shown description). Hehe, saying that - before my labels issue MDL-33774 labels were not filtered at all... 2) sure Anyway, when I was writing all that I had an idea of per-course caching. Will post soon
          Hide
          Marina Glancy added a comment -

          I created a new branch where I don't remove the static cache but just adjust the course list for the module

          But I can not change it to MUC because of some MUC error, so I created a new issue MDL-38111 and already submitted a patch (with all code refactoring like you wanted). But it is waiting on Sam

          Show
          Marina Glancy added a comment - I created a new branch where I don't remove the static cache but just adjust the course list for the module But I can not change it to MUC because of some MUC error, so I created a new issue MDL-38111 and already submitted a patch (with all code refactoring like you wanted). But it is waiting on Sam
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Damyon Wiese 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.

          Thanks!

          Show
          Damyon Wiese 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. Thanks!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Aha, I've already integrated MDL-38110, per Sam's request. Will look to this and its followup (MDL-38111) tomorrow.

          Thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Aha, I've already integrated MDL-38110 , per Sam's request. Will look to this and its followup ( MDL-38111 ) tomorrow. Thanks!
          Hide
          Marina Glancy added a comment -

          Eloy, MDL-38111 is not pushed for integration yet. Actually it is not finished yet because cache does not have a proper definition.

          Show
          Marina Glancy added a comment - Eloy, MDL-38111 is not pushed for integration yet. Actually it is not finished yet because cache does not have a proper definition.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Clever switch to activity based cache, integrated, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Clever switch to activity based cache, integrated, thanks!
          Hide
          Frédéric Massart added a comment -

          Works as described. Thanks!

          Show
          Frédéric Massart added a comment - Works as described. Thanks!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Because

          A
          MARVELOUS
          A       U
          Z  YOU  P
          I  ARE  E
          N  PPL  R
          G       B
            TNKS! 
          

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Because A MARVELOUS A U Z YOU P I ARE E N PPL R G B TNKS! Closing, ciao

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: