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

wiki doesn't redirect correctly when moodle installed in subdirectory

    Details

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

      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.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            kemitix 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
            kemitix 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
            salvetore 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
            salvetore 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
            rajeshtaneja 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
            rajeshtaneja 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
            kemitix 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
            kemitix 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
            rajeshtaneja 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
            rajeshtaneja 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
            kemitix 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
            kemitix 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
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks for providing the quick patch Paul,

            pushing it for peer-review.

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

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

            Show
            phalacee Jason Fowler added a comment - [Y] Syntax [-] Output [Y] Whitespace [-] Language [-] Databases [Y] Testing [-] Security [-] Documentation [Y] Git [Y] Sanity check
            Hide
            damyon 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 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 Damyon Wiese added a comment -

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

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

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

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

            LOL, great teamwork there by me and Damyon!

            Show
            poltawski Dan Poltawski added a comment - LOL, great teamwork there by me and Damyon!
            Hide
            poltawski 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
            poltawski 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
            rajeshtaneja 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
            rajeshtaneja 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
            poltawski 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
            poltawski 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
            poltawski Dan Poltawski added a comment -

            Passes, no regressions.

            Show
            poltawski Dan Poltawski added a comment - Passes, no regressions.
            Hide
            stronk7 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
            stronk7 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:
                  Fix Release Date:
                  13/May/13