Moodle
  1. Moodle
  2. MDL-38766

wiki doesn't redirect correctly when moodle installed in subdirectory

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.2.3, 2.2.4, 2.2.5, 2.2.10, 2.3, 2.3.1, 2.3.2, 2.3.6, 2.4.3
    • Fix Version/s: 2.3.7, 2.4.4
    • Component/s: Wiki (2.x)
    • Labels:
    • Rank:
      48825

      Description

      Introduced in MDL-30478 (commit: 5dedd64)

      The redirect to (view.php?pageid=) doesn't include path (/mod/wiki/) so moodle_url() doesn't create a valid URL.

        Issue Links

          Activity

          Hide
          Paul Campbell added a comment -

          There were several other instances of add_to_log(), and a few redirect(), that didn't use moodle_url(). These have been fixed in the pull branch linked.

          Show
          Paul Campbell added a comment - There were several other instances of add_to_log(), and a few redirect(), that didn't use moodle_url(). These have been fixed in the pull branch linked.
          Hide
          Michael de Raadt added a comment -

          Thanks for reporting that and providing a fix.

          This might also help resolve MDL-36981. We should test that after fixing this issue.

          Show
          Michael de Raadt added a comment - Thanks for reporting that and providing a fix. This might also help resolve MDL-36981 . We should test that after fixing this issue.
          Hide
          Rajesh Taneja added a comment -

          Thanks for providing the patch Paul,

          add_to_log is just populating log table and has nothing to do with redirect.
          I tried replicating this issue on master and it seems to be working fine, (moodle installed in sub-dir).

          Can you please confirm if $CFG->wwwroot is set properly.
          Also, it will be helpful if you can provide some more details about your installation, as I was unable to replicate this issue with testing instructions provided in this ticket.

          FYI: links in add_to_log are relative and get reconstructed as per module. Please check other areas like book/view.php, url/view.php etc.

          Show
          Rajesh Taneja added a comment - Thanks for providing the patch Paul, add_to_log is just populating log table and has nothing to do with redirect. I tried replicating this issue on master and it seems to be working fine, (moodle installed in sub-dir). Can you please confirm if $CFG->wwwroot is set properly. Also, it will be helpful if you can provide some more details about your installation, as I was unable to replicate this issue with testing instructions provided in this ticket. FYI: links in add_to_log are relative and get reconstructed as per module. Please check other areas like book/view.php, url/view.php etc.
          Hide
          Paul Campbell added a comment -

          You're right. All the add_to_log changes I made are irrelevant.

          The issue is actually fixed by the change to mod/wiki/pagelib.php which only uses new moodle_url('view.php... rather than specifying the path /mod/wiki/view.php

          How should I update my fix? Rewrite the branch or create a new one?

          Show
          Paul Campbell added a comment - You're right. All the add_to_log changes I made are irrelevant. The issue is actually fixed by the change to mod/wiki/pagelib.php which only uses new moodle_url('view.php... rather than specifying the path /mod/wiki/view.php How should I update my fix? Rewrite the branch or create a new one?
          Hide
          Rajesh Taneja added a comment -

          Thanks Paul,

          You are right, it's not correct. Looking at the changes you have made in your current branch, it will be easy for you to create a new branch.

          We will appreciate if you can create patch for 2.4 and 2.3 branches as well.

          Show
          Rajesh Taneja added a comment - Thanks Paul, You are right, it's not correct. Looking at the changes you have made in your current branch, it will be easy for you to create a new branch. We will appreciate if you can create patch for 2.4 and 2.3 branches as well.
          Hide
          Paul Campbell added a comment -
          • Updating Description to reflect correct origin and nature of this bug.
          • Add new pull branches for master, 2.3 and 2.4
          Show
          Paul Campbell added a comment - Updating Description to reflect correct origin and nature of this bug. Add new pull branches for master, 2.3 and 2.4
          Hide
          Rajesh Taneja added a comment -

          Thanks for providing the quick patch Paul,

          pushing it for peer-review.

          Show
          Rajesh Taneja added a comment - Thanks for providing the quick patch Paul, pushing it for peer-review.
          Hide
          Jason Fowler added a comment -

          [Y] Syntax
          [-] Output
          [Y] Whitespace
          [-] Language
          [-] Databases
          [Y] Testing
          [-] Security
          [-] Documentation
          [Y] Git
          [Y] Sanity check

          Show
          Jason Fowler added a comment - [Y] Syntax [-] Output [Y] Whitespace [-] Language [-] Databases [Y] Testing [-] Security [-] Documentation [Y] Git [Y] Sanity check
          Hide
          Damyon Wiese added a comment -

          This is not actually a bug. Sure it is nice to use the same standard as the rest of the code in wiki - but new moodle_url('view.php'...) works fine and is used in lots of places in Moodle. Pinging Raj to see if he ever managed to reproduce this bug.

          Show
          Damyon Wiese added a comment - This is not actually a bug. Sure it is nice to use the same standard as the rest of the code in wiki - but new moodle_url('view.php'...) works fine and is used in lots of places in Moodle. Pinging Raj to see if he ever managed to reproduce this bug.
          Hide
          Damyon Wiese added a comment -

          (And sorry - this will not help MDL-36981 at all - it will do nothing).

          Show
          Damyon Wiese added a comment - (And sorry - this will not help MDL-36981 at all - it will do nothing).
          Hide
          Dan Poltawski added a comment -

          Integrated to master, 24 and 23. Thanks a lot Paul!

          Show
          Dan Poltawski added a comment - Integrated to master, 24 and 23. Thanks a lot Paul!
          Hide
          Dan Poltawski added a comment -

          LOL, great teamwork there by me and Damyon!

          Show
          Dan Poltawski added a comment - LOL, great teamwork there by me and Damyon!
          Hide
          Dan Poltawski added a comment -

          Discussed with damyon that this does produce different urls (with the / then url will be an absolute rather than relative url).

          But Damyon contends that this doesn't actually change in redirect() because redirect handles it.

          So failing this, awaiting clarification.

          Show
          Dan Poltawski added a comment - Discussed with damyon that this does produce different urls (with the / then url will be an absolute rather than relative url). But Damyon contends that this doesn't actually change in redirect() because redirect handles it. So failing this, awaiting clarification.
          Hide
          Rajesh Taneja added a comment -

          Thanks Dan and Damyon,

          I couldn't reproduce this as well, but seems it make sense to have this in.
          As this is relative path moodle_url->out() = view.php?pageid=1&group=0
          Passing this to redirect create http://rajesh.moodle.local/m/mod/wiki/edit.php/../view.php?pageid=1&group=0
          which uses me() function to get name of the current script (mod/wiki/edit.php).

          As per phpDoc for me(), it will fail if needed global variables are not set.

          me() returns $ME, which is set as $rurl['fullpath'] (initialise_fullme()) and fullpath is set by setup_get_remote_url() which has a lot of TODO's and no fall-back. Seems setup_get_remote_url() is failing in Paul's setup and hence he can see this issue.

          IMHO this should be integrated and create another issue to investigate why setup_get_remote_url() could fail.

          Show
          Rajesh Taneja added a comment - Thanks Dan and Damyon, I couldn't reproduce this as well, but seems it make sense to have this in. As this is relative path moodle_url->out() = view.php?pageid=1&group=0 Passing this to redirect create http://rajesh.moodle.local/m/mod/wiki/edit.php/../view.php?pageid=1&group=0 which uses me() function to get name of the current script (mod/wiki/edit.php). As per phpDoc for me(), it will fail if needed global variables are not set. me() returns $ME, which is set as $rurl ['fullpath'] (initialise_fullme()) and fullpath is set by setup_get_remote_url() which has a lot of TODO's and no fall-back. Seems setup_get_remote_url() is failing in Paul's setup and hence he can see this issue. IMHO this should be integrated and create another issue to investigate why setup_get_remote_url() could fail.
          Hide
          Dan Poltawski added a comment -

          We're not so hot on integrating un reproducible problems. However I don't think this change will do harm, so to get this through the process we need to check that it hasn't caused any regressions. So i'll send it back to testing.

          But please make it much clearer in future when you've not been able to reproduce a problem that is not the road for success for us

          Show
          Dan Poltawski added a comment - We're not so hot on integrating un reproducible problems. However I don't think this change will do harm, so to get this through the process we need to check that it hasn't caused any regressions. So i'll send it back to testing. But please make it much clearer in future when you've not been able to reproduce a problem that is not the road for success for us
          Hide
          Dan Poltawski added a comment -

          Passes, no regressions.

          Show
          Dan Poltawski added a comment - Passes, no regressions.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Your awesome contributions are now part of Moodle, your fav LMS out there.

          Closing this as fixed.

          Many thanks for all the hard work, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Your awesome contributions are now part of Moodle, your fav LMS out there. Closing this as fixed. Many thanks for all the hard work, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: